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@xxxxxxxxxx>
>>> ---
>>> 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);
>>>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
