[libvirt] [PATCH v2]Sheepdog: Auto Adding volume and correct refresh volume problem.

[Laine Stump]

On 01/28/2014 11:48 AM, joel SIMOES wrote:
> From: Joel SIMOES <joel.simoes at 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;
>