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>

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 691a451c551de0605170931ff44d51a66913f46c..1b6ced87c532fa9984ec99adcd602f65f07d5798 100644 (file)
--- a/docs/formatdomain.html.in
+++ b/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

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index af671235d3a9d42c3b6a9941efd11903fc82bc91..6cc922c2a328640e54fb560816078e4796f15b4b 100644 (file)
--- a/docs/schemas/domaincommon.rng
+++ b/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>

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9c6c201ac0e6a44b82735b16f7b04e17ffe9fab9..ff2d447345c5b16e3292cbc130e192d7aee6cf1b 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/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'",

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror-old.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror-old.xml
new file mode 100644 (file)
index 0000000..faa0b8c
--- /dev/null
+++ b/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>

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml
index faa0b8c4a1559d16e1904696ecdb53fdb8a4b1b7..a937d0a5cad5fd2e82b9ea65ff80c13273204052 100644 (file)
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml
+++ b/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'>

diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old-inactive.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old-inactive.xml
new file mode 100644 (file)
index 0000000..b3d8c59
--- /dev/null
+++ b/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>

diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml
new file mode 100644 (file)
index 0000000..a937d0a
--- /dev/null
+++ b/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>

diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index da528dadf0dcab45e8c6bf96e04c2317246a9a04..200d50fd63f7734c702159a54d7ad26fbb6b4cf9 100644 (file)
--- a/tests/qemuxml2xmltest.c
+++ b/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");