[libvirt] [PATCH 0/3] Ditch qemu-img help scraping
Ján Tomko (3): tests: delete most qemu-img test cases assuming FMT_OPTIONS tests: assume FMT_COMPAT for qemu-img tests storage: remove qemu-img help scraping src/storage/storage_util.c | 73 ++------------------- src/storage/storage_util.h | 1 - .../qcow2-convert-nobacking.argv | 2 - .../storagevolxml2argvdata/qcow2-from-logical.argv | 2 - .../qcow2-nobacking-convert-prealloc.argv | 2 - .../qcow2-nobacking-prealloc.argv | 2 - .../qcow2-nocapacity-convert-prealloc.argv | 2 +- tests/storagevolxml2argvdata/qcow2-nocapacity.argv | 2 +- tests/storagevolxml2argvdata/qcow2.argv | 2 - tests/storagevolxml2argvtest.c | 74 ++++++---------------- 10 files changed, 24 insertions(+), 138 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 -- 2.13.6
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@redhat.com> --- .../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
On 04/17/2018 05:43 PM, Ján Tomko wrote:
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@redhat.com> --- .../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
For some reason in my mind I had R-by'd the first two, but I guess I didn't - my bad... Reviewed-by: John Ferlan <jferlan@redhat.com> John
No point in testing outdated command lines. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- .../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
On 04/17/2018 05:43 PM, Ján Tomko wrote:
No point in testing outdated command lines.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- .../qcow2-nocapacity-convert-prealloc.argv | 2 +- tests/storagevolxml2argvdata/qcow2-nocapacity.argv | 2 +- tests/storagevolxml2argvtest.c | 32 +++++++++++----------- 3 files changed, 18 insertions(+), 18 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
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@redhat.com> --- 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 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@redhat.com> --- 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) @@ -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);
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);
On Wed, Apr 18, 2018 at 08:24:49AM -0400, John Ferlan wrote:
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@redhat.com> --- 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"?
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.
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.
Right. Jano
John
cleanup: VIR_FREE(poolxml);
On 04/18/2018 10:09 AM, Ján Tomko wrote:
On Wed, Apr 18, 2018 at 08:24:49AM -0400, John Ferlan wrote:
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@redhat.com> --- 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"?
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.
Insufficient coffee, bad fingers... "1.1" So, based on Dan's comment in my series for encryption - should we "punt" and drop compat completely allowing qemu-img to decide or just use "1.1" unconditionally? Effects patch2 in this series. John
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.
Right.
Jano
John
cleanup: VIR_FREE(poolxml);
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@redhat.com> --- 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/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);
As long as you also remove FMT_OPTIONS and FMT_COMPAT and @imgformat from testInfo, then Reviewed-by: John Ferlan <jferlan@redhat.com> John
participants (2)
-
John Ferlan -
Ján Tomko