Add discard_no_unref option for qcow2 images

Qemu 8.1.0 will add discard_no_unref option for qcow2 images.
When this option is enabled (default=false), then it will no longer
unreference clusters when guest does a discard, but it will just free
the blocks (useful for incremental backups for example) and pass the
discard to the lower layer.

This was implemented to avoid fragmentation within the qcow2 image.

Signed-off-by: Jean-Louis Dupond <jean-louis@dupond.be>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index c3526439bfbf4353dd8ed15d696b63efc0d3d7dd..2ea440e1a16807c557de4f29c98f6db347ccb7ba 100644 (file)
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -3284,6 +3284,13 @@ paravirtualized driver is specified via the ``disk`` element.
       format driver of the ``qemu`` hypervisor can be controlled via the
       ``max_size`` subelement (see example below).
 
+      The optional ``discard_no_unref`` attribute can be set to control the way
+      the ``qemu`` hypervisor handles guest discard commands inside the qcow2
+      image. When enabled, a discard request from within the guest will mark the
+      qcow2 cluster as zero, but will keep the reference/offset of that cluster.
+      But it will still pass the discard further to the lower layer.
+      This will resolve fragmentation within the qcow2 image. :since:`Since 9.5.0`
+
       In the majority of cases the default configuration used by the hypervisor
       is sufficient so modifying this setting should not be necessary. For
       specifics on how the metadata cache of ``qcow2`` in ``qemu`` behaves refer

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1fde53ff4c00d9e7322c00842e221b0963dbc2b0..4121b6a05430f6a938378163dae841689d7087f6 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7826,6 +7826,10 @@ virDomainDiskDefDriverParseXML(virDomainDiskDef *def,
     if (virXMLPropUInt(cur, "queue_size", 10, VIR_XML_PROP_NONE, &def->queue_size) < 0)
         return -1;
 
+    if (virXMLPropTristateSwitch(cur, "discard_no_unref", VIR_XML_PROP_NONE,
+                                 &def->discard_no_unref) < 0)
+        return -1;
+
     return 0;
 }
 
@@ -22501,6 +22505,10 @@ virDomainDiskDefFormatDriver(virBuffer *buf,
         virBufferAsprintf(&attrBuf, " detect_zeroes='%s'",
                           virDomainDiskDetectZeroesTypeToString(disk->detect_zeroes));
 
+    if (disk->discard_no_unref)
+        virBufferAsprintf(&attrBuf, " discard_no_unref='%s'",
+                          virTristateSwitchTypeToString(disk->discard_no_unref));
+
     if (disk->queues)
         virBufferAsprintf(&attrBuf, " queues='%u'", disk->queues);
 

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 629e32c39f672f6e3ee0b8c4d98b5e5662611a6b..cddaa3824d28f0d5c395bd300e4d746a17eb21fc 100644 (file)
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -606,6 +606,7 @@ struct _virDomainDiskDef {
     virDomainDiskDiscard discard;
     unsigned int iothread; /* unused = 0, > 0 specific thread # */
     virDomainDiskDetectZeroes detect_zeroes;
+    virTristateSwitch discard_no_unref;
     char *domain_name; /* backend domain name */
     unsigned int queues;
     unsigned int queue_size;

diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index 7e3c920a38ab32dd5ec4d12ecc3f55eac91469ab..6c4cfa16d62c988ad0bc3e7f87437f96ce78d8ae 100644 (file)
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -921,6 +921,20 @@ virDomainDiskDefValidate(const virDomainDef *def,
         return -1;
     }
 
+    if (disk->discard_no_unref == VIR_TRISTATE_SWITCH_ON) {
+        if (disk->src->format != VIR_STORAGE_FILE_QCOW2) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("'discard_no_unref' only works with qcow2 disk format"));
+            return -1;
+        }
+
+        if (disk->src->readonly) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("'discard_no_unref' is not compatible with read-only disk"));
+            return -1;
+        }
+    }
+
     return 0;
 }
 

diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
index 4b595fd066e7cd639e828c4bbdd644d9bbdd966e..fcf9e00600856d32ff91319c7c58df37b98fd071 100644 (file)
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -2516,6 +2516,11 @@
       <optional>
         <ref name="detect_zeroes"/>
       </optional>
+      <optional>
+        <attribute name="discard_no_unref">
+          <ref name="virOnOff"/>
+        </attribute>
+      </optional>
       <optional>
         <attribute name="queues">
           <ref name="positiveInteger"/>

diff --git a/src/conf/storage_source_conf.c b/src/conf/storage_source_conf.c
index 99061e4df76f4db5d884ad3f279025284c145964..dcac3a8ff653e6c1b03b214d6223b85519cbd110 100644 (file)
--- a/src/conf/storage_source_conf.c
+++ b/src/conf/storage_source_conf.c
@@ -809,6 +809,7 @@ virStorageSourceCopy(const virStorageSource *src,
     def->cachemode = src->cachemode;
     def->discard = src->discard;
     def->detect_zeroes = src->detect_zeroes;
+    def->discard_no_unref = src->discard_no_unref;
     def->sslverify = src->sslverify;
     def->readahead = src->readahead;
     def->timeout = src->timeout;

diff --git a/src/conf/storage_source_conf.h b/src/conf/storage_source_conf.h
index c6187dda597174da1cd3b66b7468d3bec48288ae..f13e7c756acb3f7af342a6b7972f1bb665485ce9 100644 (file)
--- a/src/conf/storage_source_conf.h
+++ b/src/conf/storage_source_conf.h
@@ -399,6 +399,7 @@ struct _virStorageSource {
     int cachemode; /* enum virDomainDiskCache */
     int discard; /* enum virDomainDiskDiscard */
     int detect_zeroes; /* enum virDomainDiskDetectZeroes */
+    virTristateSwitch discard_no_unref;
 
     bool floppyimg; /* set to true if the storage source is going to be used
                        as a source for floppy drive */

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 8b2159f8458797ca2ce13da8d92a999f9fceea1a..dcdf883926e4f2437c1efedcf8f4cf545d2e912a 100644 (file)
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -1117,12 +1117,11 @@ qemuBlockStorageSourceGetFormatQcow2Props(virStorageSource *src,
      * see: qemu.git/docs/qcow2-cache.txt
      * https://git.qemu.org/?p=qemu.git;a=blob;f=docs/qcow2-cache.txt
      */
-    if (src->metadataCacheMaxSize > 0) {
-        if (virJSONValueObjectAdd(&props,
-                                  "U:cache-size", src->metadataCacheMaxSize,
-                                  NULL) < 0)
-            return -1;
-    }
+    if (virJSONValueObjectAdd(&props,
+                              "P:cache-size", src->metadataCacheMaxSize,
+                              "T:discard-no-unref", src->discard_no_unref,
+                              NULL) < 0)
+        return -1;
 
     return 0;
 }

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index c65c571398b3b4091d293c71122aed5f3f323886..94587638c38dea0aaa3db2d430f4e0a70df0b1cd 100644 (file)
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -11039,6 +11039,7 @@ qemuDomainPrepareDiskSourceData(virDomainDiskDef *disk,
     src->iomode = disk->iomode;
     src->cachemode = disk->cachemode;
     src->discard = disk->discard;
+    src->discard_no_unref = disk->discard_no_unref;
 
     if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)
         src->floppyimg = true;

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c4bd766531e43e43cfde8d45fbde74fba90b17df..f20544590d9d26a7538c6a2a3a7583bcfc011dcc 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14242,8 +14242,10 @@ qemuDomainBlockCopyCommon(virDomainObj *vm,
      * into the topmost virStorage source of the disk chain.
      * Since 'mirror' has the ambition to replace it we need to propagate
      * it into the mirror too. We do it directly as otherwise we'd need
-     * to modify all callers of 'qemuDomainPrepareStorageSourceBlockdev' */
+     * to modify all callers of 'qemuDomainPrepareStorageSourceBlockdev'
+     * Same for discard_no_unref */
     mirror->detect_zeroes = disk->detect_zeroes;
+    mirror->discard_no_unref = disk->discard_no_unref;
 
     /* If reusing an external image that includes a backing file but the user
      * did not enumerate the chain in the XML we need to detect the chain */

diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 04d0c9df73de71c5b501bcbf008d5e6cdf470996..a53729d3498b79b057efbb91ebbf44bca41cc114 100644 (file)
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -3266,6 +3266,13 @@ qemuValidateDomainDeviceDefDisk(const virDomainDiskDef *disk,
         return -1;
     }
 
+    if (disk->discard_no_unref != VIR_TRISTATE_SWITCH_ABSENT &&
+        !virQEMUCapsGet(qemuCaps, QEMU_CAPS_QCOW2_DISCARD_NO_UNREF)) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("'discard_no_unref' is not supported by this QEMU binary"));
+        return -1;
+    }
+
     for (n = disk->src; virStorageSourceIsBacking(n); n = n->backingStore) {
         if (qemuDomainValidateStorageSource(n, qemuCaps) < 0)
             return -1;

diff --git a/src/vz/vz_utils.c b/src/vz/vz_utils.c
index 2db1146149fd55250476cbab684e7ec52e8818f0..7db7dbd419aff43a1b18aefd1014c4d480928b95 100644 (file)
--- a/src/vz/vz_utils.c
+++ b/src/vz/vz_utils.c
@@ -347,6 +347,12 @@ vzCheckDiskUnsupportedParams(virDomainDiskDef *disk)
         return -1;
     }
 
+    if (disk->discard_no_unref) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("Disk discard_no_unref is not supported by vz driver."));
+        return -1;
+    }
+
     if (disk->startupPolicy != VIR_DOMAIN_STARTUP_POLICY_DEFAULT) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                        _("Setting up disk startup policy is not "

diff --git a/tests/qemuxml2argvdata/disk-discard_no_unref.x86_64-latest.args b/tests/qemuxml2argvdata/disk-discard_no_unref.x86_64-latest.args
new file mode 100644 (file)
index 0000000..25b8211
--- /dev/null
+++ b/tests/qemuxml2argvdata/disk-discard_no_unref.x86_64-latest.args
@@ -0,0 +1,36 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/var/lib/libvirt/qemu/domain--1-test \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-test/.local/share \
+XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-test/.cache \
+XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-test/.config \
+/usr/bin/qemu-system-x86_64 \
+-name guest=test,debug-threads=on \
+-S \
+-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-test/master-key.aes"}' \
+-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram,acpi=off \
+-accel tcg \
+-cpu qemu64 \
+-m size=1048576k \
+-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":1073741824}' \
+-overcommit mem-lock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid 92d7a226-cfae-425b-a6d3-00bbf9ec5c9e \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-boot menu=on,strict=on \
+-device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \
+-blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/f14.img","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \
+-blockdev '{"node-name":"libvirt-1-format","read-only":false,"discard":"unmap","driver":"qcow2","discard-no-unref":true,"file":"libvirt-1-storage"}' \
+-device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x4","drive":"libvirt-1-format","id":"virtio-disk0","bootindex":2}' \
+-audiodev '{"id":"audio1","driver":"none"}' \
+-device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x2"}' \
+-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
+-msg timestamp=on

diff --git a/tests/qemuxml2argvdata/disk-discard_no_unref.xml b/tests/qemuxml2argvdata/disk-discard_no_unref.xml
new file mode 100644 (file)
index 0000000..d5dd054
--- /dev/null
+++ b/tests/qemuxml2argvdata/disk-discard_no_unref.xml
@@ -0,0 +1,32 @@
+<domain type='qemu'>
+  <name>test</name>
+  <uuid>92d7a226-cfae-425b-a6d3-00bbf9ec5c9e</uuid>
+  <memory unit='KiB'>1048576</memory>
+  <currentMemory unit='KiB'>1048576</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='x86_64' machine='pc'>hvm</type>
+    <boot dev='cdrom'/>
+    <boot dev='hd'/>
+    <bootmenu enable='yes'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>restart</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <!-- For this disk, intentionally stress parser resilience to
+         atypical ordering -->
+    <disk device='disk'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
+      <source file='/var/lib/libvirt/images/f14.img'/>
+      <target dev='vda' bus='virtio'/>
+      <driver discard='unmap' discard_no_unref='on' name='qemu' type='qcow2'/>
+    </disk>
+    <controller type='usb' index='0'/>
+    <controller type='ide' index='0'/>
+    <controller type='pci' index='0' model='pci-root'/>
+    <memballoon model='virtio'/>
+  </devices>
+</domain>

diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 0b5a2475c9c9c30c4e4634cfcf727b7e80539024..c1bba779b3a58e11e0877e17bb8f026bd78b97ae 100644 (file)
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1245,6 +1245,7 @@ mymain(void)
     DO_TEST_CAPS_LATEST("disk-copy_on_read");
     DO_TEST_CAPS_LATEST("disk-discard");
     DO_TEST_CAPS_LATEST("disk-detect-zeroes");
+    DO_TEST_CAPS_LATEST("disk-discard_no_unref");
     DO_TEST_CAPS_LATEST("disk-snapshot");
     DO_TEST_CAPS_LATEST_PARSE_ERROR("disk-same-targets");
     DO_TEST_CAPS_LATEST_PARSE_ERROR("disk-missing-target-invalid");

diff --git a/tests/qemuxml2xmloutdata/disk-discard_no_unref.x86_64-latest.xml b/tests/qemuxml2xmloutdata/disk-discard_no_unref.x86_64-latest.xml
new file mode 100644 (file)
index 0000000..c57acf9
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/disk-discard_no_unref.x86_64-latest.xml
@@ -0,0 +1,42 @@
+<domain type='qemu'>
+  <name>test</name>
+  <uuid>92d7a226-cfae-425b-a6d3-00bbf9ec5c9e</uuid>
+  <memory unit='KiB'>1048576</memory>
+  <currentMemory unit='KiB'>1048576</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='x86_64' machine='pc'>hvm</type>
+    <boot dev='cdrom'/>
+    <boot dev='hd'/>
+    <bootmenu enable='yes'/>
+  </os>
+  <cpu mode='custom' match='exact' check='none'>
+    <model fallback='forbid'>qemu64</model>
+  </cpu>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>restart</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <disk type='file' device='disk'>
+      <driver name='qemu' type='qcow2' discard='unmap' discard_no_unref='on'/>
+      <source file='/var/lib/libvirt/images/f14.img'/>
+      <target dev='vda' bus='virtio'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
+    </disk>
+    <controller type='usb' index='0' model='piix3-uhci'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+    </controller>
+    <controller type='ide' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
+    </controller>
+    <controller type='pci' index='0' model='pci-root'/>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <audio id='1' type='none'/>
+    <memballoon model='virtio'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
+    </memballoon>
+  </devices>
+</domain>

diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 5821e07cf176b3dc5df01654ba0a9829e1b16e6a..565cb3e1e10093a8132c092c9ce73a528e4617e0 100644 (file)
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -599,6 +599,7 @@ mymain(void)
 
     DO_TEST_CAPS_LATEST("disk-discard");
     DO_TEST_CAPS_LATEST("disk-detect-zeroes");
+    DO_TEST_CAPS_LATEST("disk-discard_no_unref");
 
     DO_TEST_NOCAPS("disk-serial");