From b50e1049234473b36e798b8ebd842cb84ef023dc Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Mon, 16 Jun 2014 13:34:36 -0600
Subject: [PATCH] blockjob: don't remove older-style mirror XML

Commit 7c6fc39 introduced a regression in the XML produced for older
clients.  The argument at the time was that clients shouldn't be
depending on output-only data for something that is only going to
be triggered for a transient guest; but John Ferlan reported that
the automated testsuite was such a client.  It's better to be safe
than sorry by guaranteeing back-compat cruft.  Note that later
patches will be using <mirror> for active block commit, but there
we don't have to worry about back-compat.

* src/conf/domain_conf.c (virDomainDiskDefFormat): Restore old
style output when necessary.
* docs/schemas/domaincommon.rng: Validate back-compat style.
* docs/formatdomain.html.in: Update the documentation.
* tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml:
Update tests.
* tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml: Likewise.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 docs/formatdomain.html.in                     | 12 +++++++-----
 docs/schemas/domaincommon.rng                 | 12 ++++++++++--
 src/conf/domain_conf.c                        | 19 +++++++++++++------
 .../qemuxml2argv-disk-mirror.xml              |  4 ++--
 .../qemuxml2xmlout-disk-mirror-old.xml        |  4 ++--
 5 files changed, 34 insertions(+), 17 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 1b6ced87c5..79b85d5f0d 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1891,11 +1891,13 @@
         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 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.)
+        ignored on input.  The <code>source</code> sub-element exists
+        for all two-phase jobs <span class="since">since 1.2.6</span>.
+        Older libvirt supported only block copy to a
+        file, <span class="since">since 0.9.12</span>; for
+        compatibility with older clients, such jobs include redundant
+        information in the attributes <code>file</code>
+        and <code>format</code> in the <code>mirror</code> element.
       </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 6cc922c2a3..33d0308cf9 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4176,7 +4176,7 @@
   <define name='diskMirror'>
     <element name='mirror'>
       <choice>
-        <group>
+        <group> <!-- old format, for block copy back-compat -->
           <attribute name='file'>
             <ref name='absFilePath'/>
           </attribute>
@@ -4185,8 +4185,16 @@
               <ref name='storageFormat'/>
             </attribute>
           </optional>
+          <optional>
+            <interleave>
+              <ref name='diskSourceFile'/>
+              <optional>
+                <ref name="diskFormat"/>
+              </optional>
+            </interleave>
+          </optional>
         </group>
-        <group>
+        <group> <!-- preferred format -->
           <interleave>
             <ref name="diskSource"/>
             <optional>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index be81dbec85..41142897a1 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15187,19 +15187,26 @@ 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.  We only output the
-     * new style similar to backingStore, even though the parser
-     * code still accepts old style across libvirtd upgrades. */
+     * the internal parse on libvirtd restart.  We prefer to output
+     * the new style similar to backingStore, but for back-compat on
+     * blockcopy files we also have to output old style attributes.
+     * The parser accepts either style across libvirtd upgrades. */
     if (def->mirror && !(flags & VIR_DOMAIN_XML_INACTIVE)) {
+        const char *formatStr = NULL;
+
+        if (def->mirror->format)
+            formatStr = virStorageFileFormatTypeToString(def->mirror->format);
         virBufferAsprintf(buf, "<mirror type='%s'",
                           virStorageTypeToString(def->mirror->type));
+        if (def->mirror->type == VIR_STORAGE_TYPE_FILE) {
+            virBufferEscapeString(buf, " file='%s'", def->mirror->path);
+            virBufferEscapeString(buf, " format='%s'", formatStr);
+        }
         if (def->mirroring)
             virBufferAddLit(buf, " ready='yes'");
         virBufferAddLit(buf, ">\n");
         virBufferAdjustIndent(buf, 2);
-        if (def->mirror->format)
-            virBufferEscapeString(buf, "<format type='%s'/>\n",
-                                  virStorageFileFormatTypeToString(def->mirror->format));
+        virBufferEscapeString(buf, "<format type='%s'/>\n", formatStr);
         if (virDomainDiskSourceFormat(buf, def->mirror, 0, 0) < 0)
             return -1;
         virBufferAdjustIndent(buf, -2);
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml
index a937d0a5ca..72b03c96b0 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml
@@ -17,7 +17,7 @@
     <disk type='block' device='disk'>
       <source dev='/dev/HostVG/QEMUGuest1'/>
       <backingStore/>
-      <mirror type='file' ready='yes'>
+      <mirror type='file' file='/dev/HostVG/QEMUGuest1Copy' ready='yes'>
         <source file='/dev/HostVG/QEMUGuest1Copy'/>
       </mirror>
       <target dev='hda' bus='ide'/>
@@ -33,7 +33,7 @@
     <disk type='file' device='disk'>
       <source file='/tmp/data.img'/>
       <backingStore/>
-      <mirror type='file'>
+      <mirror type='file' file='/tmp/copy.img' format='qcow2'>
         <format type='qcow2'/>
         <source file='/tmp/copy.img'/>
       </mirror>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml
index a937d0a5ca..72b03c96b0 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml
@@ -17,7 +17,7 @@
     <disk type='block' device='disk'>
       <source dev='/dev/HostVG/QEMUGuest1'/>
       <backingStore/>
-      <mirror type='file' ready='yes'>
+      <mirror type='file' file='/dev/HostVG/QEMUGuest1Copy' ready='yes'>
         <source file='/dev/HostVG/QEMUGuest1Copy'/>
       </mirror>
       <target dev='hda' bus='ide'/>
@@ -33,7 +33,7 @@
     <disk type='file' device='disk'>
       <source file='/tmp/data.img'/>
       <backingStore/>
-      <mirror type='file'>
+      <mirror type='file' file='/tmp/copy.img' format='qcow2'>
         <format type='qcow2'/>
         <source file='/tmp/copy.img'/>
       </mirror>