[libvirt] [PATCH v5 3/5] storage: add support for creating qcow2 images with extensions

[John Ferlan]

On 06/19/2013 11:24 AM, Ján Tomko wrote:
> Add -o compat= and -o lazy_refcounts options for qemu-img.
> ---
>  src/storage/storage_backend.c                   | 62 ++++++++++++++++++++++---
>  tests/storagevolxml2argvdata/qcow2-1.1.argv     |  1 +
>  tests/storagevolxml2argvdata/qcow2-lazy.argv    |  1 +
>  tests/storagevolxml2argvdata/vol-qcow2-1.1.xml  | 32 +++++++++++++
>  tests/storagevolxml2argvdata/vol-qcow2-lazy.xml | 35 ++++++++++++++
>  tests/storagevolxml2argvtest.c                  |  2 +
>  6 files changed, 126 insertions(+), 7 deletions(-)
>  create mode 100644 tests/storagevolxml2argvdata/qcow2-1.1.argv
>  create mode 100644 tests/storagevolxml2argvdata/qcow2-lazy.argv
>  create mode 100644 tests/storagevolxml2argvdata/vol-qcow2-1.1.xml
>  create mode 100644 tests/storagevolxml2argvdata/vol-qcow2-lazy.xml
> 
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index deea850..b2c9608 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -633,9 +633,15 @@ static int
>  virStorageBackendCreateQemuImgOpts(char **opts,
>                                     const char *backingType,
>                                     bool encryption,
> -                                   bool preallocate)
> +                                   bool preallocate,
> +                                   int format,
> +                                   const char *compat,
> +                                   virBitmapPtr features)
>  {
>      virBuffer buf = VIR_BUFFER_INITIALIZER;
> +    bool b;
> +    int i;
> +
>      if (backingType)
>          virBufferAsprintf(&buf, "backing_fmt=%s,", backingType);
>      if (encryption)
> @@ -643,16 +649,45 @@ virStorageBackendCreateQemuImgOpts(char **opts,
>      if (preallocate)
>          virBufferAddLit(&buf, "preallocation=metadata,");
>  
> +    if (compat)
> +        virBufferAsprintf(&buf, "compat=%s,", compat);
> +    if (features && format == VIR_STORAGE_FILE_QCOW2) {
> +        for (i = 0; i < VIR_STORAGE_FILE_FEATURE_LAST; i++) {
> +            ignore_value(virBitmapGetBit(features, i, &b));
> +            if (b) {
> +                switch (i) {
> +                case VIR_STORAGE_FILE_FEATURE_LAZY_REFCOUNTS:
> +                    if (STREQ(compat, "0.10")) {
> +                        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                                       _("Feature %s not supported with compat"
> +                                         " level %s"),
> +                                       virStorageFileFeatureTypeToString(i),
> +                                       compat);
> +                        goto error;
> +                    }
> +                    break;


With the patches for virNetDevSetupControlFull() my Coverity now runs;
however, it's not very happy here...

The pushed source code  has:
...
+                case VIR_STORAGE_FILE_FEATURE_NONE:
+                case VIR_STORAGE_FILE_FEATURE_LAST:
+                    ;

But VIR_STORAGE_FILE_FEATURE_NONE = -1 and VIR_STORAGE_FILE_FEATURE_LAST
is used as the loop ender and thus Coverity deems the code unreachable
eliciting a "DEADCODE" error since 'i' could never be one or the other.

I suppose the loop could change to:

for (i = VIR_STORAGE_FILE_FEATURE_NONE;
     i <= VIR_STORAGE_FILE_FEATURE_LAST; i++)

but that'd cause virBitmapGetBit() failure since 'i' cannot be negative
in the call to it.

John






> +                case VIR_STORAGE_FILE_FEATURE_LAST:
> +                    ;
> +                }
> +                virBufferAsprintf(&buf, "%s,",
> +                                  virStorageFileFeatureTypeToString(i));
> +            }
> +        }
> +    }
> +
>      virBufferTrim(&buf, ",", -1);
...