Commit 154df5840d added support for <metadata_cache> as property of a
<disk>. Since the same parser is used to parse the XML used with
virDomainBlockCopy it starts the copy job with the appropriate cache
configured, but the <mirror> doesn't show this configuration nor it's
preserved if libvirtd is restarted during the mirror.

Add parsing, formatting and tests for <metadata_cache> for a <mirror>.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---

Since <metadata_cache> was introduced in this release we should probably
fix this too prior to the release.

 src/conf/domain_conf.c                        | 22 ++++++++++++++++++-
 tests/qemuxml2argvdata/disk-mirror.xml        | 14 ++++++++++++
 .../qemuxml2xmloutdata/disk-mirror-active.xml | 18 ++++++++++++++-
 .../disk-mirror-inactive.xml                  |  9 +++++++-
 4 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 349fc28c2a..01b7187637 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8784,6 +8784,12 @@ virDomainDiskDefMirrorParse(virDomainDiskDefPtr def,
         return -1;
     }

+    if (virParseScaledValue("./format/metadata_cache/max_size", NULL,
+                            ctxt,
+                            &def->mirror->metadataCacheMaxSize,
+                            1, ULLONG_MAX, false) < 0)
+        return -1;
+
     return 0;
 }

@@ -24283,6 +24289,8 @@ virDomainDiskDefFormatMirror(virBufferPtr buf,
 {
     g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER;
     g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf);
+    g_auto(virBuffer) formatAttrBuf = VIR_BUFFER_INITIALIZER;
+    g_auto(virBuffer) formatChildBuf = VIR_BUFFER_INIT_CHILD(&childBuf);
     const char *formatStr = NULL;

     /* For now, mirroring is currently output-only: we only output it
@@ -24311,7 +24319,19 @@ virDomainDiskDefFormatMirror(virBufferPtr buf,
         virBufferEscapeString(&attrBuf, " ready='%s'",
                               virDomainDiskMirrorStateTypeToString(disk->mirrorState));

-    virBufferEscapeString(&childBuf, "<format type='%s'/>\n", formatStr);
+    virBufferEscapeString(&formatAttrBuf, " type='%s'", formatStr);
+    if (disk->mirror->metadataCacheMaxSize > 0) {
+        g_auto(virBuffer) metadataCacheChildBuf = VIR_BUFFER_INIT_CHILD(&formatChildBuf);
+
+        virBufferAsprintf(&metadataCacheChildBuf,
+                          "<max_size unit='bytes'>%llu</max_size>\n",
+                          disk->mirror->metadataCacheMaxSize);
+
+        virXMLFormatElement(&formatChildBuf, "metadata_cache", NULL, &metadataCacheChildBuf);
+    }
+
+    virXMLFormatElement(&childBuf, "format", &formatAttrBuf, &formatChildBuf);
+
     if (virDomainDiskSourceFormat(&childBuf, disk->mirror, "source", 0, true,
                                   flags, false, false, xmlopt) < 0)
         return -1;
diff --git a/tests/qemuxml2argvdata/disk-mirror.xml b/tests/qemuxml2argvdata/disk-mirror.xml
index 2d61fe29c3..73886e99f4 100644
--- a/tests/qemuxml2argvdata/disk-mirror.xml
+++ b/tests/qemuxml2argvdata/disk-mirror.xml
@@ -54,6 +54,20 @@
       </mirror>
       <target dev='vdb' bus='virtio'/>
     </disk>
+    <disk type='file' device='disk'>
+      <source file='/tmp/data2.img'/>
+      <backingStore/>
+      <mirror type='file' file='/tmp/copy2.img' format='qcow2' job='copy'>
+        <format type='qcow2'>
+          <metadata_cache>
+            <max_size unit='bytes'>1234</max_size>
+          </metadata_cache>
+        </format>
+        <source file='/tmp/copy2.img'/>
+        <backingStore/>
+      </mirror>
+      <target dev='vdc' bus='virtio'/>
+    </disk>
     <controller type='usb' index='0'/>
     <controller type='ide' index='0'/>
     <controller type='pci' index='0' model='pci-root'/>
diff --git a/tests/qemuxml2xmloutdata/disk-mirror-active.xml b/tests/qemuxml2xmloutdata/disk-mirror-active.xml
index 17fb061d49..0e2669398c 100644
--- a/tests/qemuxml2xmloutdata/disk-mirror-active.xml
+++ b/tests/qemuxml2xmloutdata/disk-mirror-active.xml
@@ -61,6 +61,22 @@
       <target dev='vdb' bus='virtio'/>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
     </disk>
+    <disk type='file' device='disk'>
+      <driver name='qemu' type='raw'/>
+      <source file='/tmp/data2.img'/>
+      <backingStore/>
+      <mirror type='file' file='/tmp/copy2.img' format='qcow2' job='copy'>
+        <format type='qcow2'>
+          <metadata_cache>
+            <max_size unit='bytes'>1234</max_size>
+          </metadata_cache>
+        </format>
+        <source file='/tmp/copy2.img'/>
+        <backingStore/>
+      </mirror>
+      <target dev='vdc' bus='virtio'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/>
+    </disk>
     <controller type='usb' index='0'>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
     </controller>
@@ -71,7 +87,7 @@
     <input type='mouse' bus='ps2'/>
     <input type='keyboard' bus='ps2'/>
     <memballoon model='virtio'>
-      <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/>
     </memballoon>
   </devices>
 </domain>
diff --git a/tests/qemuxml2xmloutdata/disk-mirror-inactive.xml b/tests/qemuxml2xmloutdata/disk-mirror-inactive.xml
index 157ffcf6b2..6c7f92c1cc 100644
--- a/tests/qemuxml2xmloutdata/disk-mirror-inactive.xml
+++ b/tests/qemuxml2xmloutdata/disk-mirror-inactive.xml
@@ -43,6 +43,13 @@
       <target dev='vdb' bus='virtio'/>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
     </disk>
+    <disk type='file' device='disk'>
+      <driver name='qemu' type='raw'/>
+      <source file='/tmp/data2.img'/>
+      <backingStore/>
+      <target dev='vdc' bus='virtio'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/>
+    </disk>
     <controller type='usb' index='0'>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
     </controller>
@@ -53,7 +60,7 @@
     <input type='mouse' bus='ps2'/>
     <input type='keyboard' bus='ps2'/>
     <memballoon model='virtio'>
-      <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/>
     </memballoon>
   </devices>
 </domain>
-- 
2.29.2

On Thu, Jan 14, 2021 at 14:15:17 +0100, Peter Krempa wrote:
> Commit 154df5840d added support for <metadata_cache> as property of a
> <disk>. Since the same parser is used to parse the XML used with
> virDomainBlockCopy it starts the copy job with the appropriate cache
> configured, but the <mirror> doesn't show this configuration nor it's
> preserved if libvirtd is restarted during the mirror.
> 
> Add parsing, formatting and tests for <metadata_cache> for a <mirror>.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
> 
> Since <metadata_cache> was introduced in this release we should probably
> fix this too prior to the release.

Yeah, I think it makes sense to fix this bug in 7.0. After all, fixing
and stabilizing what we did since the last release is the main point of
the freeze period.

Reviewed-by: Jiri Denemark <jdenemar@redhat.com>