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@xxxxxxxxxx> --- 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list