Merge ~rafaeldtinoco/ubuntu/+source/libvirt:focal-clustersize into ubuntu/+source/libvirt:ubuntu/focal-devel

[Rafael David Tinoco]

[Rafael David Tinoco (rafaeldtinoco)]

Alright, this is a set of patches to be applied on top of libvirt 6.0.0-0ubuntu8.9 (focal). It can also be applied to other branches based on focal's version.

@lucas,

(1)

The xml stanza clusterSize is being added to libvirt so the qcow2 image file can be created with that parameter. A quick way to achieve the same without libvirt is:

# qemu-img create -f qcow2 -o cluster_size=2M /var/lib/libvirt/images/guest-disk01.qcow2 5G

and verify it has "cluster_size: 2097152" with

# qemu-img info /var/lib/libvirt/images/guest-disk01.qcow2

(2)

Libvirt NEEDS the qcow2 volume to be created, and managed, through it in order to test this feature. This means that you can create a managed volume by having the following XML:

$ cat ./guest-disk01.xml

<volume type='file'>
  <name>guest-disk01.qcow2</name>
  <key>/var/lib/libvirt/images/guest-disk01.qcow2</key>
  <source>
  </source>
  <capacity unit='bytes'>5368709120</capacity>
  <allocation unit='bytes'>6295552</allocation>
  <physical unit='bytes'>6291464</physical>
  <target>
    <path>/var/lib/libvirt/images/guest-disk01.qcow2</path>
    <format type='qcow2'/>
    <permissions>
      <mode>0644</mode>
      <owner>0</owner>
      <group>0</group>
    </permissions>
    <timestamps>
      <atime>1621917565.209401319</atime>
      <mtime>1621917565.261402011</mtime>
      <ctime>1621917565.261402011</ctime>
    </timestamps>
    <compat>1.1</compat>
    <clusterSize unit='B'>2097152</clusterSize>
    <features/>
  </target>
</volume>

for example, and define the volume with it:

$ virsh vol-create --pool default --file ./guest-disk01.xml
Vol guest-disk01.qcow2 created from ./guest-disk01.xml

and, in order to make sure the feature worked, verify by hand if the cluster_size was set in the managed qcow2 file:

$ sudo qemu-img info /var/lib/libvirt/images/guest-disk01.qcow2
image: /var/lib/libvirt/images/guest-disk01.qcow2
file format: qcow2
virtual size: 5 GiB (5368709120 bytes)
disk size: 6 MiB
cluster_size: 2097152
Format specific information:
    compat: 1.1
    lazy refcounts: false
    refcount bits: 16
    corrupt: false

[Rafael David Tinoco (rafaeldtinoco)]

Regarding the patchset:

* Add support for QCOW2 cluster_size option:
  + d/p/clustersize/0001-qemu-monitor-Detect-image-cluster-size-from-query-na.patch
  + d/p/clustersize/0002-qemu-block-Allow-specifying-cluster-size-when-using-.patch
  + d/p/clustersize/0003-qemuBlockStorageSourceCreateDetectSize-Propagate-clu.patch
  + d/p/clustersize/0004-conf-Move-and-rename-virDomainParseScaledValue.patch
  + d/p/clustersize/0005-storage-add-support-for-QCOW2-cluster_size-option.patch
  + d/p/clustersize/0006-storage_file-add-support-to-probe-cluster_size-from-.patch

You will follow clean cherry-picks but 2 of them, which I classified as 'backports'. Explanation on why they are backports was added to their git logs. The *only* (perhaps) discussion that could be raised from this set *would* be whether we should have:

  + d/p/clustersize/0004-conf-Move-and-rename-virDomainParseScaledValue.patch

cherry-picked together. It mainly relocates a function from one place to another. I thought it was easier to have it in the set so we don't have to declare the function prototype in the header for the subsequent patch to work (adding a delta that does not exist upstream).

Let me know if you have any doubs, and feel free to merge this patchset to any other repository you feel like ;).

-rafaeldtinoco

[Rafael David Tinoco (rafaeldtinoco)]

BTW, forgot to mention, ignore the last commit as it has a delta I added so I could better emulate other environment. Let me know if you want me to have it removed or you are good in doing it by yourself during review time. Thanks.

[Lucas Kanashiro (lucaskanashiro)]

Thanks for the MP and the detailed steps on how to test this feature.

I tried to build the package from your branch locally in Focal to performed the test you described, but it FTBFS for me. Is it working for you? The following test is failing during the build:

FAIL: storagevolxml2xmltest
===========================

TEST: storagevolxml2xmltest
 1) Storage Vol XML-2-XML vol-file ... OK
 2) Storage Vol XML-2-XML vol-file-naming ... OK
 3) Storage Vol XML-2-XML vol-file-backing ... OK
 4) Storage Vol XML-2-XML vol-file-iso ... OK
 5) Storage Vol XML-2-XML vol-qcow2 ... OK
 6) Storage Vol XML-2-XML vol-qcow2-1.1 ... OK
 7) Storage Vol XML-2-XML vol-qcow2-lazy ... OK
 8) Storage Vol XML-2-XML vol-qcow2-0.10-lazy ... OK
 9) Storage Vol XML-2-XML vol-qcow2-nobacking ... OK
10) Storage Vol XML-2-XML vol-qcow2-encryption ... OK
11) Storage Vol XML-2-XML vol-qcow2-clusterSize ...
In '/<<PKGBUILDDIR>>/debian/build/../../tests/storagevolxml2xmlout/vol-qcow2-clusterSize.xml':
Offset 104
Expect [c]
Actual [source>
  </source>
  <c]
                                                                      ... FAILED
12) Storage Vol XML-2-XML vol-luks ... OK
13) Storage Vol XML-2-XML vol-luks-cipher ... OK
14) Storage Vol XML-2-XML vol-partition ... OK
15) Storage Vol XML-2-XML vol-logical ... OK
16) Storage Vol XML-2-XML vol-logical-backing ... OK
17) Storage Vol XML-2-XML vol-sheepdog ... OK
18) Storage Vol XML-2-XML vol-gluster-dir ... OK
19) Storage Vol XML-2-XML vol-gluster-dir-neg-uid ... OK
20) Storage Vol XML-2-XML vol-qcow2-nocapacity ... OK
FAIL storagevolxml2xmltest (exit status: 1)

Apart from that, I have some questions for you:

1) In the diff there are some changes introduced in d/control and d/rules not mentioned in the changelog, those changes were added in the "IBM layer - Ignore" commit (bc66b07c). Don't we want to advertise those things in the changelog? This is minor, I can add the existent IBM delta to the changelog when putting it in the virt-stack-ibmcloud git repository.

2) You prepared this package based on Focal but in the IBM proposed PPA we have packages targeting Bionic only. Did you try to build this version in Bionic?

3) The version we have in the proposed PPA is 6.0.0-0ubuntu8.8~ibmcloud0.6. Are you sure IBM wants to move to 6.0.0-0ubuntu8.9? This is the changelog entry for -0ubuntu8.9:

libvirt (6.0.0-0ubuntu8.9) focal; urgency=medium

  * d/p/u/lp-1921754*: add EPYC-Rome-v2 as v1 missed IBRS and thereby fails
    on some HW/Guest combinations e.g. Windows 10 on Threadripper
    (LP: #1921754)
  * d/p/u/lp-1921880*: add EPYC-Milan features and named cpu type support
    (LP...

review: Needs Fixing

[Rafael David Tinoco (rafaeldtinoco)]

1) In the diff there are some changes introduced in d/control and d/rules not mentioned in the changelog, those changes were added in the "IBM layer - Ignore" commit (bc66b07c). Don't we want to advertise those things in the changelog? This is minor, I can add the existent IBM delta to the changelog when putting it in the virt-stack-ibmcloud git repository.

> This is an external layer (already included in another internal repo for other builds). I added as a last commit so the build would be the same as the one we get from internal repos.

2) You prepared this package based on Focal but in the IBM proposed PPA we have packages targeting Bionic only. Did you try to build this version in Bionic?

> That is why we have the "IBM layer - Ignore" commit. It makes the focal-dev branch compatible to Bionic (as a backport). So this repo will build in Bionic and, if you remove the first commit, it will build in Focal as well.

3) The version we have in the proposed PPA is 6.0.0-0ubuntu8.8~ibmcloud0.6. Are you sure IBM wants to move to 6.0.0-0ubuntu8.9? This is the changelog entry for -0ubuntu8.9:

libvirt (6.0.0-0ubuntu8.9) focal; urgency=medium

> Version present in the 'build' PPA is 6.0.0-0ubuntu8.9~ibmcloud0.1. The correct version for a package to be put there is the next version: 6.0.0-0ubuntu8.9~ibmcloud0.2. I have not followed correct versioning for that PPA, I was searching for patch review, not package logical one, for now.

4) Considering we want to import 6.0.0-0ubuntu8.9 and apply those patches on top of it, do we really want to use your version string 6.0.0-0ubuntu8.9+20210524~clustersize01? Or follow the pattern and use 6.0.0-0ubuntu8.9~ibmcloud0.1?

> NO =), I used it locally only.

The patch set looks sane to me.

> Thanks! I'll check the tests failure and reply here.

[Rafael David Tinoco (rafaeldtinoco)]

make[3]: Entering directory '/home/rafaeldtinoco/work/sources/ubuntu/libvirt/build/tests'
PASS: virshtest
PASS: sockettest
PASS: virhostcputest
PASS: virbuftest
PASS: commandtest
PASS: seclabeltest
PASS: virhashtest
PASS: virconftest
PASS: viratomictest
PASS: utiltest
PASS: shunloadtest
PASS: virtimetest
PASS: viruritest
PASS: virkeyfiletest
PASS: viralloctest
PASS: virauthconfigtest
PASS: virbitmaptest
SKIP: vircgrouptest
PASS: vircryptotest
PASS: virpcitest
PASS: virendiantest
PASS: virfiletest
PASS: virfilecachetest
PASS: virfirewalltest
PASS: viriscsitest
PASS: virkeycodetest
PASS: virlockspacetest
PASS: virlogtest
PASS: virrotatingfiletest
PASS: virschematest
PASS: virstringtest
PASS: virportallocatortest
PASS: sysinfotest
PASS: virkmodtest
PASS: vircapstest
PASS: domaincapstest
PASS: domainconftest
PASS: virhostdevtest
PASS: virnetdevtest
PASS: virtypedparamtest
PASS: vshtabletest
PASS: virerrortest
PASS: virnetmessagetest
PASS: virnetsockettest
PASS: virnetdaemontest
PASS: virnetserverclienttest
PASS: virnettlscontexttest
PASS: virnettlssessiontest
PASS: fchosttest
PASS: scsihosttest
PASS: vircaps2xmltest
PASS: virresctrltest
PASS: fdstreamtest
PASS: virdbustest
PASS: virsystemdtest
PASS: virpolkittest
PASS: qemuxml2argvtest
PASS: qemuxml2xmltest
PASS: qemudomaincheckpointxml2xmltest
PASS: qemudomainsnapshotxml2xmltest
PASS: qemumonitorjsontest
PASS: qemuhotplugtest
PASS: qemuagenttest
PASS: qemucapabilitiestest
PASS: qemucaps2xmltest
PASS: qemumemlocktest
PASS: qemucommandutiltest
PASS: qemublocktest
PASS: qemumigparamstest
PASS: qemusecuritytest
PASS: qemufirmwaretest
PASS: qemuvhostusertest
PASS: openvzutilstest
PASS: esxutilstest
PASS: vmx2xmltest
PASS: xml2vmxtest
PASS: vmwarevertest
PASS: virjsontest
PASS: networkxml2xmlupdatetest
PASS: virnetworkportxml2xmltest
PASS: nwfilterxml2xmltest
PASS: virnwfilterbindingxml2xmltest
PASS: nwfilterebiptablestest
PASS: nwfilterxml2firewalltest
PASS: storagevolxml2argvtest
PASS: storagepoolxml2argvtest
PASS: virstorageutiltest
PASS: storagepoolxml2xmltest
PASS: storagepoolcapstest
PASS: virstoragetest
PASS: virscsitest
FAIL: storagevolxml2xmltest
PASS: nodedevxml2xmltest
PASS: interfacexml2xmltest
PASS: cputest
PASS: metadatatest
PASS: secretxml2xmltest
PASS: genericxml2xmltest
PASS: virusbtest
PASS: virnetdevbandwidthtest
PASS: eventtest
PASS: virdrivermoduletest
PASS: virdriverconnvalidatetest
PASS: objecteventtest
PASS: virmacmaptest
PASS: virnetdevopenvswitchtest
PASS: libvirtd-fail
PASS: libvirtd-pool
PASS: virsh-cpuset
PASS: virsh-define-dev-segfault
PASS: virsh-int-overflow
SKIP: virsh-optparse
PASS: virsh-read-bufsiz
PASS: virsh-read-non-seekable
PASS: virsh-schedinfo
PASS: virsh-self-test
PASS: virt-admin-self-test
PASS: virsh-checkpoint
PASS: virsh-snapshot
PASS: virsh-start
PASS: virsh-undefine
PASS: virsh-uriprecedence
PASS: virsh-vcpupin
============================================================================
Testsuite summary for libvirt 6.0.0
============================================================================
# TOTAL: 123
# PASS: 120
# SKIP: 2
# XFAIL: 0
# FAIL: 1
# XPASS: 0
# ERROR: 0

FAILED TEST:

FAIL: storagevolxml2xmltest, working on it.

[Rafael David Tinoco (rafaeldtinoco)]

Alright @lucas,

Now tests are good:

PASS: virshtest
PASS: sockettest
PASS: virhostcputest
PASS: virbuftest
PASS: commandtest
PASS: seclabeltest
PASS: virhashtest
PASS: virconftest
PASS: viratomictest
PASS: utiltest
PASS: shunloadtest
PASS: virtimetest
PASS: viruritest
PASS: virkeyfiletest
PASS: viralloctest
PASS: virauthconfigtest
PASS: virbitmaptest
PASS: vircgrouptest
PASS: vircryptotest
PASS: virpcitest
PASS: virendiantest
PASS: virfiletest
PASS: virfilecachetest
PASS: virfirewalltest
PASS: viriscsitest
PASS: virkeycodetest
PASS: virlockspacetest
PASS: virlogtest
PASS: virrotatingfiletest
PASS: virschematest
PASS: virstringtest
PASS: virportallocatortest
PASS: sysinfotest
PASS: virkmodtest
PASS: vircapstest
PASS: domaincapstest
PASS: domainconftest
PASS: virhostdevtest
PASS: virnetdevtest
PASS: virtypedparamtest
PASS: vshtabletest
PASS: virerrortest
PASS: virnetmessagetest
PASS: virnetsockettest
PASS: virnetdaemontest
PASS: virnetserverclienttest
PASS: virnettlscontexttest
PASS: virnettlssessiontest
PASS: fchosttest
PASS: scsihosttest
PASS: vircaps2xmltest
PASS: virresctrltest
PASS: fdstreamtest
PASS: virdbustest
PASS: virsystemdtest
PASS: virpolkittest
PASS: qemuxml2argvtest
PASS: qemuxml2xmltest
PASS: qemudomaincheckpointxml2xmltest
PASS: qemudomainsnapshotxml2xmltest
PASS: qemumonitorjsontest
PASS: qemuhotplugtest
PASS: qemuagenttest
PASS: qemucapabilitiestest
PASS: qemucaps2xmltest
PASS: qemumemlocktest
PASS: qemucommandutiltest
PASS: qemublocktest
PASS: qemumigparamstest
PASS: qemusecuritytest
PASS: qemufirmwaretest
PASS: qemuvhostusertest
PASS: openvzutilstest
PASS: esxutilstest
PASS: vmx2xmltest
PASS: xml2vmxtest
PASS: vmwarevertest
PASS: virjsontest
PASS: networkxml2xmlupdatetest
PASS: virnetworkportxml2xmltest
PASS: nwfilterxml2xmltest
PASS: virnwfilterbindingxml2xmltest
PASS: nwfilterebiptablestest
PASS: nwfilterxml2firewalltest
PASS: storagevolxml2argvtest
PASS: storagepoolxml2argvtest
PASS: virstorageutiltest
PASS: storagepoolxml2xmltest
PASS: storagepoolcapstest
PASS: virstoragetest
PASS: virscsitest
PASS: storagevolxml2xmltest
PASS: nodedevxml2xmltest
PASS: interfacexml2xmltest
PASS: cputest
PASS: metadatatest
PASS: secretxml2xmltest
PASS: genericxml2xmltest
PASS: virusbtest
PASS: virnetdevbandwidthtest
PASS: eventtest
PASS: virdrivermoduletest
PASS: virdriverconnvalidatetest
PASS: objecteventtest
PASS: virmacmaptest
PASS: virnetdevopenvswitchtest
PASS: libvirtd-fail
PASS: libvirtd-pool
PASS: virsh-cpuset
PASS: virsh-define-dev-segfault
PASS: virsh-int-overflow
SKIP: virsh-optparse
PASS: virsh-read-bufsiz
PASS: virsh-read-non-seekable
PASS: virsh-schedinfo
PASS: virsh-self-test
PASS: virt-admin-self-test
PASS: virsh-checkpoint
PASS: virsh-snapshot
PASS: virsh-start
PASS: virsh-undefine
PASS: virsh-uriprecedence
PASS: virsh-vcpupin
============================================================================
Testsuite summary for libvirt 6.0.0
============================================================================
# TOTAL: 123
# PASS: 122
# SKIP: 1
# XFAIL: 0
# FAIL: 0
# XPASS: 0
# ERROR: 0

Problem was an extra stanza added by default in the XML output of the created test. That happens because of the libvirt version ...

[Rafael David Tinoco (rafaeldtinoco)]

Good to go. Feel free to merge to 'other repos' if you feel like it.

Thanks a lot

review: Approve

[Lucas Kanashiro (lucaskanashiro)]

Rafael, I am still unable to build the package in Focal. The same test is still failing for me, however, the error is different now:

FAIL: storagevolxml2xmltest
===========================

TEST: storagevolxml2xmltest
 1) Storage Vol XML-2-XML vol-file ... OK
 2) Storage Vol XML-2-XML vol-file-naming ... OK
 3) Storage Vol XML-2-XML vol-file-backing ... OK
 4) Storage Vol XML-2-XML vol-file-iso ... OK
 5) Storage Vol XML-2-XML vol-qcow2 ... OK
 6) Storage Vol XML-2-XML vol-qcow2-1.1 ... OK
 7) Storage Vol XML-2-XML vol-qcow2-lazy ... OK
 8) Storage Vol XML-2-XML vol-qcow2-0.10-lazy ... OK
 9) Storage Vol XML-2-XML vol-qcow2-nobacking ... OK
10) Storage Vol XML-2-XML vol-qcow2-encryption ... OK
11) Storage Vol XML-2-XML vol-qcow2-clusterSize ...
In '/<<PKGBUILDDIR>>/debian/build/../../tests/storagevolxml2xmlout/vol-qcow2-clusterSize.xml':
Offset 524
Expect []
Actual [ </target>
</volume>
]
                                                                      ... FAILED
12) Storage Vol XML-2-XML vol-luks ... OK
13) Storage Vol XML-2-XML vol-luks-cipher ... OK
14) Storage Vol XML-2-XML vol-partition ... OK
15) Storage Vol XML-2-XML vol-logical ... OK
16) Storage Vol XML-2-XML vol-logical-backing ... OK
17) Storage Vol XML-2-XML vol-sheepdog ... OK
18) Storage Vol XML-2-XML vol-gluster-dir ... OK
19) Storage Vol XML-2-XML vol-gluster-dir-neg-uid ... OK
20) Storage Vol XML-2-XML vol-qcow2-nocapacity ... OK
FAIL storagevolxml2xmltest (exit status: 1)

Did you forget to git push any local change? I am wondering why this test is passing for you but failing for me.

review: Needs Fixing

[Rafael David Tinoco (rafaeldtinoco)]

The real problem is that a virsh vol-dumpxml is including a:

<source>
</source>

at the final XML, which causes no harm but the test compares with given output (brought by my set) and says its wrong. The backport is good and works.

This behaviour is fixed by:

commit 6423e30828
Author: Peter Krempa <email address hidden>
Date: Thu Feb 25 10:11:05 2021

    virStorageVolDefFormat: Don't format empty <source>

    If there are no source extents the volume XML has an empty <source>
    element. Remove it if there's nothing in it by using
    virXMLFormatElement.

    Signed-off-by: Peter Krempa <email address hidden>
    Reviewed-by: Michal Privoznik <email address hidden>

but if bringing this patch before my patchset ... other tests break and I dont want to go deep into why previous tests are being broken by this.

Previous attempt I tried to include <source></source> in the XML and it worked for my local builds, when test would compare the output of the vol-dumpxml sub-command it would find a local file exactly the same as expected.

I'm afraid I did not test with all other patches included, that is why its failing in dpkg-buildpackage. Being in hurry is enemy of perfection, as usually said.

I'm fixing this now and will make sure it works for the dpkg-buildpackage of both focal and bionic.

[Rafael David Tinoco (rafaeldtinoco)]

The focal PPA:

https://launchpad.net/~rafaeldtinoco/+archive/ubuntu/libvirtclustersize

with the contents of this MR.

The test with that PPA's packages:

$ virsh pool-create-as --name default --type dir --source-path /var/lib/libvirt/images --target /var/lib/libvirt/images
Pool default created

$ virsh vol-create --pool default --file ./guest01-disk01.xml
Vol guest-disk01.qcow2 created from ./guest01-disk01.xml

$ sudo qemu-img info /var/lib/libvirt/images/guest-disk01.qcow2
image: /var/lib/libvirt/images/guest-disk01.qcow2
file format: qcow2
virtual size: 5 GiB (5368709120 bytes)
disk size: 6.13 MiB
cluster_size: 2097152
Format specific information:
    compat: 1.1
    lazy refcounts: false
    refcount bits: 16
    corrupt: false

$ virsh vol-dumpxml --pool default --vol guest-disk01.qcow2
<volume type='file'>
  <name>guest-disk01.qcow2</name>
  <key>/var/lib/libvirt/images/guest-disk01.qcow2</key>
  <source>
  </source>
  <capacity unit='bytes'>5368709120</capacity>
  <allocation unit='bytes'>6425088</allocation>
  <physical unit='bytes'>6291464</physical>
  <target>
    <path>/var/lib/libvirt/images/guest-disk01.qcow2</path>
    <format type='qcow2'/>
    <permissions>
      <mode>0644</mode>
      <owner>0</owner>
      <group>0</group>
    </permissions>
    <timestamps>
      <atime>1622086325.115175639</atime>
      <mtime>1622086325.147176059</mtime>
      <ctime>1622086325.147176059</ctime>
    </timestamps>
    <compat>1.1</compat>
    <clusterSize unit='B'>2097152</clusterSize>
    <features/>
  </target>
</volume>

[Rafael David Tinoco (rafaeldtinoco)]

If you check that PPA's build log:

...
PASS: storagepoolcapstest
PASS: storagepoolxml2xmltest
PASS: virscsitest
PASS: nsstest
PASS: nssguesttest
PASS: storagevolxml2xmltest
PASS: nodedevxml2xmltest
PASS: interfacexml2xmltest
PASS: metadatatest
PASS: secretxml2xmltest
PASS: virstoragetest
PASS: virusbtest
PASS: genericxml2xmltest
...

So the test storagevolxml2xmltest has passed.

[Rafael David Tinoco (rafaeldtinoco)]

Indeed there is something strange with the focal's libvirt tests, like the one commit fixing empty XML blocks. I don't think we should worry with this as the build works, the expected results, out of my patchset, are good and all seems fine. We can leave that to @paelzer when he is back IF he wants to pursue that, IMO.

[Rafael David Tinoco (rafaeldtinoco)]

Just pushed my latest branch, the one I used to build the PPA.

[Rafael David Tinoco (rafaeldtinoco)]

Okay, now, for the Bionic backport you can grab these 2 patches:

https://paste.ubuntu.com/p/JgBmvMB4tq/

and

https://paste.ubuntu.com/p/H6tBGfcz4T/

on top of the source package directly from the private PPA (with all delta it has).

You will have d/p/ibm/:

$ cat 0001-Add-support-for-QCOW2-clusterSize-option.patch | diffstat -l
ibm/clusz-conf-Move-and-rename-virDomainParseScaledValue.patch
ibm/clusz-storage-add-support-for-QCOW2-cluster_size-option.patch
ibm/clusz-storage_file-add-support-to-probe-cluster_size-from.patch
series

which are the 3 needed patches (as the other 3 patches are already included).

With this, you will be able to build the same binary private PPA has with these extra feature.

Ah, the tests for the Bionic backport (with all other patches) have worked and should work in the **build PPA** as well.

[Lucas Kanashiro (lucaskanashiro)]

Thanks for the analysis and the fix Rafael. Just to make sure everything is all right I built it locally successfully this time, and also followed the steps you described above to test the feature. All looks good!

I'll start to move those patches to the virt-stack-ibmcloud git repository and soon after upload it to the IBM build PPA.

review: Approve

[Rafael David Tinoco (rafaeldtinoco)]

Thanks Lucas!

[Christian Ehrhardt  (paelzer)]

Thank you!
LGTM as well

I had to smile about "Being in hurry is enemy of perfection, as usually said.". I sometimes refer to "Perfection is the enemy of progress". Now I wondered if we can derive from that "Being in a Hurry == Progress". Unless this isn't a bijective relation ... anyway :-)

review: Approve

Unmerged commits

e59a822... by Rafael David Tinoco on 2021-05-25

changelog

c83fec2... by Rafael David Tinoco on 2021-05-25

* Add support for QCOW2 cluster_size option:
  + d/p/clustersize/0001-qemu-monitor-Detect-image-cluster-size-from-query-na.patch
  + d/p/clustersize/0002-qemu-block-Allow-specifying-cluster-size-when-using-.patch
  + d/p/clustersize/0003-qemuBlockStorageSourceCreateDetectSize-Propagate-clu.patch
  + d/p/clustersize/0004-conf-Move-and-rename-virDomainParseScaledValue.patch
  + d/p/clustersize/0005-storage-add-support-for-QCOW2-cluster_size-option.patch
  + d/p/clustersize/0006-storage_file-add-support-to-probe-cluster_size-from-.patch

diff --git a/debian/changelog b/debian/changelog
index fa7efba..c883c1e 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,15 @@
+libvirt (6.0.0-0ubuntu8.9+hf20210524~01) focal; urgency=medium
+
+  * Add support for QCOW2 cluster_size option:
+    + d/p/clustersize/0001-qemu-monitor-Detect-image-cluster-size-from-query-na.patch
+    + d/p/clustersize/0002-qemu-block-Allow-specifying-cluster-size-when-using-.patch
+    + d/p/clustersize/0003-qemuBlockStorageSourceCreateDetectSize-Propagate-clu.patch
+    + d/p/clustersize/0004-conf-Move-and-rename-virDomainParseScaledValue.patch
+    + d/p/clustersize/0005-storage-add-support-for-QCOW2-cluster_size-option.patch
+    + d/p/clustersize/0006-storage_file-add-support-to-probe-cluster_size-from-.patch
+
+ -- Rafael David Tinoco <[e-mail]>  Tue, 25 May 2021 02:52:28 +0000
+
 libvirt (6.0.0-0ubuntu8.9) focal; urgency=medium
 
   * d/p/u/lp-1921754*: add EPYC-Rome-v2 as v1 missed IBRS and thereby fails
diff --git a/debian/patches/clustersize/0001-qemu-monitor-Detect-image-cluster-size-from-query-na.patch b/debian/patches/clustersize/0001-qemu-monitor-Detect-image-cluster-size-from-query-na.patch
new file mode 100644
index 0000000..67df248
--- /dev/null
+++ b/debian/patches/clustersize/0001-qemu-monitor-Detect-image-cluster-size-from-query-na.patch
@@ -0,0 +1,53 @@
+From fd49364d8b3e30db1891685ec8a7886ba05f2476 Mon Sep 17 00:00:00 2001
+Description: qemu: monitor: Detect image cluster size from 'query-named-block-nodes'
+Author: Peter Krempa <[e-mail]>
+Origin: upstream, https://github.com/libvirt/libvirt/commit/fd49364d8b3e30db1891685ec8a7886ba05f2476
+Bug: https://gitlab.com/libvirt/libvirt/-/issues/154
+Reviewed-by: Rafael David Tinoco <[e-mail]>
+Last-Update: 2021-05-24
+
+qemu: monitor: Detect image cluster size from 'query-named-block-nodes'
+
+Configuring the cluster size of an image may have performance
+implications. This patch allows us to detect cluster size for existing
+images so that we will be able to propagate it to new images which are
+based on existing images e.g. during snapshots/block-copy/etc.
+
+Signed-off-by: Peter Krempa <[e-mail]>
+Reviewed-by: Eric Blake <[e-mail]>
+---
+ src/qemu/qemu_monitor.h      | 3 +++
+ src/qemu/qemu_monitor_json.c | 3 +++
+ 2 files changed, 6 insertions(+)
+
+diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
+index 3f3b81cddd..d4ce0b4a05 100644
+--- a/src/qemu/qemu_monitor.h
++++ b/src/qemu/qemu_monitor.h
+@@ -697,6 +697,9 @@ struct _qemuBlockNamedNodeData {
+ 
+     qemuBlockNamedNodeDataBitmapPtr *bitmaps;
+     size_t nbitmaps;
++
++    /* the cluster size of the image is valid only when > 0 */
++    unsigned long long clusterSize;
+ };
+ 
+ virHashTablePtr
+diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
+index e5164d218a..fcbe94a5f1 100644
+--- a/src/qemu/qemu_monitor_json.c
++++ b/src/qemu/qemu_monitor_json.c
+@@ -2970,6 +2970,9 @@ qemuMonitorJSONBlockGetNamedNodeDataWorker(size_t pos G_GNUC_UNUSED,
+     if (virJSONValueObjectGetNumberUlong(img, "actual-size", &ent->physical) < 0)
+         ent->physical = ent->capacity;
+ 
++    /* try looking up the cluster size */
++    ignore_value(virJSONValueObjectGetNumberUlong(img, "cluster-size", &ent->clusterSize));
++
+     if ((bitmaps = virJSONValueObjectGetArray(val, "dirty-bitmaps")))
+         qemuMonitorJSONBlockGetNamedNodeDataBitmaps(bitmaps, ent);
+ 
+-- 
+2.25.1
+
diff --git a/debian/patches/clustersize/0002-qemu-block-Allow-specifying-cluster-size-when-using-.patch b/debian/patches/clustersize/0002-qemu-block-Allow-specifying-cluster-size-when-using-.patch
new file mode 100644
index 0000000..b964de4
--- /dev/null
+++ b/debian/patches/clustersize/0002-qemu-block-Allow-specifying-cluster-size-when-using-.patch
@@ -0,0 +1,47 @@
+From e60620e28b63378bb215bfd61b525a60b6660983 Mon Sep 17 00:00:00 2001
+Description: qemu: block: Allow specifying cluster size when using 'blockdev-create'
+Author: Peter Krempa <[e-mail]>
+Origin: upstream, https://github.com/libvirt/libvirt/commit/e60620e28b63378bb215bfd61b525a60b6660983
+Bug: https://gitlab.com/libvirt/libvirt/-/issues/154
+Reviewed-by: Rafael David Tinoco <[e-mail]>
+Last-Update: 2021-05-24
+
+qemu: block: Allow specifying cluster size when using 'blockdev-create'
+
+'blockdev-create' allows us to create the image with a custom cluster
+size if we wish to. Wire it up for 'qcow2'.
+
+Signed-off-by: Peter Krempa <[e-mail]>
+Reviewed-by: Eric Blake <[e-mail]>
+---
+ src/qemu/qemu_block.c     | 1 +
+ src/util/virstoragefile.h | 1 +
+ 2 files changed, 2 insertions(+)
+
+diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
+index eab21bc107..639ff37e1f 100644
+--- a/src/qemu/qemu_block.c
++++ b/src/qemu/qemu_block.c
+@@ -2149,6 +2149,7 @@ qemuBlockStorageSourceCreateGetFormatPropsQcow2(virStorageSourcePtr src,
+                                  "s:file", src->nodestorage,
+                                  "U:size", src->capacity,
+                                  "S:version", qcow2version,
++                                 "P:cluster-size", src->clusterSize,
+                                  NULL) < 0)
+         return -1;
+ 
+diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
+index 39e50a989d..7ec31652d5 100644
+--- a/src/util/virstoragefile.h
++++ b/src/util/virstoragefile.h
+@@ -291,6 +291,7 @@ struct _virStorageSource {
+     unsigned long long capacity; /* in bytes, 0 if unknown */
+     unsigned long long allocation; /* in bytes, 0 if unknown */
+     unsigned long long physical; /* in bytes, 0 if unknown */
++    unsigned long long clusterSize; /* in bytes, 0 if unknown */
+     bool has_allocation; /* Set to true when provided in XML */
+ 
+     size_t nseclabels;
+-- 
+2.25.1
+
diff --git a/debian/patches/clustersize/0003-qemuBlockStorageSourceCreateDetectSize-Propagate-clu.patch b/debian/patches/clustersize/0003-qemuBlockStorageSourceCreateDetectSize-Propagate-clu.patch
new file mode 100644
index 0000000..a9ad5bc
--- /dev/null
+++ b/debian/patches/clustersize/0003-qemuBlockStorageSourceCreateDetectSize-Propagate-clu.patch
@@ -0,0 +1,40 @@
+From de79fad40fb6a970fab9dd566ed83ecf56877ea2 Mon Sep 17 00:00:00 2001
+Description: qemuBlockStorageSourceCreateDetectSize: Propagate cluster size for 'qcow2'
+Author: Peter Krempa <[e-mail]>
+Origin: upstream, https://github.com/libvirt/libvirt/commit/de79fad40fb6a970fab9dd566ed83ecf56877ea2
+Bug: https://gitlab.com/libvirt/libvirt/-/issues/154
+Reviewed-by: Rafael David Tinoco <[e-mail]>
+Last-Update: 2021-05-24
+
+qemuBlockStorageSourceCreateDetectSize: Propagate cluster size for 'qcow2'
+
+Propagate the cluster size from the original image as the user might
+have configured a custom cluster size for performance reasons. Propagate
+the cluster size of a qcow2 image to the new overlay or copy.
+
+Signed-off-by: Peter Krempa <[e-mail]>
+Reviewed-by: Eric Blake <[e-mail]>
+---
+ src/qemu/qemu_block.c | 6 ++++++
+ 1 file changed, 6 insertions(+)
+
+diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
+index 639ff37e1f..e0af61954d 100644
+--- a/src/qemu/qemu_block.c
++++ b/src/qemu/qemu_block.c
+@@ -2604,6 +2604,12 @@ qemuBlockStorageSourceCreateDetectSize(virHashTablePtr blockNamedNodeData,
+         return -1;
+     }
+ 
++    /* propagate cluster size if the images are compatible */
++    if (templ->format == VIR_STORAGE_FILE_QCOW2 &&
++        src->format == VIR_STORAGE_FILE_QCOW2 &&
++        src->clusterSize == 0)
++        src->clusterSize = entry->clusterSize;
++
+     if (src->format == VIR_STORAGE_FILE_RAW) {
+         src->physical = entry->capacity;
+     } else {
+-- 
+2.25.1
+
diff --git a/debian/patches/clustersize/0004-conf-Move-and-rename-virDomainParseScaledValue.patch b/debian/patches/clustersize/0004-conf-Move-and-rename-virDomainParseScaledValue.patch
new file mode 100644
index 0000000..fc04190
--- /dev/null
+++ b/debian/patches/clustersize/0004-conf-Move-and-rename-virDomainParseScaledValue.patch
@@ -0,0 +1,352 @@
+From 04bd77a19f8312493151ce377da40577b1470a0b Mon Sep 17 00:00:00 2001
+Description: conf: Move and rename virDomainParseScaledValue()
+Author: Michal Privoznik <[e-mail]>
+Origin: backport, https://github.com/libvirt/libvirt/commit/04bd77a19f8312493151ce377da40577b1470a0b
+Bug: https://gitlab.com/libvirt/libvirt/-/issues/154
+Reviewed-by: Rafael David Tinoco <[e-mail]>
+Last-Update: 2021-05-24
+
+conf: Move and rename virDomainParseScaledValue()
+
+There is nothing domain specific about the function, thus it
+should not have virDomain prefix. Also, the fact that it is a
+static function makes it impossible to use from other files.
+Move the function to virxml.c and drop the 'Domain' infix.
+
+[Backport]
+  This commit renames, relocates and declares a function without
+  changing its logic: makes cherry-picking the feature "QCOW2
+  cluster_size option" fast forward without changing its logic.
+
+Signed-off-by: Michal Privoznik <[e-mail]>
+Reviewed-by: Daniel Henrique Barboza <[e-mail]>
+Signed-off-by: Rafael David Tinoco <[e-mail]>
+---
+ src/conf/domain_conf.c   | 135 ++++++++++-----------------------------
+ src/libvirt_private.syms |   1 +
+ src/util/virxml.c        |  72 +++++++++++++++++++++
+ src/util/virxml.h        |   8 +++
+ 4 files changed, 114 insertions(+), 102 deletions(-)
+
+diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
+index ee57152da7..aac6c6df07 100644
+--- a/src/conf/domain_conf.c
++++ b/src/conf/domain_conf.c
+@@ -10459,75 +10459,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
+     goto cleanup;
+ }
+ 
+-/**
+- * virDomainParseScaledValue:
+- * @xpath: XPath to memory amount
+- * @units_xpath: XPath to units attribute
+- * @ctxt: XPath context
+- * @val: scaled value is stored here
+- * @scale: default scale for @val
+- * @max: maximal @val allowed
+- * @required: is the value required?
+- *
+- * Parse a value located at @xpath within @ctxt, and store the
+- * result into @val. The value is scaled by units located at
+- * @units_xpath (or the 'unit' attribute under @xpath if
+- * @units_xpath is NULL). If units are not present, the default
+- * @scale is used. If @required is set, then the value must
+- * exist; otherwise, the value is optional. The resulting value
+- * is in bytes.
+- *
+- * Returns 1 on success,
+- *         0 if the value was not present and !@required,
+- *         -1 on failure after issuing error.
+- */
+-static int
+-virDomainParseScaledValue(const char *xpath,
+-                          const char *units_xpath,
+-                          xmlXPathContextPtr ctxt,
+-                          unsigned long long *val,
+-                          unsigned long long scale,
+-                          unsigned long long max,
+-                          bool required)
+-{
+-    unsigned long long bytes;
+-    g_autofree char *xpath_full = NULL;
+-    g_autofree char *unit = NULL;
+-    g_autofree char *bytes_str = NULL;
+-
+-    *val = 0;
+-    xpath_full = g_strdup_printf("string(%s)", xpath);
+-
+-    bytes_str = virXPathString(xpath_full, ctxt);
+-    if (!bytes_str) {
+-        if (!required)
+-            return 0;
+-        virReportError(VIR_ERR_XML_ERROR,
+-                       _("missing element or attribute '%s'"),
+-                       xpath);
+-        return -1;
+-    }
+-    VIR_FREE(xpath_full);
+-
+-    if (virStrToLong_ullp(bytes_str, NULL, 10, &bytes) < 0) {
+-        virReportError(VIR_ERR_XML_ERROR,
+-                       _("Invalid value '%s' for element or attribute '%s'"),
+-                       bytes_str, xpath);
+-        return -1;
+-    }
+-
+-    if (units_xpath)
+-         xpath_full = g_strdup_printf("string(%s)", units_xpath);
+-    else
+-         xpath_full = g_strdup_printf("string(%s/@unit)", xpath);
+-    unit = virXPathString(xpath_full, ctxt);
+-
+-    if (virScaleInteger(&bytes, unit, scale, max) < 0)
+-        return -1;
+-
+-    *val = bytes;
+-    return 1;
+-}
+ 
+ 
+ /**
+@@ -10564,8 +10495,8 @@ virDomainParseMemory(const char *xpath,
+ 
+     max = virMemoryMaxValue(capped);
+ 
+-    if (virDomainParseScaledValue(xpath, units_xpath, ctxt,
+-                                  &bytes, 1024, max, required) < 0)
++    if (virParseScaledValue(xpath, units_xpath, ctxt,
++                            &bytes, 1024, max, required) < 0)
+         return -1;
+ 
+     /* Yes, we really do use kibibytes for our internal sizing.  */
+@@ -10607,9 +10538,9 @@ virDomainParseMemoryLimit(const char *xpath,
+     int ret;
+     unsigned long long bytes;
+ 
+-    ret = virDomainParseScaledValue(xpath, units_xpath, ctxt, &bytes, 1024,
+-                                    VIR_DOMAIN_MEMORY_PARAM_UNLIMITED << 10,
+-                                    false);
++    ret = virParseScaledValue(xpath, units_xpath, ctxt, &bytes, 1024,
++                              VIR_DOMAIN_MEMORY_PARAM_UNLIMITED << 10,
++                              false);
+ 
+     if (ret < 0)
+         return -1;
+@@ -10938,9 +10869,9 @@ virDomainControllerDefParseXML(virDomainXMLOptionPtr xmlopt,
+                                  "have an address"));
+                 goto error;
+             }
+-            if ((rc = virDomainParseScaledValue("./pcihole64", NULL,
+-                                                ctxt, &bytes, 1024,
+-                                                1024ULL * ULONG_MAX, false)) < 0)
++            if ((rc = virParseScaledValue("./pcihole64", NULL,
++                                          ctxt, &bytes, 1024,
++                                          1024ULL * ULONG_MAX, false)) < 0)
+                 goto error;
+ 
+             if (rc == 1)
+@@ -11151,14 +11082,14 @@ virDomainFSDefParseXML(virDomainXMLOptionPtr xmlopt,
+         }
+     }
+ 
+-    if (virDomainParseScaledValue("./space_hard_limit[1]",
+-                                  NULL, ctxt, &def->space_hard_limit,
+-                                  1, ULLONG_MAX, false) < 0)
++    if (virParseScaledValue("./space_hard_limit[1]",
++                            NULL, ctxt, &def->space_hard_limit,
++                            1, ULLONG_MAX, false) < 0)
+         goto error;
+ 
+-    if (virDomainParseScaledValue("./space_soft_limit[1]",
+-                                  NULL, ctxt, &def->space_soft_limit,
+-                                  1, ULLONG_MAX, false) < 0)
++    if (virParseScaledValue("./space_soft_limit[1]",
++                            NULL, ctxt, &def->space_soft_limit,
++                            1, ULLONG_MAX, false) < 0)
+         goto error;
+ 
+     cur = node->children;
+@@ -14931,8 +14862,8 @@ virDomainShmemDefParseXML(virDomainXMLOptionPtr xmlopt,
+         goto cleanup;
+     }
+ 
+-    if (virDomainParseScaledValue("./size[1]", NULL, ctxt,
+-                                  &def->size, 1, ULLONG_MAX, false) < 0)
++    if (virParseScaledValue("./size[1]", NULL, ctxt,
++                            &def->size, 1, ULLONG_MAX, false) < 0)
+         goto cleanup;
+ 
+     if ((server = virXPathNode("./server[1]", ctxt))) {
+@@ -19308,9 +19239,9 @@ virDomainCachetuneDefParseCache(xmlXPathContextPtr ctxt,
+         return -1;
+     }
+ 
+-    if (virDomainParseScaledValue("./@size", "./@unit",
+-                                  ctxt, &size, 1024,
+-                                  ULLONG_MAX, true) < 0)
++    if (virParseScaledValue("./@size", "./@unit",
++                             ctxt, &size, 1024,
++                             ULLONG_MAX, true) < 0)
+         return -1;
+ 
+     if (virResctrlAllocSetCacheSize(alloc, level, type, cache, size) < 0)
+@@ -20417,13 +20348,13 @@ virDomainDefParseXML(xmlDocPtr xml,
+                 VIR_FREE(tmp);
+             }
+ 
+-            if (virDomainParseScaledValue("./features/hpt/maxpagesize",
+-                                          NULL,
+-                                          ctxt,
+-                                          &def->hpt_maxpagesize,
+-                                          1024,
+-                                          ULLONG_MAX,
+-                                          false) < 0) {
++            if (virParseScaledValue("./features/hpt/maxpagesize",
++                                    NULL,
++                                    ctxt,
++                                    &def->hpt_maxpagesize,
++                                    1024,
++                                    ULLONG_MAX,
++                                    false) < 0) {
+                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                                "%s",
+                                _("Unable to parse HPT maxpagesize setting"));
+@@ -20649,13 +20580,13 @@ virDomainDefParseXML(xmlDocPtr xml,
+     }
+ 
+     if (def->features[VIR_DOMAIN_FEATURE_SMM] == VIR_TRISTATE_SWITCH_ON) {
+-        int rv = virDomainParseScaledValue("string(./features/smm/tseg)",
+-                                           "string(./features/smm/tseg/@unit)",
+-                                           ctxt,
+-                                           &def->tseg_size,
+-                                           1024 * 1024, /* Defaults to mebibytes */
+-                                           ULLONG_MAX,
+-                                           false);
++        int rv = virParseScaledValue("string(./features/smm/tseg)",
++                                     "string(./features/smm/tseg/@unit)",
++                                     ctxt,
++                                     &def->tseg_size,
++                                     1024 * 1024, /* Defaults to mebibytes */
++                                     ULLONG_MAX,
++                                     false);
+         if (rv < 0)
+             goto error;
+         def->tseg_specified = rv;
+diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
+index b97906b852..7bd5e3f445 100644
+--- a/src/libvirt_private.syms
++++ b/src/libvirt_private.syms
+@@ -3435,6 +3435,7 @@ virVsockSetGuestCid;
+ 
+ 
+ # util/virxml.h
++virParseScaledValue;
+ virXMLCheckIllegalChars;
+ virXMLChildElementCount;
+ virXMLExtractNamespaceXML;
+diff --git a/src/util/virxml.c b/src/util/virxml.c
+index 0e66d1623b..bae2e6aca5 100644
+--- a/src/util/virxml.c
++++ b/src/util/virxml.c
+@@ -33,6 +33,7 @@
+ #include "viralloc.h"
+ #include "virfile.h"
+ #include "virstring.h"
++#include "virutil.h"
+ 
+ #define VIR_FROM_THIS VIR_FROM_XML
+ 
+@@ -1433,3 +1434,74 @@ virXMLNamespaceRegister(xmlXPathContextPtr ctxt,
+ 
+     return 0;
+ }
++
++
++/**
++ * virParseScaledValue:
++ * @xpath: XPath to memory amount
++ * @units_xpath: XPath to units attribute
++ * @ctxt: XPath context
++ * @val: scaled value is stored here
++ * @scale: default scale for @val
++ * @max: maximal @val allowed
++ * @required: is the value required?
++ *
++ * Parse a value located at @xpath within @ctxt, and store the
++ * result into @val. The value is scaled by units located at
++ * @units_xpath (or the 'unit' attribute under @xpath if
++ * @units_xpath is NULL). If units are not present, the default
++ * @scale is used. If @required is set, then the value must
++ * exist; otherwise, the value is optional. The resulting value
++ * is in bytes.
++ *
++ * Returns 1 on success,
++ *         0 if the value was not present and !@required,
++ *         -1 on failure after issuing error.
++ */
++int
++virParseScaledValue(const char *xpath,
++                    const char *units_xpath,
++                    xmlXPathContextPtr ctxt,
++                    unsigned long long *val,
++                    unsigned long long scale,
++                    unsigned long long max,
++                    bool required)
++{
++    unsigned long long bytes;
++    g_autofree char *xpath_full = NULL;
++    g_autofree char *unit = NULL;
++    g_autofree char *bytes_str = NULL;
++
++    *val = 0;
++    xpath_full = g_strdup_printf("string(%s)", xpath);
++
++    bytes_str = virXPathString(xpath_full, ctxt);
++    if (!bytes_str) {
++        if (!required)
++            return 0;
++        virReportError(VIR_ERR_XML_ERROR,
++                       _("missing element or attribute '%s'"),
++                       xpath);
++        return -1;
++    }
++    VIR_FREE(xpath_full);
++
++    if (virStrToLong_ullp(bytes_str, NULL, 10, &bytes) < 0) {
++        virReportError(VIR_ERR_XML_ERROR,
++                       _("Invalid value '%s' for element or attribute '%s'"),
++                       bytes_str, xpath);
++        return -1;
++    }
++
++    if (units_xpath)
++         xpath_full = g_strdup_printf("string(%s)", units_xpath);
++    else
++         xpath_full = g_strdup_printf("string(%s/@unit)", xpath);
++    unit = virXPathString(xpath_full, ctxt);
++
++    if (virScaleInteger(&bytes, unit, scale, max) < 0)
++        return -1;
++
++    *val = bytes;
++    return 1;
++}
+diff --git a/src/util/virxml.h b/src/util/virxml.h
+index 26ab9f9c2d..39b261687a 100644
+--- a/src/util/virxml.h
++++ b/src/util/virxml.h
+@@ -269,3 +269,11 @@ virXMLNamespaceFormatNS(virBufferPtr buf,
+ int
+ virXMLNamespaceRegister(xmlXPathContextPtr ctxt,
+                         virXMLNamespace const *ns);
++
++int virParseScaledValue(const char *xpath,
++                        const char *units_xpath,
++                        xmlXPathContextPtr ctxt,
++                        unsigned long long *val,
++                        unsigned long long scale,
++                        unsigned long long max,
++                        bool required);
+-- 
+2.25.1
+
diff --git a/debian/patches/clustersize/0005-storage-add-support-for-QCOW2-cluster_size-option.patch b/debian/patches/clustersize/0005-storage-add-support-for-QCOW2-cluster_size-option.patch
new file mode 100644
index 0000000..ee13577
--- /dev/null
+++ b/debian/patches/clustersize/0005-storage-add-support-for-QCOW2-cluster_size-option.patch
@@ -0,0 +1,191 @@
+From 3e1d2c93a3edce65ce8cb589edb07e2f830c9c12 Mon Sep 17 00:00:00 2001
+Description: storage: add support for QCOW2 cluster_size option
+Author: Pavel Hrdina <[e-mail]>
+Origin: upstream, https://github.com/libvirt/libvirt/commit/3e1d2c93a3edce65ce8cb589edb07e2f830c9c12
+Bug: https://gitlab.com/libvirt/libvirt/-/issues/154
+Reviewed-by: Rafael David Tinoco <[e-mail]>
+Last-Update: 2021-05-24
+
+storage: add support for QCOW2 cluster_size option
+
+The default value hard-coded in QEMU (64KiB) is not always the ideal.
+Having a possibility to set the cluster_size by user may in specific
+use-cases improve performance for QCOW2 images.
+
+QEMU internally has some limits, the value has to be between 512B and
+2048KiB and must by power of two, except when the image has Extended L2
+Entries the minimal value has to be 16KiB.
+
+Since qemu-img ensures the value is correct and the limit is not always
+the same libvirt will not duplicate any of these checks as the error
+message from qemu-img is good enough:
+
+    Cluster size must be a power of two between 512 and 2048k
+
+Resolves: https://gitlab.com/libvirt/libvirt/-/issues/154
+
+Signed-off-by: Pavel Hrdina <[e-mail]>
+Reviewed-by: Peter Krempa <[e-mail]>
+---
+ docs/formatstorage.html.in                      |  6 ++++++
+ docs/schemas/storagecommon.rng                  |  6 ++++++
+ docs/schemas/storagevol.rng                     |  3 +++
+ src/conf/storage_conf.c                         | 12 ++++++++++++
+ src/storage/storage_util.c                      |  5 +++++
+ .../qcow2-clusterSize.argv                      |  6 ++++++
+ tests/storagevolxml2argvtest.c                  |  4 ++++
+ .../vol-qcow2-clusterSize.xml                   | 17 +++++++++++++++++
+ .../vol-qcow2-clusterSize.xml                   | 17 +++++++++++++++++
+ tests/storagevolxml2xmltest.c                   |  1 +
+ 10 files changed, 77 insertions(+)
+ create mode 100644 tests/storagevolxml2argvdata/qcow2-clusterSize.argv
+ create mode 100644 tests/storagevolxml2xmlin/vol-qcow2-clusterSize.xml
+ create mode 100644 tests/storagevolxml2xmlout/vol-qcow2-clusterSize.xml
+
+--- a/docs/formatstorage.html.in
++++ b/docs/formatstorage.html.in
+@@ -766,6 +766,7 @@
+   &lt;/encryption&gt;
+   &lt;compat&gt;1.1&lt;/compat&gt;
+   &lt;nocow/&gt;
++  &lt;clusterSize unit='KiB'&gt;64&lt;/clusterSize&gt;
+   &lt;features&gt;
+     &lt;lazy_refcounts/&gt;
+   &lt;/features&gt;
+@@ -842,6 +843,11 @@
+         the file image is used in VM. To create non-raw file images, it
+         requires QEMU version since 2.1. <span class="since">Since 1.2.7</span>
+       </dd>
++      <dt><code>clusterSize</code></dt>
++      <dd>Changes the qcow2 cluster size which can affect image file size
++        and performance.
++        <span class="since">Since 7.4.0</span>
++      </dd>
+       <dt><code>features</code></dt>
+       <dd>Format-specific features. Only used for <code>qcow2</code> now.
+         Valid sub-elements are:
+--- a/docs/schemas/storagecommon.rng
++++ b/docs/schemas/storagecommon.rng
+@@ -111,6 +111,12 @@
+     </element>
+   </define>
+ 
++  <define name="clusterSize">
++    <element name="clusterSize">
++      <ref name="scaledInteger"/>
++    </element>
++  </define>
++
+   <define name='compat'>
+     <element name='compat'>
+       <data type='string'>
+--- a/docs/schemas/storagevol.rng
++++ b/docs/schemas/storagevol.rng
+@@ -125,6 +125,9 @@
+           </element>
+         </optional>
+         <optional>
++          <ref name="clusterSize"/>
++        </optional>
++        <optional>
+           <ref name='fileFormatFeatures'/>
+         </optional>
+       </interleave>
+--- a/src/conf/storage_conf.c
++++ b/src/conf/storage_conf.c
+@@ -1397,6 +1397,13 @@
+     if (virXPathNode("./target/nocow", ctxt))
+         def->target.nocow = true;
+ 
++    if (virParseScaledValue("./target/clusterSize",
++                            "./target/clusterSize/@unit",
++                            ctxt, &def->target.clusterSize,
++                            1, ULLONG_MAX, false) < 0) {
++        return NULL;
++    }
++
+     if (virXPathNode("./target/features", ctxt)) {
+         if ((n = virXPathNodeSet("./target/features/*", ctxt, &nodes)) < 0)
+             return NULL;
+@@ -1562,6 +1569,11 @@
+ 
+     virBufferEscapeString(buf, "<compat>%s</compat>\n", def->compat);
+ 
++    if (def->clusterSize > 0) {
++        virBufferAsprintf(buf, "<clusterSize unit='B'>%llu</clusterSize>\n",
++                          def->clusterSize);
++    }
++
+     if (def->features) {
+         size_t i;
+         bool empty = virBitmapIsAllClear(def->features);
+--- a/src/storage/storage_util.c
++++ b/src/storage/storage_util.c
+@@ -693,6 +693,7 @@
+     const char *path;
+     unsigned long long size_arg;
+     unsigned long long allocation;
++    unsigned long long clusterSize;
+     bool encryption;
+     bool preallocate;
+     const char *compat;
+@@ -739,6 +740,9 @@
+     else if (info->format == VIR_STORAGE_FILE_QCOW2)
+         virBufferAddLit(&buf, "compat=0.10,");
+ 
++    if (info->clusterSize > 0)
++        virBufferAsprintf(&buf, "cluster_size=%llu,", info->clusterSize);
++
+     if (info->features && info->format == VIR_STORAGE_FILE_QCOW2) {
+         if (virBitmapIsBitSet(info->features,
+                               VIR_STORAGE_FILE_FEATURE_LAZY_REFCOUNTS)) {
+@@ -1069,6 +1073,7 @@
+         .compat = vol->target.compat,
+         .features = vol->target.features,
+         .nocow = vol->target.nocow,
++        .clusterSize = vol->target.clusterSize,
+         .secretAlias = NULL,
+     };
+     virStorageEncryptionPtr enc = vol->target.encryption;
+--- /dev/null
++++ b/tests/storagevolxml2argvdata/qcow2-clusterSize.argv
+@@ -0,0 +1,6 @@
++qemu-img \
++create \
++-f qcow2 \
++-o compat=0.10,cluster_size=131072 \
++/var/lib/libvirt/images/OtherDemo.img \
++5242880K
+--- a/tests/storagevolxml2argvtest.c
++++ b/tests/storagevolxml2argvtest.c
+@@ -238,6 +238,10 @@
+                  "pool-dir", "vol-qcow2-nocapacity-backing", NULL, NULL,
+                  "qcow2-nocapacity", 0);
+ 
++    DO_TEST("pool-dir", "vol-qcow2-clusterSize",
++            NULL, NULL,
++            "qcow2-clusterSize", 0);
++
+     DO_TEST("pool-dir", "vol-file-iso",
+             NULL, NULL,
+             "iso", 0);
+--- /dev/null
++++ b/tests/storagevolxml2xmlin/vol-qcow2-clusterSize.xml
+@@ -0,0 +1,17 @@
++<volume>
++  <name>OtherDemo.img</name>
++  <key>/var/lib/libvirt/images/OtherDemo.img</key>
++  <capacity unit="G">5</capacity>
++  <allocation>294912</allocation>
++  <target>
++    <path>/var/lib/libvirt/images/OtherDemo.img</path>
++    <format type='qcow2'/>
++    <permissions>
++      <mode>0644</mode>
++      <owner>0</owner>
++      <group>0</group>
++      <label>unconfined_u:object_r:virt_image_t:s0</label>
++    </permissions>
++    <clusterSize unit='KiB'>128</clusterSize>
++  </target>
++</volume>
diff --git a/debian/patches/clustersize/0006-storage_file-add-support-to-probe-cluster_size-from-.patch b/debian/patches/clustersize/0006-storage_file-add-support-to-probe-cluster_size-from-.patch
new file mode 100644
index 0000000..4966303
--- /dev/null
+++ b/debian/patches/clustersize/0006-storage_file-add-support-to-probe-cluster_size-from-.patch
@@ -0,0 +1,228 @@
+From 93344aed27c686239ff53b8912eb4476be5e9ef7 Mon Sep 17 00:00:00 2001
+Description: storage_file: add support to probe cluster_size from QCOW2 images
+Author: Pavel Hrdina <[e-mail]>
+Origin: backport, https://github.com/libvirt/libvirt/commit/93344aed27c686239ff53b8912eb4476be5e9ef7
+Bug: https://gitlab.com/libvirt/libvirt/-/issues/154
+Reviewed-by: Rafael David Tinoco <[e-mail]>
+Last-Update: 2021-05-24
+
+storage_file: add support to probe cluster_size from QCOW2 images
+
+From QEMU docs/interop/qcow2.txt :
+
+   Byte  20 - 23:   cluster_bits
+                    Number of bits that are used for addressing an offset
+                    within a cluster (1 << cluster_bits is the cluster size).
+
+With this patch libvirt will be able to report the current cluster_size
+for all existing storage volumes managed by storage driver.
+
+[Backport]
+
+  Commit 2cdd833eae ("util: move virStorageFileProbe code into
+  storage_file") did move the src/util/virstoragefile.c into
+  storage_file/storage_file_probe.c. For backport purposes that
+  move is not needed for this patch to be functional.
+
+Signed-off-by: Pavel Hrdina <[e-mail]>
+Reviewed-by: Peter Krempa <[e-mail]>
+Signed-off-by: Rafael David Tinoco <[e-mail]>
+---
+ src/storage/storage_util.c |  3 ++
+ src/util/virstoragefile.c  | 62 +++++++++++++++++++++++++++-----------
+ 2 files changed, 48 insertions(+), 17 deletions(-)
+
+diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
+index 08e6ab31b1..0e06db341f 100644
+--- a/src/storage/storage_util.c
++++ b/src/storage/storage_util.c
+@@ -3400,6 +3400,9 @@ storageBackendProbeTarget(virStorageSourcePtr target,
+     if (meta->capacity)
+         target->capacity = meta->capacity;
+ 
++    if (meta->clusterSize > 0)
++        target->clusterSize = meta->clusterSize;
++
+     if (encryption && meta->encryption) {
+         if (meta->encryption->payload_offset != -1)
+             target->capacity -= meta->encryption->payload_offset * 512;
+diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
+index 1397f532fd..7e3957de0f 100644
+--- a/src/util/virstoragefile.c
++++ b/src/util/virstoragefile.c
+@@ -170,6 +170,8 @@ struct FileTypeInfo {
+                            * or NULL if there is no COW base image, to RES;
+                            * return BACKING_STORE_* */
+     const struct FileEncryptionInfo *cryptInfo; /* Encryption info */
++    unsigned long long (*getClusterSize)(const char *buf,
++                                         size_t buf_size);
+     int (*getBackingStore)(char **res, int *format,
+                            const char *buf, size_t buf_size);
+     int (*getFeatures)(virBitmapPtr *features, int format,
+@@ -179,6 +181,9 @@ struct FileTypeInfo {
+ 
+ static int cowGetBackingStore(char **, int *,
+                               const char *, size_t);
++static unsigned long long
++qcow2GetClusterSize(const char *buf,
++                    size_t buf_size);
+ static int qcowXGetBackingStore(char **, int *,
+                                 const char *, size_t);
+ static int qcow2GetFeatures(virBitmapPtr *features, int format,
+@@ -191,7 +196,8 @@ qedGetBackingStore(char **, int *, const char *, size_t);
+ #define QCOWX_HDR_VERSION (4)
+ #define QCOWX_HDR_BACKING_FILE_OFFSET (QCOWX_HDR_VERSION+4)
+ #define QCOWX_HDR_BACKING_FILE_SIZE (QCOWX_HDR_BACKING_FILE_OFFSET+8)
+-#define QCOWX_HDR_IMAGE_SIZE (QCOWX_HDR_BACKING_FILE_SIZE+4+4)
++#define QCOWX_HDR_CLUSTER_BITS_OFFSET (QCOWX_HDR_BACKING_FILE_SIZE+4)
++#define QCOWX_HDR_IMAGE_SIZE (QCOWX_HDR_CLUSTER_BITS_OFFSET+4)
+ 
+ #define QCOW1_HDR_CRYPT (QCOWX_HDR_IMAGE_SIZE+8+1+1+2)
+ #define QCOW2_HDR_CRYPT (QCOWX_HDR_IMAGE_SIZE+8)
+@@ -298,18 +304,18 @@ static struct FileEncryptionInfo const qcow2EncryptionInfo[] = {
+ 
+ static struct FileTypeInfo const fileTypeInfo[] = {
+     [VIR_STORAGE_FILE_NONE] = { 0, NULL, NULL, LV_LITTLE_ENDIAN,
+-                                -1, 0, {0}, 0, 0, 0, NULL, NULL, NULL },
++                               -1, 0, {0}, 0, 0, 0, NULL, NULL, NULL, NULL },
+     [VIR_STORAGE_FILE_RAW] = { 0, NULL, NULL, LV_LITTLE_ENDIAN,
+                                -1, 0, {0}, 0, 0, 0,
+                                luksEncryptionInfo,
+-                               NULL, NULL },
++                               NULL, NULL, NULL },
+     [VIR_STORAGE_FILE_DIR] = { 0, NULL, NULL, LV_LITTLE_ENDIAN,
+-                               -1, 0, {0}, 0, 0, 0, NULL, NULL, NULL },
++                               -1, 0, {0}, 0, 0, 0, NULL, NULL, NULL, NULL },
+     [VIR_STORAGE_FILE_BOCHS] = {
+         /*"Bochs Virtual HD Image", */ /* Untested */
+         0, NULL, NULL,
+         LV_LITTLE_ENDIAN, 64, 4, {0x20000},
+-        32+16+16+4+4+4+4+4, 8, 1, NULL, NULL, NULL
++        32+16+16+4+4+4+4+4, 8, 1, NULL, NULL, NULL, NULL
+     },
+     [VIR_STORAGE_FILE_CLOOP] = {
+         /* #!/bin/sh
+@@ -318,7 +324,7 @@ static struct FileTypeInfo const fileTypeInfo[] = {
+         */ /* Untested */
+         0, NULL, NULL,
+         LV_LITTLE_ENDIAN, -1, 0, {0},
+-        -1, 0, 0, NULL, NULL, NULL
++        -1, 0, 0, NULL, NULL, NULL, NULL
+     },
+     [VIR_STORAGE_FILE_DMG] = {
+         /* XXX QEMU says there's no magic for dmg,
+@@ -326,51 +332,52 @@ static struct FileTypeInfo const fileTypeInfo[] = {
+          * would have to match) but then disables that check. */
+         0, NULL, ".dmg",
+         0, -1, 0, {0},
+-        -1, 0, 0, NULL, NULL, NULL
++        -1, 0, 0, NULL, NULL, NULL, NULL
+     },
+     [VIR_STORAGE_FILE_ISO] = {
+         32769, "CD001", ".iso",
+         LV_LITTLE_ENDIAN, -2, 0, {0},
+-        -1, 0, 0, NULL, NULL, NULL
++        -1, 0, 0, NULL, NULL, NULL, NULL
+     },
+     [VIR_STORAGE_FILE_VPC] = {
+         0, "conectix", NULL,
+         LV_BIG_ENDIAN, 12, 4, {0x10000},
+-        8 + 4 + 4 + 8 + 4 + 4 + 2 + 2 + 4, 8, 1, NULL, NULL, NULL
++        8 + 4 + 4 + 8 + 4 + 4 + 2 + 2 + 4, 8, 1, NULL, NULL, NULL, NULL
+     },
+     /* TODO: add getBackingStore function */
+     [VIR_STORAGE_FILE_VDI] = {
+         64, "\x7f\x10\xda\xbe", ".vdi",
+         LV_LITTLE_ENDIAN, 68, 4, {0x00010001},
+-        64 + 5 * 4 + 256 + 7 * 4, 8, 1, NULL, NULL, NULL},
++        64 + 5 * 4 + 256 + 7 * 4, 8, 1, NULL, NULL, NULL, NULL},
+ 
+     /* Not direct file formats, but used for various drivers */
+     [VIR_STORAGE_FILE_FAT] = { 0, NULL, NULL, LV_LITTLE_ENDIAN,
+-                               -1, 0, {0}, 0, 0, 0, NULL, NULL, NULL },
++                               -1, 0, {0}, 0, 0, 0, NULL, NULL, NULL, NULL },
+     [VIR_STORAGE_FILE_VHD] = { 0, NULL, NULL, LV_LITTLE_ENDIAN,
+-                               -1, 0, {0}, 0, 0, 0, NULL, NULL, NULL },
++                               -1, 0, {0}, 0, 0, 0, NULL, NULL, NULL, NULL },
+     [VIR_STORAGE_FILE_PLOOP] = { 0, "WithouFreSpacExt", NULL, LV_LITTLE_ENDIAN,
+                                  -2, 0, {0}, PLOOP_IMAGE_SIZE_OFFSET, 0,
+-                                 PLOOP_SIZE_MULTIPLIER, NULL, NULL, NULL },
++                                 PLOOP_SIZE_MULTIPLIER, NULL, NULL, NULL, NULL },
+ 
+     /* All formats with a backing store probe below here */
+     [VIR_STORAGE_FILE_COW] = {
+         0, "OOOM", NULL,
+         LV_BIG_ENDIAN, 4, 4, {2},
+-        4+4+1024+4, 8, 1, NULL, cowGetBackingStore, NULL
++        4+4+1024+4, 8, 1, NULL, NULL, cowGetBackingStore, NULL
+     },
+     [VIR_STORAGE_FILE_QCOW] = {
+         0, "QFI", NULL,
+         LV_BIG_ENDIAN, 4, 4, {1},
+         QCOWX_HDR_IMAGE_SIZE, 8, 1,
+         qcow1EncryptionInfo,
+-        qcowXGetBackingStore, NULL
++        NULL, qcowXGetBackingStore, NULL
+     },
+     [VIR_STORAGE_FILE_QCOW2] = {
+         0, "QFI", NULL,
+         LV_BIG_ENDIAN, 4, 4, {2, 3},
+         QCOWX_HDR_IMAGE_SIZE, 8, 1,
+         qcow2EncryptionInfo,
++        qcow2GetClusterSize,
+         qcowXGetBackingStore,
+         qcow2GetFeatures
+     },
+@@ -378,12 +385,12 @@ static struct FileTypeInfo const fileTypeInfo[] = {
+         /* https://wiki.qemu.org/Features/QED */
+         0, "QED", NULL,
+         LV_LITTLE_ENDIAN, -2, 0, {0},
+-        QED_HDR_IMAGE_SIZE, 8, 1, NULL, qedGetBackingStore, NULL
++        QED_HDR_IMAGE_SIZE, 8, 1, NULL, NULL, qedGetBackingStore, NULL
+     },
+     [VIR_STORAGE_FILE_VMDK] = {
+         0, "KDMV", NULL,
+         LV_LITTLE_ENDIAN, 4, 4, {1, 2, 3},
+-        4+4+4, 8, 512, NULL, vmdk4GetBackingStore, NULL
++        4+4+4, 8, 512, NULL, NULL, vmdk4GetBackingStore, NULL
+     },
+ };
+ verify(G_N_ELEMENTS(fileTypeInfo) == VIR_STORAGE_FILE_LAST);
+@@ -543,6 +550,24 @@ qcow2GetExtensions(const char *buf,
+ }
+ 
+ 
++static unsigned long long
++qcow2GetClusterSize(const char *buf,
++                    size_t buf_size)
++{
++    int clusterBits = 0;
++
++    if ((QCOWX_HDR_CLUSTER_BITS_OFFSET + 4) > buf_size)
++        return 0;
++
++    clusterBits = virReadBufInt32BE(buf + QCOWX_HDR_CLUSTER_BITS_OFFSET);
++
++    if (clusterBits > 0)
++        return 1 << clusterBits;
++
++    return 0;
++}
++
++
+ static int
+ qcowXGetBackingStore(char **res,
+                      int *format,
+@@ -1030,6 +1055,9 @@ virStorageFileGetMetadataInternal(virStorageSourcePtr meta,
+         meta->capacity *= fileTypeInfo[meta->format].sizeMultiplier;
+     }
+ 
++    if (fileTypeInfo[meta->format].getClusterSize != NULL)
++        meta->clusterSize = fileTypeInfo[meta->format].getClusterSize(buf, len);
++
+     VIR_FREE(meta->backingStoreRaw);
+     if (fileTypeInfo[meta->format].getBackingStore != NULL) {
+         int store = fileTypeInfo[meta->format].getBackingStore(&meta->backingStoreRaw,
+-- 
+2.25.1
+
diff --git a/debian/patches/series b/debian/patches/series
index 33f754c..37fb113 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -155,3 +155,14 @@ ubuntu/lp-1922907-cputest-Add-data-for-Intel-R-Xeon-R-Platinum-9242-CP.patch
 ubuntu/lp-1922907-cputest-Add-data-for-Intel-R-Xeon-R-Gold-6130-CPU.patch
 ubuntu/lp-1922907-cpu_map-Distinguish-Cascadelake-Server-from-Skylake-.patch
 ubuntu/lp-1922907-cleanup-test-data.patch
+
+# backport QCOW2 cluster_size support
+# (https://gitlab.com/libvirt/libvirt/-/issues/154)
+
+clustersize/0001-qemu-monitor-Detect-image-cluster-size-from-query-na.patch
+clustersize/0002-qemu-block-Allow-specifying-cluster-size-when-using-.patch
+clustersize/0003-qemuBlockStorageSourceCreateDetectSize-Propagate-clu.patch
+clustersize/0004-conf-Move-and-rename-virDomainParseScaledValue.patch
+clustersize/0005-storage-add-support-for-QCOW2-cluster_size-option.patch
+clustersize/0006-storage_file-add-support-to-probe-cluster_size-from-.patch
+