[libvirt] [PATCH 2/4] logical: Create helper virStorageBackendLogicalParseVolDevice

[Pavel Hrdina]

On Fri, Jan 22, 2016 at 05:21:04PM -0500, John Ferlan wrote:
> Create a helper routine in order to parse the 'device' string contained
> within the generated 'lvs' output string.
> 
> A future patch would then be able to avoid the code more cleanly
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/storage/storage_backend_logical.c | 186 +++++++++++++++++++---------------
>  1 file changed, 104 insertions(+), 82 deletions(-)
> 
> diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c
> index 76ea00a..bf67faf 100644
> --- a/src/storage/storage_backend_logical.c
> +++ b/src/storage/storage_backend_logical.c
> @@ -72,21 +72,115 @@ struct virStorageBackendLogicalPoolVolData {
>  };
>  
>  static int
> -virStorageBackendLogicalMakeVol(char **const groups,
> -                                void *opaque)
> +virStorageBackendLogicalParseVolDevice(virStorageVolDefPtr vol,
> +                                       char **const groups,
> +                                       int nextents,
> +                                       unsigned long long size,
> +                                       unsigned long long length)
>  {

You've called the new helper *ParseVolDevice, but it actually does more than
that.  I would rather see it like ParseExtents and also move all the extents
related code into this function (parsing length and size)

[...]

> +    /* vars[0] is skipped */
> +    for (i = 0; i < nextents; i++) {
> +        size_t j;
> +        int len;
> +        char *offset_str = NULL;
> +
> +        j = (i * 2) + 1;
> +        len = vars[j].rm_eo - vars[j].rm_so;
> +        p[vars[j].rm_eo] = '\0';
> +
> +        if (VIR_STRNDUP(vol->source.extents[vol->source.nextent].path,
> +                        p + vars[j].rm_so, len) < 0)
> +            goto cleanup;
> +
> +        len = vars[j + 1].rm_eo - vars[j + 1].rm_so;
> +        if (VIR_STRNDUP(offset_str, p + vars[j + 1].rm_so, len) < 0)
> +            goto cleanup;
> +
> +        if (virStrToLong_ull(offset_str, NULL, 10, &offset) < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("malformed volume extent offset value"));
> +            VIR_FREE(offset_str);
> +            goto cleanup;
> +        }
> +
> +        VIR_FREE(offset_str);
> +
> +        vol->source.extents[vol->source.nextent].start = offset * size;
> +        vol->source.extents[vol->source.nextent].end = (offset * size) + length;
> +        vol->source.nextent++;

This would be much nicer to be done using VIR_APPEND_ELEMENT().  I know that
this commit is just a code movement so this should be done in separate commit.