11:50 p.m.
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.
TODO: this patch still needs to update formatdomain.html.in to
document the new output, as well as provide more justification
why we can drop the old output without breaking things (or
alternatively, we must continue to provide the old output in
parallel with type='file').
* 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(a)redhat.com>
---
docs/schemas/domaincommon.rng | 29 ++++++---
src/conf/domain_conf.c | 69 ++++++++++++++++------
.../qemuxml2argv-disk-mirror-old.xml | 47 +++++++++++++++
.../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml | 9 ++-
.../qemuxml2xmlout-disk-mirror-old-inactive.xml | 41 +++++++++++++
.../qemuxml2xmlout-disk-mirror-old.xml | 52 ++++++++++++++++
tests/qemuxml2xmltest.c | 1 +
7 files changed, 221 insertions(+), 27 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-mirror-old.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old-inactive.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4249ed5..7de5832 100644
--- 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 57142cb..f477c19 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5218,6 +5218,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;
@@ -5354,18 +5356,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 cleanup;
+ }
+ 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);
@@ -5373,10 +5391,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) {
@@ -5962,6 +5989,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;
@@ -15134,15 +15163,21 @@ 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);
+ virBufferAsprintf(buf, "<mirror type='%s'%s>\n",
+ virStorageTypeToString(def->mirror->type),
+ def->mirroring ? " ready='yes'" :
"");
+ virBufferAdjustIndent(buf, 2);
+ if (virDomainDiskSourceFormat(buf, def->mirror, 0, 0) < 0)
+ return -1;
if (def->mirror->format)
- virBufferAsprintf(buf, " format='%s'",
-
virStorageFileFormatTypeToString(def->mirror->format));
- if (def->mirroring)
- virBufferAddLit(buf, " ready='yes'");
- virBufferAddLit(buf, "/>\n");
+ virBufferEscapeString(buf, "<format type='%s'/>\n",
+
virStorageFileFormatTypeToString(def->mirror->format));
+ 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
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 faa0b8c..338e3c3 100644
--- 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'>
+ <source file='/tmp/copy.img'/>
+ <format type='qcow2'/>
+ </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
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
index 0000000..338e3c3
--- /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'>
+ <source file='/tmp/copy.img'/>
+ <format type='qcow2'/>
+ </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 da528da..200d50f 100644
--- 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");
--
1.9.0