[libvirt] [PATCH 1/7] storage: rework qemu-img command line generation

[Martin Kletzander]

On 06/10/2013 02:10 PM, Ján Tomko wrote:
> Split out option string generation to make adding new options easier
> and simplify the code.
> ---
>  src/storage/storage_backend.c | 111 ++++++++++++++++++++++--------------------
>  1 file changed, 59 insertions(+), 52 deletions(-)
> 
[...]
> @@ -795,65 +824,43 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn,
>  
>      cmd = virCommandNew(create_tool);
>  
> -    if (inputvol) {
> -        virCommandAddArgList(cmd, "convert", "-f", inputType, "-O", type, NULL);
> +    convert = !!inputvol;
> +    backing = !inputvol && vol->backingStore.path;
>  
> -        if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS &&
> -            (do_encryption || preallocate)) {
> -            virCommandAddArg(cmd, "-o");
> -            virCommandAddArgFormat(cmd, "%s%s%s", do_encryption ? "encryption=on" : "",
> -                                   (do_encryption && preallocate) ? "," : "",
> -                                   preallocate ? "preallocation=metadata" : "");
> -        } else if (do_encryption) {
> -            virCommandAddArg(cmd, "-e");
> -        }
> -        virCommandAddArgList(cmd, inputPath, vol->target.path, NULL);
> -    } else if (vol->backingStore.path) {
> -        virCommandAddArgList(cmd, "create", "-f", type,
> -                             "-b", vol->backingStore.path, NULL);
> -
> -        switch (imgformat) {
> -        case QEMU_IMG_BACKING_FORMAT_FLAG:
> -            virCommandAddArgList(cmd, "-F", backingType, NULL);
> -            if (do_encryption)
> -                virCommandAddArg(cmd, "-e");
> -            virCommandAddArg(cmd, vol->target.path);
> -            virCommandAddArgFormat(cmd, "%lluK", size_arg);
> -            break;
> -
> -        case QEMU_IMG_BACKING_FORMAT_OPTIONS:
> -            virCommandAddArg(cmd, "-o");
> -            virCommandAddArgFormat(cmd, "backing_fmt=%s%s", backingType,
> -                                   do_encryption ? ",encryption=on" : "");
> -            virCommandAddArg(cmd, vol->target.path);
> -            virCommandAddArgFormat(cmd, "%lluK", size_arg);
> -            break;
> +    if (convert)
> +        virCommandAddArgList(cmd, "convert", "-f", inputType, "-O", type, NULL);
> +    else
> +        virCommandAddArgList(cmd, "create", "-f", type, NULL);
>  
> -        default:
> -            VIR_DEBUG("Unable to set backing store format for %s with %s",
> -                      vol->target.path, create_tool);
> +    if (backing)
> +        virCommandAddArgList(cmd, "-b", vol->backingStore.path, NULL);
>  
> -            if (do_encryption)
> -                virCommandAddArg(cmd, "-e");
> -            virCommandAddArg(cmd, vol->target.path);
> -            virCommandAddArgFormat(cmd, "%lluK", size_arg);
> -        }
> +    if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) {
> +        if (virStorageBackendCreateQemuImgOpts(&opts,
> +                                               backing ? backingType : NULL,
> +                                               do_encryption, preallocate))

I guess we should use '< 0' here to unify the looks of it.

Other than that it's way more readable than the previous version, so ACK
with that fixed.

Martin