[PATCH v2] storage: Upgrade default qcow2 verion to 1.1
Change the default to modern qcow2 as it's supported by all qemu versions supported by libvirt and in fact 'qemu-img' already defaults to the new format for a long time. Some Unittests require changes to pass, now that version 1.1 is default. Unittests like `qcow2-1.1.argv` may not be relevant anymore, but this patch doesn't affect them. Signed-off-by: Abhiram Tilak <atp.exp@gmail.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- src/storage/storage_util.c | 2 +- .../storagevolxml2argvdata/luks-convert-encrypt2fileqcow2.argv | 2 +- tests/storagevolxml2argvdata/qcow2-compat.argv | 2 +- tests/storagevolxml2argvdata/qcow2-from-logical-compat.argv | 2 +- tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt.argv | 2 +- .../qcow2-luks-convert-encrypt2fileqcow2.argv | 2 +- tests/storagevolxml2argvdata/qcow2-luks.argv | 2 +- .../qcow2-nobacking-convert-prealloc-compat.argv | 2 +- .../storagevolxml2argvdata/qcow2-nobacking-prealloc-compat.argv | 2 +- .../qcow2-nocapacity-convert-prealloc.argv | 2 +- tests/storagevolxml2argvdata/qcow2-nocapacity.argv | 2 +- tests/storagevolxml2argvdata/qcow2-nocow-compat.argv | 2 +- tests/storagevolxml2argvdata/qcow2-zerocapacity.argv | 2 +- 13 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 7bf815d978..28d5fce4f0 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -765,7 +765,7 @@ storageBackendCreateQemuImgOpts(virStorageEncryptionInfoDef *encinfo, if (info->compat) virBufferAsprintf(&buf, "compat=%s,", info->compat); else if (info->format == VIR_STORAGE_FILE_QCOW2) - virBufferAddLit(&buf, "compat=0.10,"); + virBufferAddLit(&buf, "compat=1.1,"); if (info->clusterSize > 0) virBufferAsprintf(&buf, "cluster_size=%llu,", info->clusterSize); diff --git a/tests/storagevolxml2argvdata/luks-convert-encrypt2fileqcow2.argv b/tests/storagevolxml2argvdata/luks-convert-encrypt2fileqcow2.argv index 4b9ccfe8dc..705604b162 100644 --- a/tests/storagevolxml2argvdata/luks-convert-encrypt2fileqcow2.argv +++ b/tests/storagevolxml2argvdata/luks-convert-encrypt2fileqcow2.argv @@ -1,7 +1,7 @@ qemu-img \ create \ -f qcow2 \ --o compat=0.10 \ +-o compat=1.1 \ /var/lib/libvirt/images/sparse-qcow2.img \ 1073741824K qemu-img \ diff --git a/tests/storagevolxml2argvdata/qcow2-compat.argv b/tests/storagevolxml2argvdata/qcow2-compat.argv index bf3a50a7f3..40fbe065e0 100644 --- a/tests/storagevolxml2argvdata/qcow2-compat.argv +++ b/tests/storagevolxml2argvdata/qcow2-compat.argv @@ -2,6 +2,6 @@ qemu-img \ create \ -f qcow2 \ -b /dev/null \ --o backing_fmt=raw,compat=0.10 \ +-o backing_fmt=raw,compat=1.1 \ /var/lib/libvirt/images/OtherDemo.img \ 5242880K diff --git a/tests/storagevolxml2argvdata/qcow2-from-logical-compat.argv b/tests/storagevolxml2argvdata/qcow2-from-logical-compat.argv index dbc7deb16a..b68da425d9 100644 --- a/tests/storagevolxml2argvdata/qcow2-from-logical-compat.argv +++ b/tests/storagevolxml2argvdata/qcow2-from-logical-compat.argv @@ -2,6 +2,6 @@ qemu-img \ convert \ -f raw \ -O qcow2 \ --o compat=0.10 \ +-o compat=1.1 \ /dev/HostVG/Swap \ /var/lib/libvirt/images/OtherDemo.img diff --git a/tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt.argv b/tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt.argv index d89622d4a6..3068b4b38d 100644 --- a/tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt.argv +++ b/tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt.argv @@ -2,7 +2,7 @@ qemu-img \ create \ -f qcow2 \ --object secret,id=OtherDemoLuks.img_encrypt0,file=/path/to/secretFile \ --o encrypt.format=luks,encrypt.key-secret=OtherDemoLuks.img_encrypt0,compat=0.10 \ +-o encrypt.format=luks,encrypt.key-secret=OtherDemoLuks.img_encrypt0,compat=1.1 \ /var/lib/libvirt/images/OtherDemoLuks.img \ 5242880K qemu-img \ diff --git a/tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt2fileqcow2.argv b/tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt2fileqcow2.argv index 4d910552d0..948e9ac66d 100644 --- a/tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt2fileqcow2.argv +++ b/tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt2fileqcow2.argv @@ -1,7 +1,7 @@ qemu-img \ create \ -f qcow2 \ --o compat=0.10 \ +-o compat=1.1 \ /var/lib/libvirt/images/sparse-qcow2.img \ 1073741824K qemu-img \ diff --git a/tests/storagevolxml2argvdata/qcow2-luks.argv b/tests/storagevolxml2argvdata/qcow2-luks.argv index 308316c90c..a3be41a406 100644 --- a/tests/storagevolxml2argvdata/qcow2-luks.argv +++ b/tests/storagevolxml2argvdata/qcow2-luks.argv @@ -3,6 +3,6 @@ create \ -f qcow2 \ -b /dev/null \ --object secret,id=OtherDemoLuks.img_encrypt0,file=/path/to/secretFile \ --o backing_fmt=raw,encrypt.format=luks,encrypt.key-secret=OtherDemoLuks.img_encrypt0,compat=0.10 \ +-o backing_fmt=raw,encrypt.format=luks,encrypt.key-secret=OtherDemoLuks.img_encrypt0,compat=1.1 \ /var/lib/libvirt/images/OtherDemoLuks.img \ 5242880K diff --git a/tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc-compat.argv b/tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc-compat.argv index 463ae26779..a130ed8894 100644 --- a/tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc-compat.argv +++ b/tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc-compat.argv @@ -2,6 +2,6 @@ qemu-img \ convert \ -f raw \ -O qcow2 \ --o preallocation=metadata,compat=0.10 \ +-o preallocation=metadata,compat=1.1 \ /var/lib/libvirt/images/sparse.img \ /var/lib/libvirt/images/OtherDemo.img diff --git a/tests/storagevolxml2argvdata/qcow2-nobacking-prealloc-compat.argv b/tests/storagevolxml2argvdata/qcow2-nobacking-prealloc-compat.argv index 510e0c13f6..440ad8f122 100644 --- a/tests/storagevolxml2argvdata/qcow2-nobacking-prealloc-compat.argv +++ b/tests/storagevolxml2argvdata/qcow2-nobacking-prealloc-compat.argv @@ -1,6 +1,6 @@ qemu-img \ create \ -f qcow2 \ --o preallocation=metadata,compat=0.10 \ +-o preallocation=metadata,compat=1.1 \ /var/lib/libvirt/images/OtherDemo.img \ 5242880K diff --git a/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv b/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv index 0152b1efb6..3bf8613d72 100644 --- a/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv +++ b/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv @@ -2,6 +2,6 @@ qemu-img \ convert \ -f raw \ -O qcow2 \ --o preallocation=falloc,compat=0.10 \ +-o preallocation=falloc,compat=1.1 \ /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 047932a559..924c5c6084 100644 --- a/tests/storagevolxml2argvdata/qcow2-nocapacity.argv +++ b/tests/storagevolxml2argvdata/qcow2-nocapacity.argv @@ -2,5 +2,5 @@ qemu-img \ create \ -f qcow2 \ -b /dev/null \ --o backing_fmt=raw,compat=0.10 \ +-o backing_fmt=raw,compat=1.1 \ /var/lib/libvirt/images/OtherDemo.img diff --git a/tests/storagevolxml2argvdata/qcow2-nocow-compat.argv b/tests/storagevolxml2argvdata/qcow2-nocow-compat.argv index 4cc7904cfc..ae94e4e588 100644 --- a/tests/storagevolxml2argvdata/qcow2-nocow-compat.argv +++ b/tests/storagevolxml2argvdata/qcow2-nocow-compat.argv @@ -2,6 +2,6 @@ qemu-img \ create \ -f qcow2 \ -b /dev/null \ --o backing_fmt=raw,nocow=on,compat=0.10 \ +-o backing_fmt=raw,nocow=on,compat=1.1 \ /var/lib/libvirt/images/OtherDemo.img \ 5242880K diff --git a/tests/storagevolxml2argvdata/qcow2-zerocapacity.argv b/tests/storagevolxml2argvdata/qcow2-zerocapacity.argv index 607c642e6f..05e31509cb 100644 --- a/tests/storagevolxml2argvdata/qcow2-zerocapacity.argv +++ b/tests/storagevolxml2argvdata/qcow2-zerocapacity.argv @@ -1,6 +1,6 @@ qemu-img \ create \ -f qcow2 \ --o compat=0.10 \ +-o compat=1.1 \ /var/lib/libvirt/images/OtherDemo.img \ 0K -- 2.42.1
Apparently, if you change the commit message, patchew doesn't pick up v2 and mark the v1 as old (O). Sorry for the inconvenience if there is a patch, with a changed commit message [1], please ignore it and consider the present one. [1] https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/CETEZ...
On Wed, Mar 06, 2024 at 01:20:13 +0530, Abhiram Tilak wrote:
Change the default to modern qcow2 as it's supported by all qemu versions supported by libvirt and in fact 'qemu-img' already defaults to the new format for a long time.
Some Unittests require changes to pass, now that version 1.1 is default. Unittests like `qcow2-1.1.argv` may not be relevant anymore, but this patch doesn't affect them.
I've added a: Closes: https://gitlab.com/libvirt/libvirt/-/issues/602
Signed-off-by: Abhiram Tilak <atp.exp@gmail.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> ---
And pushed the patch; thanks for fixing this.
On Tue, Mar 12, 2024 at 10:36:28AM +0100, Peter Krempa wrote:
On Wed, Mar 06, 2024 at 01:20:13 +0530, Abhiram Tilak wrote:
Change the default to modern qcow2 as it's supported by all qemu versions supported by libvirt and in fact 'qemu-img' already defaults to the new format for a long time.
Some Unittests require changes to pass, now that version 1.1 is default. Unittests like `qcow2-1.1.argv` may not be relevant anymore, but this patch doesn't affect them.
I've added a:
Closes: https://gitlab.com/libvirt/libvirt/-/issues/602
Signed-off-by: Abhiram Tilak <atp.exp@gmail.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> ---
And pushed the patch; thanks for fixing this.
...but this is a behavioural change of our API. Images created by applications will now have reduced portability across platforms, since v3 is supported by a subset of platforms that support v2, which I would call a regression. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, Mar 12, 2024 at 10:01:25 +0000, Daniel P. Berrangé wrote:
On Tue, Mar 12, 2024 at 10:36:28AM +0100, Peter Krempa wrote:
On Wed, Mar 06, 2024 at 01:20:13 +0530, Abhiram Tilak wrote:
Change the default to modern qcow2 as it's supported by all qemu versions supported by libvirt and in fact 'qemu-img' already defaults to the new format for a long time.
Some Unittests require changes to pass, now that version 1.1 is default. Unittests like `qcow2-1.1.argv` may not be relevant anymore, but this patch doesn't affect them.
I've added a:
Closes: https://gitlab.com/libvirt/libvirt/-/issues/602
Signed-off-by: Abhiram Tilak <atp.exp@gmail.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> ---
And pushed the patch; thanks for fixing this.
...but this is a behavioural change of our API. Images created by applications will now have reduced portability across platforms, since v3 is supported by a subset of platforms that support v2, which I would call a regression.
I remember you making this argument the last time this was discussed, but this was so long ago that libvirt already stopped supporting any qemu that would not support v3 qcow2 images. Now obviously the counter argument may be that there may be other software that implemented only v2 support and thus would break, but I'd consider this to be a very minor corner case. The same goes for the argument that "anyone who cares about a specific version should use it explicitly" which goes both ways. I try to take the compatibility promises we give to users very seriously but depending on a default that was replaced even in qemu more than 10 years ago seems to be a bit extreme.
On Tue, Mar 12, 2024 at 11:09:47AM +0100, Peter Krempa wrote:
On Tue, Mar 12, 2024 at 10:01:25 +0000, Daniel P. Berrangé wrote:
On Tue, Mar 12, 2024 at 10:36:28AM +0100, Peter Krempa wrote:
On Wed, Mar 06, 2024 at 01:20:13 +0530, Abhiram Tilak wrote:
Change the default to modern qcow2 as it's supported by all qemu versions supported by libvirt and in fact 'qemu-img' already defaults to the new format for a long time.
Some Unittests require changes to pass, now that version 1.1 is default. Unittests like `qcow2-1.1.argv` may not be relevant anymore, but this patch doesn't affect them.
I've added a:
Closes: https://gitlab.com/libvirt/libvirt/-/issues/602
Signed-off-by: Abhiram Tilak <atp.exp@gmail.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> ---
And pushed the patch; thanks for fixing this.
...but this is a behavioural change of our API. Images created by applications will now have reduced portability across platforms, since v3 is supported by a subset of platforms that support v2, which I would call a regression.
I remember you making this argument the last time this was discussed, but this was so long ago that libvirt already stopped supporting any qemu that would not support v3 qcow2 images.
Disk images are special though, because the compatibility is not self contained to a single deployment. Vendors/users can be building disk images with tools on new platform, but deploying them on existing old platforms. So whether today's libvirt supports such QEMU versions is not the determining factor.
Now obviously the counter argument may be that there may be other software that implemented only v2 support and thus would break, but I'd consider this to be a very minor corner case.
The same goes for the argument that "anyone who cares about a specific version should use it explicitly" which goes both ways.
The problem is that's not what we have documented as the intended semantics for omitting the version: "Specify compatibility level. So far, this is only used for type='qcow2' volumes. Valid values are 0.10 and 1.1 so far, specifying QEMU version the images should be compatible with. If the feature element is present, 1.1 is used (Since 1.1.0) If omitted, 0.10 is used (Since 1.1.2)" so said it defaults to 0.10 unless explicitly told otherwise. This was/is an unfortunate choice. We should have defined and documented omission as being "platform defined version". We would still have had to leave it on 0.10 at that time to avoid major regression, but we would have given ourselves wiggle room to change it at a later date.
I try to take the compatibility promises we give to users very seriously but depending on a default that was replaced even in qemu more than 10 years ago seems to be a bit extreme.
We've never had a need for a /etc/libvirt/storage.conf configuration file for the storage driver, but perhaps the qcow2 default version is a use case for it. IOW, change the docs to say it omissions means platform defined, allow an override in the config file, and set default to 1.1. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (4)
-
Abhiram Tilak -
atp exp
-
Daniel P. Berrangé -
Peter Krempa