On 01/28/2014 11:48 AM, joel SIMOES wrote:
> From: Joel SIMOES <joel.sim...@laposte.net>
>
> Correct ret return 0 on success
> ---
>  src/storage/storage_backend_sheepdog.c | 92 
> ++++++++++++++++++++++++++++++----
>  1 file changed, 82 insertions(+), 10 deletions(-)
>
> diff --git a/src/storage/storage_backend_sheepdog.c 
> b/src/storage/storage_backend_sheepdog.c
> index a6981ce..75304b1 100644
> --- a/src/storage/storage_backend_sheepdog.c
> +++ b/src/storage/storage_backend_sheepdog.c
> @@ -86,7 +86,8 @@ virStorageBackendSheepdogParseNodeInfo(virStoragePoolDefPtr 
> pool,
>          pool->available = pool->capacity - pool->allocation;
>          return 0;
>  
> -    } while ((p = next));
> +    }
> +    while ((p = next));


You have ignored Eric's review comments (or perhaps you didn't see them?).

1) The above change is not only unnecessary, but changes the code to not
fit libvirt's coding style, so it must be removed.

2) Additionally (as Eric also mentioned), for anything more than a
trivial change, you *MUST* have a descriptive message in the commit log
that explains what was changed and why (e.g. a reference to a bug report
and/or a description of the bug, why it was occurring, how the code was
changed, and how it fixed the problem.)

We will need this information added to the commit log message before we
can consider pushing the patch.


>  
>      return -1;
>  }
> @@ -109,6 +110,70 @@ virStorageBackendSheepdogAddHostArg(virCommandPtr cmd,
>      virCommandAddArgFormat(cmd, "%d", port);
>  }
>  
> +static int
> +virStorageBackendSheepdogRefreshAllVol(virConnectPtr conn ATTRIBUTE_UNUSED,
> +                                       virStoragePoolObjPtr pool)
> +{
> +    int ret;
> +    char *output = NULL;
> +
> +    virCommandPtr cmd = virCommandNewArgList(COLLIE, "vdi", "list", "-r", 
> NULL);
> +    virStorageBackendSheepdogAddHostArg(cmd, pool);
> +    virCommandSetOutputBuffer(cmd, &output);
> +    ret = virCommandRun(cmd, NULL);
> +    char** lines;
> +    char** cells;
> +
> +    if (ret < 0)
> +        goto cleanup;
> +
> +    ret = -1;
> +    lines = virStringSplit(output, "\n", 0);
> +    size_t i;
> +    for (i = 0; *(lines + i); i++) {
> +        char *line = *(lines + i);
> +        cells = virStringSplit(line, " ", 0);
> +        size_t j;
> +        for (j = 0; *(cells + j); j++) {
> +
> +            char *cell = *(cells + j);
> +            if (j == 1) {
> +                virStorageVolDefPtr vol = NULL;
> +                if (VIR_ALLOC(vol) < 0)
> +                    goto cleanup;
> +
> +                if (VIR_STRDUP(vol->name, cell) < 0)
> +                    goto cleanup;
> +
> +                vol->type = VIR_STORAGE_VOL_BLOCK;
> +
> +                if (VIR_EXPAND_N(pool->volumes.objs, pool->volumes.count, 1) 
> < 0)
> +                    goto cleanup;
> +                pool->volumes.objs[pool->volumes.count - 1] = vol;
> +
> +                if (virStorageBackendSheepdogRefreshVol(conn, pool, vol) < 0)
> +                    goto cleanup;
> +
> +                vol = NULL;
> +            }
> +
> +            VIR_FREE(*(cells + j));
> +        }
> +
> +        VIR_FREE(cells);
> +        VIR_FREE(*(lines + i));
> +    }
> +
> +
> +    VIR_FREE(lines);
> +    ret = 0;
> +
> +cleanup:
> +    virCommandFree(cmd);
> +    VIR_FREE(lines);
> +    VIR_FREE(cells);
> +    return ret;
> +}
>  
>  static int
>  virStorageBackendSheepdogRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED,
> @@ -122,15 +187,18 @@ virStorageBackendSheepdogRefreshPool(virConnectPtr conn 
> ATTRIBUTE_UNUSED,
>      virStorageBackendSheepdogAddHostArg(cmd, pool);
>      virCommandSetOutputBuffer(cmd, &output);
>      ret = virCommandRun(cmd, NULL);
> -    if (ret == 0)
> -        ret = virStorageBackendSheepdogParseNodeInfo(pool->def, output);
> +    if (ret < 0)
> +        goto cleanup;
>  
> +    ret = virStorageBackendSheepdogParseNodeInfo(pool->def, output);
> +    if (ret < 0)
> +        goto cleanup;
> +    ret = virStorageBackendSheepdogRefreshAllVol(conn, pool);
> +cleanup:
>      virCommandFree(cmd);
> -    VIR_FREE(output);
>      return ret;
>  }
>  
> -
>  static int
>  virStorageBackendSheepdogDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED,
>                                     virStoragePoolObjPtr pool,
> @@ -143,12 +211,14 @@ virStorageBackendSheepdogDeleteVol(virConnectPtr conn 
> ATTRIBUTE_UNUSED,
>      virCommandPtr cmd = virCommandNewArgList(COLLIE, "vdi", "delete", 
> vol->name, NULL);
>      virStorageBackendSheepdogAddHostArg(cmd, pool);
>      int ret = virCommandRun(cmd, NULL);
> +    if (ret < 0)
> +        goto cleanup;
>  
> +cleanup:
>      virCommandFree(cmd);
>      return ret;
>  }
>  
> -
>  static int
>  virStorageBackendSheepdogCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED,
>                                     virStoragePoolObjPtr pool,
> @@ -174,7 +244,6 @@ virStorageBackendSheepdogCreateVol(virConnectPtr conn 
> ATTRIBUTE_UNUSED,
>      return 0;
>  }
>  
> -
>  static int
>  virStorageBackendSheepdogBuildVol(virConnectPtr conn,
>                                    virStoragePoolObjPtr pool,
> @@ -195,12 +264,12 @@ virStorageBackendSheepdogBuildVol(virConnectPtr conn,
>          goto cleanup;
>  
>      ret = 0;
> +
>  cleanup:
>      virCommandFree(cmd);
>      return ret;
>  }
>  
> -
>  int
>  virStorageBackendSheepdogParseVdiList(virStorageVolDefPtr vol,
>                                        char *output)
> @@ -257,7 +326,8 @@ virStorageBackendSheepdogParseVdiList(virStorageVolDefPtr 
> vol,
>              return -1;
>  
>          return 0;
> -    } while ((p = next));
> +    }
> +    while ((p = next));
>  
>      return -1;
>  }
> @@ -295,7 +365,6 @@ cleanup:
>      return ret;
>  }
>  
> -
>  static int
>  virStorageBackendSheepdogResizeVol(virConnectPtr conn ATTRIBUTE_UNUSED,
>                                     virStoragePoolObjPtr pool,
> @@ -310,7 +379,10 @@ virStorageBackendSheepdogResizeVol(virConnectPtr conn 
> ATTRIBUTE_UNUSED,
>      virCommandAddArgFormat(cmd, "%llu", capacity);
>      virStorageBackendSheepdogAddHostArg(cmd, pool);
>      int ret = virCommandRun(cmd, NULL);
> +    if (ret < 0)
> +        goto cleanup;
>  
> +cleanup:
>      virCommandFree(cmd);
>      return ret;
>  

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to