conf: alter disk mirror xml output

Now that we track a disk mirror as a virStorageSource, we might
as well update the XML to theoretically allow any type of
mirroring destination (not just a local file).  A later patch
will also be reusing <mirror> to track the block commit of the
top layer of a chain, which is another case where libvirt needs
to update the backing chain after the job is finally pivoted,
and since backing chains can have network backing files as the
destination to commit into, it makes more sense to display that
in the XML.

This patch changes output-only XML; it was already documented
that <mirror> does not affect a domain definition at this point
(because qemu doesn't provide persistent bitmaps yet).  Any
application that was starting a block copy job with older libvirt
and then relying on the domain XML to determine if it was
complete will no longer be able to access the file= and format=
attributes of mirror that were previously used.  However, this is
not going to be a problem in practice: the only time a block copy
job works is on a transient domain, and any app that is managing
a transient domain probably already does enough of its own
bookkeeping to know which file it is mirroring into without
having to re-read it from the libvirt XML.  The one thing that
was likely to be used in a mirroring job was the ready=
attribute, which is unchanged.  Meanwhile, I made sure the schema
and parser still accept the old format, even if we no longer
output it, so that upgrading from an older version of libvirt is
seamless.

* docs/schemas/domaincommon.rng (diskMirror): Alter definition.
* src/conf/domain_conf.c (virDomainDiskDefParseXML): Parse two
styles of mirror elements.
(virDomainDiskDefFormat): Output new style.
* tests/qemuxml2argvdata/qemuxml2argv-disk-mirror-old.xml: New
file, copied from...
* tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml: ...here
before modernizing.
* tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old*: New
files.
* tests/qemuxml2xmltest.c (mymain): Test both styles.

Signed-off-by: Eric Blake <eblake@redhat.com>

docs/formatdomain.html.in

@@ -1879,15 +1879,23 @@
       <dt><code>mirror</code></dt>
       <dd>
         This element is present if the hypervisor has started a block
-        copy operation (via the <code>virDomainBlockCopy</code> API),
-        where the mirror location in attribute <code>file</code> will
-        eventually have the same contents as the source, and with the
-        file format in attribute <code>format</code> (which might
-        differ from the format of the source).  If
+        copy operation (via the <code>virDomainBlockRebase</code>
+        API), where the mirror location in the <code>source</code>
+        sub-element will eventually have the same contents as the
+        source, and with the file format in the
+        sub-element <code>format</code> (which might differ from the
+        format of the source).  The details of the <code>source</code>
+        sub-element are determined by the <code>type</code> attribute
+        of the mirror, similar to what is done for the
+        overall <code>disk</code> device element. If
         attribute <code>ready</code> is present, then it is known the
         disk is ready to pivot; otherwise, the disk is probably still
         copying.  For now, this element only valid in output; it is
-        ignored on input.  <span class="since">Since 0.9.12</span>
+        ignored on input.  <span class="since">Since 1.2.6</span>
+        (Older libvirt <span class="since">since 0.9.12</span> allowed
+        only mirroring to a file, with the information reported
+        in <code>file</code> and <code>format</code> attributes
+        of <code>mirror</code> rather than subelements.)
       </dd>
       <dt><code>target</code></dt>
       <dd>The <code>target</code> element controls the bus / device

docs/schemas/domaincommon.rng

@@ -4172,16 +4172,29 @@
       <empty/>
     </element>
   </define>
+
   <define name='diskMirror'>
     <element name='mirror'>
-      <attribute name='file'>
-        <ref name='absFilePath'/>
-      </attribute>
-      <optional>
-        <attribute name='format'>
-          <ref name='storageFormat'/>
-        </attribute>
-      </optional>
+      <choice>
+        <group>
+          <attribute name='file'>
+            <ref name='absFilePath'/>
+          </attribute>
+          <optional>
+            <attribute name='format'>
+              <ref name='storageFormat'/>
+            </attribute>
+          </optional>
+        </group>
+        <group>
+          <interleave>
+            <ref name="diskSource"/>
+            <optional>
+              <ref name="diskFormat"/>
+            </optional>
+          </interleave>
+        </group>
+      </choice>
       <optional>
         <attribute name='ready'>
           <value>yes</value>

src/conf/domain_conf.c

@@ -5239,6 +5239,8 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
     char *vendor = NULL;
     char *product = NULL;
     char *discard = NULL;
+    char *mirrorFormat = NULL;
+    char *mirrorType = NULL;
     int expected_secret_usage = -1;
     int auth_secret_usage = -1;
 
@@ -5375,18 +5377,34 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
                        xmlStrEqual(cur->name, BAD_CAST "mirror") &&
                        !(flags & VIR_DOMAIN_XML_INACTIVE)) {
                 char *ready;
-                char *mirrorFormat;
 
                 if (VIR_ALLOC(def->mirror) < 0)
                     goto error;
-                def->mirror->type = VIR_STORAGE_TYPE_FILE;
-                def->mirror->path = virXMLPropString(cur, "file");
-                if (!def->mirror->path) {
-                    virReportError(VIR_ERR_XML_ERROR, "%s",
-                                   _("mirror requires file name"));
-                    goto error;
+
+                mirrorType = virXMLPropString(cur, "type");
+                if (mirrorType) {
+                    def->mirror->type = virStorageTypeFromString(mirrorType);
+                    if (def->mirror->type <= 0) {
+                        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                                       _("unknown mirror backing store "
+                                         "type '%s'"), mirrorType);
+                        goto error;
+                    }
+                    mirrorFormat = virXPathString("string(./mirror/format/@type)",
+                                                  ctxt);
+                } else {
+                    /* For back-compat reasons, we handle a file name
+                     * encoded as attributes, even though we prefer
+                     * modern output in the style of backingStore */
+                    def->mirror->type = VIR_STORAGE_TYPE_FILE;
+                    def->mirror->path = virXMLPropString(cur, "file");
+                    if (!def->mirror->path) {
+                        virReportError(VIR_ERR_XML_ERROR, "%s",
+                                       _("mirror requires file name"));
+                        goto error;
+                    }
+                    mirrorFormat = virXMLPropString(cur, "format");
                 }
-                mirrorFormat = virXMLPropString(cur, "format");
                 if (mirrorFormat) {
                     def->mirror->format =
                         virStorageFileFormatTypeFromString(mirrorFormat);
@@ -5394,10 +5412,19 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
                         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                                        _("unknown mirror format value '%s'"),
                                        mirrorFormat);
-                        VIR_FREE(mirrorFormat);
                         goto error;
                     }
-                    VIR_FREE(mirrorFormat);
+                }
+                if (mirrorType) {
+                    xmlNodePtr mirrorNode;
+
+                    if (!(mirrorNode = virXPathNode("./mirror/source", ctxt))) {
+                        virReportError(VIR_ERR_XML_ERROR, "%s",
+                                       _("mirror requires source element"));
+                        goto error;
+                    }
+                    if (virDomainDiskSourceParse(mirrorNode, def->mirror) < 0)
+                        goto error;
                 }
                 ready = virXMLPropString(cur, "ready");
                 if (ready) {
@@ -5983,6 +6010,8 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
     VIR_FREE(wwn);
     VIR_FREE(vendor);
     VIR_FREE(product);
+    VIR_FREE(mirrorType);
+    VIR_FREE(mirrorFormat);
 
     ctxt->node = save_ctxt;
     return def;
@@ -15158,15 +15187,23 @@ virDomainDiskDefFormat(virBufferPtr buf,
 
     /* For now, mirroring is currently output-only: we only output it
      * for live domains, therefore we ignore it on input except for
-     * the internal parse on libvirtd restart.  */
+     * the internal parse on libvirtd restart.  We only output the
+     * new style similar to backingStore, even though the parser
+     * code still accepts old style across libvirtd upgrades. */
     if (def->mirror && !(flags & VIR_DOMAIN_XML_INACTIVE)) {
-        virBufferEscapeString(buf, "<mirror file='%s'", def->mirror->path);
-        if (def->mirror->format)
-            virBufferAsprintf(buf, " format='%s'",
-                              virStorageFileFormatTypeToString(def->mirror->format));
+        virBufferAsprintf(buf, "<mirror type='%s'",
+                          virStorageTypeToString(def->mirror->type));
         if (def->mirroring)
             virBufferAddLit(buf, " ready='yes'");
-        virBufferAddLit(buf, "/>\n");
+        virBufferAddLit(buf, ">\n");
+        virBufferAdjustIndent(buf, 2);
+        if (def->mirror->format)
+            virBufferEscapeString(buf, "<format type='%s'/>\n",
+                                  virStorageFileFormatTypeToString(def->mirror->format));
+        if (virDomainDiskSourceFormat(buf, def->mirror, 0, 0) < 0)
+            return -1;
+        virBufferAdjustIndent(buf, -2);
+        virBufferAddLit(buf, "</mirror>\n");
     }
 
     virBufferAsprintf(buf, "<target dev='%s' bus='%s'",

tests/qemuxml2argvdata/qemuxml2argv-disk-mirror-old.xml

@@ -0,0 +1,47 @@
+<domain type='qemu' id='1'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu</emulator>
+    <disk type='block' device='disk'>
+      <source dev='/dev/HostVG/QEMUGuest1'/>
+      <backingStore/>
+      <mirror file='/dev/HostVG/QEMUGuest1Copy' ready='yes'/>
+      <target dev='hda' bus='ide'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <disk type='block' device='cdrom'>
+      <source dev='/dev/HostVG/QEMUGuest2'/>
+      <backingStore/>
+      <target dev='hdc' bus='ide'/>
+      <readonly/>
+      <address type='drive' controller='0' bus='1' target='0' unit='0'/>
+    </disk>
+    <disk type='file' device='disk'>
+      <source file='/tmp/data.img'/>
+      <backingStore/>
+      <mirror file='/tmp/copy.img' format='qcow2'/>
+      <target dev='vda' bus='virtio'/>
+    </disk>
+    <disk type='file' device='disk'>
+      <source file='/tmp/logs.img'/>
+      <backingStore/>
+      <target dev='vdb' bus='virtio'/>
+    </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/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml

@@ -17,7 +17,9 @@
     <disk type='block' device='disk'>
       <source dev='/dev/HostVG/QEMUGuest1'/>
       <backingStore/>
-      <mirror file='/dev/HostVG/QEMUGuest1Copy' ready='yes'/>
+      <mirror type='file' ready='yes'>
+        <source file='/dev/HostVG/QEMUGuest1Copy'/>
+      </mirror>
       <target dev='hda' bus='ide'/>
       <address type='drive' controller='0' bus='0' target='0' unit='0'/>
     </disk>
@@ -31,7 +33,10 @@
     <disk type='file' device='disk'>
       <source file='/tmp/data.img'/>
       <backingStore/>
-      <mirror file='/tmp/copy.img' format='qcow2'/>
+      <mirror type='file'>
+        <format type='qcow2'/>
+        <source file='/tmp/copy.img'/>
+      </mirror>
       <target dev='vda' bus='virtio'/>
     </disk>
     <disk type='file' device='disk'>

tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old-inactive.xml

@@ -0,0 +1,41 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu</emulator>
+    <disk type='block' device='disk'>
+      <source dev='/dev/HostVG/QEMUGuest1'/>
+      <target dev='hda' bus='ide'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <disk type='block' device='cdrom'>
+      <source dev='/dev/HostVG/QEMUGuest2'/>
+      <target dev='hdc' bus='ide'/>
+      <readonly/>
+      <address type='drive' controller='0' bus='1' target='0' unit='0'/>
+    </disk>
+    <disk type='file' device='disk'>
+      <source file='/tmp/data.img'/>
+      <target dev='vda' bus='virtio'/>
+    </disk>
+    <disk type='file' device='disk'>
+      <source file='/tmp/logs.img'/>
+      <target dev='vdb' bus='virtio'/>
+    </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/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml

@@ -0,0 +1,52 @@
+<domain type='qemu' id='1'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu</emulator>
+    <disk type='block' device='disk'>
+      <source dev='/dev/HostVG/QEMUGuest1'/>
+      <backingStore/>
+      <mirror type='file' ready='yes'>
+        <source file='/dev/HostVG/QEMUGuest1Copy'/>
+      </mirror>
+      <target dev='hda' bus='ide'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <disk type='block' device='cdrom'>
+      <source dev='/dev/HostVG/QEMUGuest2'/>
+      <backingStore/>
+      <target dev='hdc' bus='ide'/>
+      <readonly/>
+      <address type='drive' controller='0' bus='1' target='0' unit='0'/>
+    </disk>
+    <disk type='file' device='disk'>
+      <source file='/tmp/data.img'/>
+      <backingStore/>
+      <mirror type='file'>
+        <format type='qcow2'/>
+        <source file='/tmp/copy.img'/>
+      </mirror>
+      <target dev='vda' bus='virtio'/>
+    </disk>
+    <disk type='file' device='disk'>
+      <source file='/tmp/logs.img'/>
+      <backingStore/>
+      <target dev='vdb' bus='virtio'/>
+    </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/qemuxml2xmltest.c

@@ -224,6 +224,7 @@ mymain(void)
     DO_TEST("disk-scsi-virtio-scsi");
     DO_TEST("disk-virtio-scsi-num_queues");
     DO_TEST("disk-scsi-megasas");
+    DO_TEST_DIFFERENT("disk-mirror-old");
     DO_TEST_FULL("disk-mirror", false, WHEN_ACTIVE);
     DO_TEST_FULL("disk-mirror", true, WHEN_INACTIVE);
     DO_TEST("graphics-listen-network");