[libvirt] [PATCH v2 1/3] storage: Adjust qemu-img switches check

[Peter Krempa]

On Mon, Jun 06, 2016 at 13:24:50 -0400, John Ferlan wrote:
> Since we support QEMU 0.12 and later, checking for support of specific flags
> added prior to that isn't necessary.
> 
> Thus start with the base of having the "-o options" available for the
> qemu-img create option and then determine whether we have the compat
> option for qcow2 files (which would be necessary up through qemu 2.0
> where the default changes to compat 0.11).
> 
> Adjust test to no long check for NONE and FLAG options as well was removing
> results of tests that would use that option.
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/storage/storage_backend.c                      | 91 +++++++---------------
>  tests/storagevolxml2argvdata/qcow2-flag.argv       |  2 -
>  .../qcow2-nobacking-convert-flag.argv              |  2 -
>  .../qcow2-nobacking-convert-none.argv              |  2 -
>  .../qcow2-nobacking-flag.argv                      |  1 -
>  .../qcow2-nobacking-none.argv                      |  1 -
>  tests/storagevolxml2argvdata/qcow2-none.argv       |  1 -
>  tests/storagevolxml2argvtest.c                     | 22 +-----
>  8 files changed, 29 insertions(+), 93 deletions(-)
>  delete mode 100644 tests/storagevolxml2argvdata/qcow2-flag.argv
>  delete mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-convert-flag.argv
>  delete mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-convert-none.argv
>  delete mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-flag.argv
>  delete mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-none.argv
>  delete mode 100644 tests/storagevolxml2argvdata/qcow2-none.argv
> 
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index 8f03a6e..ff3f8c1 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -869,10 +869,15 @@ virStoragePloopResize(virStorageVolDefPtr vol,
>      return ret;
>  }
>  
> +/* Flag values shared w/ storagevolxml2argvtest.c.
> + *
> + * QEMU_IMG_BACKING_FORMAT_OPTIONS (added in qemu 0.11)
> + * QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT
> + *    was made necessary due to 2.0 change to change the default
> + *    qcow2 file format from 0.10 to 1.1.
> + */
>  enum {
> -    QEMU_IMG_BACKING_FORMAT_NONE = 0,
> -    QEMU_IMG_BACKING_FORMAT_FLAG,
> -    QEMU_IMG_BACKING_FORMAT_OPTIONS,
> +    QEMU_IMG_BACKING_FORMAT_OPTIONS = 0,
>      QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT,

So if these are declared as flags they either should be binary or-able
or be used as indexes into a virBitmap as we do for qemu caps.

This use doesn't make sense as you can effectively return just one
capability.

>  };
>  
> @@ -904,46 +909,18 @@ virStorageBackendQemuImgSupportsCompat(const char *qemuimg)
>  static int
>  virStorageBackendQEMUImgBackingFormat(const char *qemuimg)
>  {
> -    char *help = NULL;
> -    char *start;
> -    char *end;
> -    char *tmp;
> -    int ret = -1;
> -    int exitstatus;
> -    virCommandPtr cmd = virCommandNewArgList(qemuimg, "-h", NULL);
> -
> -    virCommandAddEnvString(cmd, "LC_ALL=C");
> -    virCommandSetOutputBuffer(cmd, &help);
> -    virCommandClearCaps(cmd);
> -
> -    /* qemuimg doesn't return zero exit status on -h,
> -     * therefore we need to provide pointer for storing
> -     * exit status, although we don't parse it any later */
> -    if (virCommandRun(cmd, &exitstatus) < 0)
> -        goto cleanup;
> -
> -    if ((start = strstr(help, " create ")) == NULL ||
> -        (end = strstr(start, "\n")) == NULL) {
> -        virReportError(VIR_ERR_INTERNAL_ERROR,
> -                       _("unable to parse qemu-img output '%s'"),
> -                       help);
> -        goto cleanup;
> -    }
> -    if (((tmp = strstr(start, "-F fmt")) && tmp < end) ||
> -        ((tmp = strstr(start, "-F backing_fmt")) && tmp < end)) {
> -        ret = QEMU_IMG_BACKING_FORMAT_FLAG;
> -    } else if ((tmp = strstr(start, "[-o options]")) && tmp < end) {
> -        if (virStorageBackendQemuImgSupportsCompat(qemuimg))
> -            ret = QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT;
> -        else
> -            ret = QEMU_IMG_BACKING_FORMAT_OPTIONS;
> -    } else {
> -        ret = QEMU_IMG_BACKING_FORMAT_NONE;
> -    }
> +    /* As of QEMU 0.11 the [-o options] support was added via qemu
> +     * commit id '9ea2ea71', so we start with that base and figure
> +     * out what else we have */
> +    int ret = QEMU_IMG_BACKING_FORMAT_OPTIONS;

If this flag is always set (and 0) it doesn't make sense to keep it.

> +
> +    /* QEMU 2.0 changed to using a format that only QEMU 1.1 and newer
> +     * understands. Since we still support QEMU 0.12 and newer, we need
> +     * to be able to handle the previous format as can be set via a
> +     * compat=0.10 option. */
> +    if (virStorageBackendQemuImgSupportsCompat(qemuimg))
> +        ret = QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT;
>  
> - cleanup:
> -    virCommandFree(cmd);
> -    VIR_FREE(help);
>      return ret;
>  }
>