We have two leftover "capabilites" for qemu-img: QEMU_IMG_BACKING_FORMAT_OPTIONS QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT The former says we are able to specify the backing format via -o (which has been the case for a long time now) and the second one says we can use -o compat to specify the qcow2 version. Since we require QEMU 1.5.0, we can always assume -o compat, which was introduced in QEMU 1.1. Drop the test cases using FMT_OPTIONS which have a FMT_COMPAT counterpart to prepare for deprecating FMT_OPTIONS (and these flags) completely. Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; --- .../qcow2-convert-nobacking.argv | 2 -- .../storagevolxml2argvdata/qcow2-from-logical.argv | 2 -- .../qcow2-nobacking-convert-prealloc.argv | 2 -- .../qcow2-nobacking-prealloc.argv | 2 -- tests/storagevolxml2argvdata/qcow2.argv | 2 -- tests/storagevolxml2argvtest.c | 37 ---------------------- 6 files changed, 47 deletions(-) delete mode 100644 tests/storagevolxml2argvdata/qcow2-convert-nobacking.argv delete mode 100644 tests/storagevolxml2argvdata/qcow2-from-logical.argv delete mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc.argv delete mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-prealloc.argv delete mode 100644 tests/storagevolxml2argvdata/qcow2.argv diff --git a/tests/storagevolxml2argvdata/qcow2-convert-nobacking.argv b/tests/storagevolxml2argvdata/qcow2-convert-nobacking.argv deleted file mode 100644 index fd1f4c078..000000000 --- a/tests/storagevolxml2argvdata/qcow2-convert-nobacking.argv +++ /dev/null @@ -1,2 +0,0 @@ -qemu-img convert -f raw -O qcow2 -o encryption=on \ -/var/lib/libvirt/images/sparse.img /var/lib/libvirt/images/OtherDemo.img diff --git a/tests/storagevolxml2argvdata/qcow2-from-logical.argv b/tests/storagevolxml2argvdata/qcow2-from-logical.argv deleted file mode 100644 index 6a7581564..000000000 --- a/tests/storagevolxml2argvdata/qcow2-from-logical.argv +++ /dev/null @@ -1,2 +0,0 @@ -qemu-img convert -f raw -O qcow2 -o encryption=on /dev/HostVG/Swap \ -/var/lib/libvirt/images/OtherDemo.img diff --git a/tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc.argv b/tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc.argv deleted file mode 100644 index a49285f89..000000000 --- a/tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc.argv +++ /dev/null @@ -1,2 +0,0 @@ -qemu-img convert -f raw -O qcow2 -o encryption=on,preallocation=metadata \ -/var/lib/libvirt/images/sparse.img /var/lib/libvirt/images/OtherDemo.img diff --git a/tests/storagevolxml2argvdata/qcow2-nobacking-prealloc.argv b/tests/storagevolxml2argvdata/qcow2-nobacking-prealloc.argv deleted file mode 100644 index c74258882..000000000 --- a/tests/storagevolxml2argvdata/qcow2-nobacking-prealloc.argv +++ /dev/null @@ -1,2 +0,0 @@ -qemu-img create -f qcow2 -o encryption=on,preallocation=metadata \ -/var/lib/libvirt/images/OtherDemo.img 5242880K diff --git a/tests/storagevolxml2argvdata/qcow2.argv b/tests/storagevolxml2argvdata/qcow2.argv deleted file mode 100644 index 6ca9a45f0..000000000 --- a/tests/storagevolxml2argvdata/qcow2.argv +++ /dev/null @@ -1,2 +0,0 @@ -qemu-img create -f qcow2 -b /dev/null -o backing_fmt=raw,encryption=on \ -/var/lib/libvirt/images/OtherDemo.img 5242880K diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c index 5857d5e35..4353ad467 100644 --- a/tests/storagevolxml2argvtest.c +++ b/tests/storagevolxml2argvtest.c @@ -198,40 +198,6 @@ mymain(void) DO_TEST("pool-dir", "vol-qcow2", NULL, NULL, - "qcow2", 0, FMT_OPTIONS); - DO_TEST_FAIL("pool-dir", "vol-qcow2", - NULL, NULL, - "qcow2-prealloc", flags, FMT_OPTIONS); - DO_TEST("pool-dir", "vol-qcow2-nobacking", - NULL, NULL, - "qcow2-nobacking-prealloc", flags, FMT_OPTIONS); - DO_TEST("pool-dir", "vol-qcow2-nobacking", - "pool-dir", "vol-file", - "qcow2-nobacking-convert-prealloc", flags, FMT_OPTIONS); - DO_TEST_FAIL("pool-dir", "vol-qcow2", - "pool-dir", "vol-file", - "qcow2-convert-nobacking", 0, FMT_OPTIONS); - DO_TEST_FAIL("pool-dir", "vol-qcow2", - "pool-dir", "vol-file", - "qcow2-convert-prealloc", flags, FMT_OPTIONS); - DO_TEST("pool-dir", "vol-qcow2-lazy", - NULL, NULL, - "qcow2-lazy", 0, FMT_OPTIONS); - DO_TEST("pool-dir", "vol-qcow2-1.1", - NULL, NULL, - "qcow2-1.1", 0, FMT_OPTIONS); - DO_TEST_FAIL("pool-dir", "vol-qcow2-0.10-lazy", - NULL, NULL, - "qcow2-0.10-lazy", 0, FMT_OPTIONS); - DO_TEST("pool-dir", "vol-qcow2-nobacking", - "pool-logical", "vol-logical", - "qcow2-from-logical", 0, FMT_OPTIONS); - DO_TEST("pool-logical", "vol-logical", - "pool-dir", "vol-qcow2-nobacking", - "logical-from-qcow2", 0, FMT_OPTIONS); - - DO_TEST("pool-dir", "vol-qcow2", - NULL, NULL, "qcow2-compat", 0, FMT_COMPAT); DO_TEST("pool-dir", "vol-qcow2-nobacking", NULL, NULL, @@ -256,9 +222,6 @@ mymain(void) "logical-from-qcow2", 0, FMT_COMPAT); DO_TEST("pool-dir", "vol-qcow2-nocow", NULL, NULL, - "qcow2-nocow", 0, FMT_OPTIONS); - DO_TEST("pool-dir", "vol-qcow2-nocow", - NULL, NULL, "qcow2-nocow-compat", 0, FMT_COMPAT); DO_TEST("pool-dir", "vol-qcow2-nocapacity", "pool-dir", "vol-file", -- 2.13.6

No point in testing outdated command lines. Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; --- .../qcow2-nocapacity-convert-prealloc.argv | 2 +- tests/storagevolxml2argvdata/qcow2-nocapacity.argv | 2 +- tests/storagevolxml2argvtest.c | 32 +++++++++++----------- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv b/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv index b151b9401..73499178e 100644 --- a/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv +++ b/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv @@ -1,4 +1,4 @@ qemu-img convert -f raw -O qcow2 \ --o encryption=on,preallocation=falloc \ +-o encryption=on,preallocation=falloc,compat=0.10 \ /var/lib/libvirt/images/sparse.img \ /var/lib/libvirt/images/OtherDemo.img diff --git a/tests/storagevolxml2argvdata/qcow2-nocapacity.argv b/tests/storagevolxml2argvdata/qcow2-nocapacity.argv index 1198cbaf2..fd8805589 100644 --- a/tests/storagevolxml2argvdata/qcow2-nocapacity.argv +++ b/tests/storagevolxml2argvdata/qcow2-nocapacity.argv @@ -1,5 +1,5 @@ qemu-img create \ -f qcow2 \ -b /dev/null \ --o backing_fmt=raw,encryption=on \ +-o backing_fmt=raw,encryption=on,compat=0.10 \ /var/lib/libvirt/images/OtherDemo.img diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c index 4353ad467..68ee9c3d8 100644 --- a/tests/storagevolxml2argvtest.c +++ b/tests/storagevolxml2argvtest.c @@ -180,10 +180,10 @@ mymain(void) unsigned int flags = VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA; #define DO_TEST_FULL(shouldFail, parseflags, pool, vol, inputpool, inputvol, \ - cmdline, flags, imgformat) \ + cmdline, flags) \ do { \ struct testInfo info = { shouldFail, pool, vol, inputpool, inputvol, \ - cmdline, flags, imgformat, parseflags }; \ + cmdline, flags, FMT_COMPAT, parseflags }; \ if (virTestRun("Storage Vol XML-2-argv " cmdline, \ testCompareXMLToArgvHelper, &info) < 0) \ ret = -1; \ @@ -198,47 +198,47 @@ mymain(void) DO_TEST("pool-dir", "vol-qcow2", NULL, NULL, - "qcow2-compat", 0, FMT_COMPAT); + "qcow2-compat", 0); DO_TEST("pool-dir", "vol-qcow2-nobacking", NULL, NULL, - "qcow2-nobacking-prealloc-compat", flags, FMT_COMPAT); + "qcow2-nobacking-prealloc-compat", flags); DO_TEST("pool-dir", "vol-qcow2-nobacking", "pool-dir", "vol-file", - "qcow2-nobacking-convert-prealloc-compat", flags, FMT_COMPAT); + "qcow2-nobacking-convert-prealloc-compat", flags); DO_TEST("pool-dir", "vol-qcow2-lazy", NULL, NULL, - "qcow2-lazy", 0, FMT_COMPAT); + "qcow2-lazy", 0); DO_TEST("pool-dir", "vol-qcow2-1.1", NULL, NULL, - "qcow2-1.1", 0, FMT_COMPAT); + "qcow2-1.1", 0); DO_TEST_FAIL("pool-dir", "vol-qcow2-0.10-lazy", NULL, NULL, - "qcow2-0.10-lazy", 0, FMT_COMPAT); + "qcow2-0.10-lazy", 0); DO_TEST("pool-dir", "vol-qcow2-nobacking", "pool-logical", "vol-logical", - "qcow2-from-logical-compat", 0, FMT_COMPAT); + "qcow2-from-logical-compat", 0); DO_TEST("pool-logical", "vol-logical", "pool-dir", "vol-qcow2-nobacking", - "logical-from-qcow2", 0, FMT_COMPAT); + "logical-from-qcow2", 0); DO_TEST("pool-dir", "vol-qcow2-nocow", NULL, NULL, - "qcow2-nocow-compat", 0, FMT_COMPAT); + "qcow2-nocow-compat", 0); DO_TEST("pool-dir", "vol-qcow2-nocapacity", "pool-dir", "vol-file", - "qcow2-nocapacity-convert-prealloc", flags, FMT_OPTIONS); + "qcow2-nocapacity-convert-prealloc", flags); DO_TEST("pool-dir", "vol-qcow2-zerocapacity", NULL, NULL, - "qcow2-zerocapacity", 0, FMT_COMPAT); + "qcow2-zerocapacity", 0); DO_TEST_FULL(false, VIR_VOL_XML_PARSE_OPT_CAPACITY, "pool-dir", "vol-qcow2-nocapacity-backing", NULL, NULL, - "qcow2-nocapacity", 0, FMT_OPTIONS); + "qcow2-nocapacity", 0); DO_TEST("pool-dir", "vol-file-iso", NULL, NULL, - "iso", 0, FMT_OPTIONS); + "iso", 0); DO_TEST("pool-dir", "vol-file", "pool-dir", "vol-file-iso", - "iso-input", 0, FMT_OPTIONS); + "iso-input", 0); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.13.6

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"; if (storageBackendCreateQemuImgOpts(enc, &opts, info) < 0) @@ -1170,16 +1113,13 @@ storageBackendResizeQemuImgImageOpts(virCommandPtr cmd, } -/* Create a qemu-img virCommand from the supplied binary path, - * volume definitions and imgformat - */ +/* Create a qemu-img virCommand from the supplied arguments */ virCommandPtr virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol, unsigned int flags, const char *create_tool, - int imgformat, const char *secretPath) { virCommandPtr cmd = NULL; @@ -1293,7 +1233,7 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, enc = &vol->target.encryption->encinfo; } - if (storageBackendCreateQemuImgSetOptions(cmd, imgformat, enc, info) < 0) + if (storageBackendCreateQemuImgSetOptions(cmd, enc, info) < 0) goto error; VIR_FREE(info.secretAlias); @@ -1386,7 +1326,6 @@ storageBackendCreateQemuImg(virStoragePoolObjPtr pool, { int ret = -1; char *create_tool; - int imgformat; virCommandPtr cmd; char *secretPath = NULL; @@ -1400,10 +1339,6 @@ storageBackendCreateQemuImg(virStoragePoolObjPtr pool, return -1; } - imgformat = virStorageBackendQEMUImgBackingFormat(create_tool); - if (imgformat < 0) - goto cleanup; - if (vol->target.format == VIR_STORAGE_FILE_RAW && vol->target.encryption && vol->target.encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) { @@ -1414,7 +1349,7 @@ storageBackendCreateQemuImg(virStoragePoolObjPtr pool, cmd = virStorageBackendCreateQemuImgCmdFromVol(pool, vol, inputvol, flags, create_tool, - imgformat, secretPath); + secretPath); if (!cmd) goto cleanup; diff --git a/src/storage/storage_util.h b/src/storage/storage_util.h index e9cb98211..930770275 100644 --- a/src/storage/storage_util.h +++ b/src/storage/storage_util.h @@ -159,7 +159,6 @@ virStorageBackendCreateQemuImgCmdFromVol(virStoragePoolObjPtr pool, virStorageVolDefPtr inputvol, unsigned int flags, const char *create_tool, - int imgformat, const char *secretPath); int virStorageBackendSCSIFindLUs(virStoragePoolObjPtr pool, diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c index 68ee9c3d8..ca44ccd48 100644 --- a/tests/storagevolxml2argvtest.c +++ b/tests/storagevolxml2argvtest.c @@ -40,7 +40,6 @@ testCompareXMLToArgvFiles(bool shouldFail, const char *inputvolxml, const char *cmdline, unsigned int flags, - int imgformat, unsigned long parse_flags) { char *actualCmdline = NULL; @@ -82,7 +81,7 @@ testCompareXMLToArgvFiles(bool shouldFail, cmd = virStorageBackendCreateQemuImgCmdFromVol(obj, vol, inputvol, flags, - create_tool, imgformat, + create_tool, NULL); if (!cmd) { if (shouldFail) { @@ -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); cleanup: VIR_FREE(poolxml); -- 2.13.6

On Wed, Apr 18, 2018 at 08:24:49AM -0400, John Ferlan wrote: I am not sure what you're asking here. Per the comment above, 0.11 is the version where you could specify the backing format via -o backing_fmt. Before that, we used the -F option, but we dropped that in commit f6a92f8e. 1.1 is the version that introduced qcow2v3, so it's the first one to support the compat option. In 2.0, qemu changed the default compat from 0.10 to 1.1 and libvirt started always specifying -o compat to preserve XML->qcow2 format compatibility (qcow2v3 needs to be requested explicitly via <compat>1.1</compat>). No idea where you got "0.11" from. Right. Jano

On 04/17/2018 05:43 PM, Ján Tomko wrote: [...] As long as you also remove FMT_OPTIONS and FMT_COMPAT and @imgformat from testInfo, then Reviewed-by: John Ferlan <jferlan(a)redhat.com&gt; John