On Fri, Jun 03, 2016 at 06:42:09 -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).
>
> NOTE: Keeping old tests around since it's still possible to create in
> the old format.
>
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
> src/storage/storage_backend.c | 66 ++++++++++++++++---------------------------
> 1 file changed, 24 insertions(+), 42 deletions(-)
>
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index 2076155..4c40e43 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -869,9 +869,19 @@ virStoragePloopResize(virStorageVolDefPtr vol,
> return ret;
> }
>
> +/* Flag values shared w/ storagevolxml2argvtest.c.
> + * Since it's still possible to provide the old format args, just
> + * keep them; however, prefix with an "X_" (similar to qemu_capabilities.c)
> + * to indicate the are older.
We use that approach as we can't delete the already existing flags since
we wouldn't be able to parse the status XML. For qemu-img we don't store
the flags anywhere so the old ones can be deleted.
> + *
> + * 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,
> + X_QEMU_IMG_BACKING_FORMAT_NONE = 0,
So this can be dropped entirely after the hunk below.
> + X_QEMU_IMG_BACKING_FORMAT_FLAG,
This will be never set either after the hunk below.
> QEMU_IMG_BACKING_FORMAT_OPTIONS,
> QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT,
> };
> @@ -904,46 +914,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;
> + /* 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;
So this flag can basically be completely removed as it should be always
present in qemu-img 0.12
> +
> + /* 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;
Yup, we need this.
>
> - 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;
> - }
> -
> - cleanup:
> - virCommandFree(cmd);
> - VIR_FREE(help);
> return ret;
> }
>
> @@ -1219,7 +1201,7 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn,
> VIR_FREE(opts);
> } else {
> if (info.backingPath) {
> - if (imgformat == QEMU_IMG_BACKING_FORMAT_FLAG)
> + if (imgformat == X_QEMU_IMG_BACKING_FORMAT_FLAG)
So ... if the above is never set, why aren't you dropping this entire
case?
> virCommandAddArgList(cmd, "-F", backingType, NULL);
> else
> VIR_DEBUG("Unable to set backing store format for %s with %s",
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list