2:47 p.m.
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(a)redhat.com>
---
I have to rebase my remaining active commit series on top of this,
but wanted to get this out now so that John can test if this is
sufficient to resolve the test failure without having to modify
things in the automated tester.
docs/formatdomain.html.in | 12 +++++++-----
docs/schemas/domaincommon.rng | 12 ++++++++++--
src/conf/domain_conf.c | 19 +++++++++++++------
tests/qemuxml2argvdata/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 1b6ced8..13ba541 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 6cc922c..33d0308 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 be81dbe..4114289 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 a937d0a..72b03c9 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 a937d0a..72b03c9 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>
--
1.9.3