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>

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

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);
 

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;

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;
 }
 

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"/>

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;

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 */

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;
 }

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;

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 */

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;

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 "

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

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>

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");

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>

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");