[libvirt] [PATCH 3/3] storage: remove qemu-img help scraping
John Ferlan
jferlan at redhat.com
Wed Apr 18 12:24:49 UTC 2018
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 at 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);
>
More information about the libvir-list
mailing list