Re: [libvirt] [PATCH 3/3] storage: remove qemu-img help scraping

On 04/17/2018 05:43 PM, Ján Tomko wrote: > We have been checking whether qemu-img supports the -o compat > option by scraping the -help output. > > Since we require QEMU 1.5.0 now and this option was introduced in 1.1, > assume we support it and ditch the help parsing code along with the > extra qemu-img invocation. > > Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; > --- > src/storage/storage_util.c | 73 +++--------------------------------------- > src/storage/storage_util.h | 1 - > tests/storagevolxml2argvtest.c | 5 ++- > 3 files changed, 6 insertions(+), 73 deletions(-) > > diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c > index 897dfdaae..f7a4231e2 100644 > --- a/src/storage/storage_util.c > +++ b/src/storage/storage_util.c > @@ -787,61 +787,6 @@ storagePloopResize(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_OPTIONS = 0, > - QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT, > -}; > - > -static bool > -virStorageBackendQemuImgSupportsCompat(const char *qemuimg) > -{ > - bool ret = false; > - char *output; > - virCommandPtr cmd = NULL; > - > - cmd = virCommandNewArgList(qemuimg, "create", "-o", "?", "-f", "qcow2", > - "/dev/null", NULL); > - > - virCommandAddEnvString(cmd, "LC_ALL=C"); > - virCommandSetOutputBuffer(cmd, &output); > - > - if (virCommandRun(cmd, NULL) < 0) > - goto cleanup; > - > - if (strstr(output, "\ncompat ")) > - ret = true; > - > - cleanup: > - virCommandFree(cmd); > - VIR_FREE(output); > - return ret; > -} > - > - > -static int > -virStorageBackendQEMUImgBackingFormat(const char *qemuimg) > -{ > - /* 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; > - > - /* 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; > - > - return ret; > -} > > /* The _virStorageBackendQemuImgInfo separates the command line building from > * the volume definition so that qemuDomainSnapshotCreateInactiveExternal can > @@ -1089,14 +1034,12 @@ storageBackendCreateQemuImgSetBacking(virStoragePoolObjPtr pool, > > static int > storageBackendCreateQemuImgSetOptions(virCommandPtr cmd, > - int imgformat, > virStorageEncryptionInfoDefPtr enc, > struct _virStorageBackendQemuImgInfo info) > { > char *opts = NULL; > > - if (info.format == VIR_STORAGE_FILE_QCOW2 && !info.compat && > - imgformat >= QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT) > + if (info.format == VIR_STORAGE_FILE_QCOW2 && !info.compat) > info.compat = "0.10"; The comments for _COMPAT indicate that 0.11 was supported in QEMU 1.1 and that QEMU 2.0 started using that as the default. So, since this series is predicated upon QEMU 1.5 being the new default, why isn't this "0.11"?

For those installations stuck somewhere between 1.5 and 2.0, they could provide the "0.10" on the command line still > > if (storageBackendCreateQemuImgOpts(enc, &opts, info) < 0)

> @@ -154,7 +153,7 @@ testCompareXMLToArgvHelper(const void *data) > result = testCompareXMLToArgvFiles(info->shouldFail, poolxml, volxml, > inputpoolxml, inputvolxml, > cmdline, info->flags, > - info->imgformat, info->parseflags); > + info->parseflags); This effectively removes the need for info->imgformat and thus makes FMT_OPTIONS and FMT_COMPAT unnecessary. IOW: Those need to be removed from testInfo and the DO_TEST_FULL @define.

John > > cleanup: > VIR_FREE(poolxml); >