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
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org

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.
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org

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 :|
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org

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.
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org

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 :|
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org

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/CETEZZHEQ2BT73ICKOUFN26GM2NCYBO4/
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org