From 7c6fc3948e9439141aeb7623450d9369dbc187d5 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Wed, 21 May 2014 22:39:57 -0600 Subject: [PATCH] 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 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 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 --- docs/formatdomain.html.in | 20 ++++-- docs/schemas/domaincommon.rng | 29 +++++--- src/conf/domain_conf.c | 69 ++++++++++++++----- .../qemuxml2argv-disk-mirror-old.xml | 47 +++++++++++++ .../qemuxml2argv-disk-mirror.xml | 9 ++- ...emuxml2xmlout-disk-mirror-old-inactive.xml | 41 +++++++++++ .../qemuxml2xmlout-disk-mirror-old.xml | 52 ++++++++++++++ tests/qemuxml2xmltest.c | 1 + 8 files changed, 236 insertions(+), 32 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/formatdomain.html.in b/docs/formatdomain.html.in index 691a451c55..1b6ced87c5 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1879,15 +1879,23 @@
mirror
This element is present if the hypervisor has started a block - copy operation (via the virDomainBlockCopy API), - where the mirror location in attribute file will - eventually have the same contents as the source, and with the - file format in attribute format (which might - differ from the format of the source). If + copy operation (via the virDomainBlockRebase + API), where the mirror location in the source + sub-element will eventually have the same contents as the + source, and with the file format in the + sub-element format (which might differ from the + format of the source). The details of the source + sub-element are determined by the type attribute + of the mirror, similar to what is done for the + overall disk device element. If attribute ready 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. Since 0.9.12 + ignored on input. Since 1.2.6 + (Older libvirt since 0.9.12 allowed + only mirroring to a file, with the information reported + in file and format attributes + of mirror rather than subelements.)
target
The target element controls the bus / device diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index af671235d3..6cc922c2a3 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4172,16 +4172,29 @@ + - - - - - - - - + + + + + + + + + + + + + + + + + + + + yes diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9c6c201ac0..ff2d447345 100644 --- 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->path); - if (def->mirror->format) - virBufferAsprintf(buf, " format='%s'", - virStorageFileFormatTypeToString(def->mirror->format)); + virBufferAsprintf(buf, "mirror->type)); if (def->mirroring) virBufferAddLit(buf, " ready='yes'"); - virBufferAddLit(buf, "/>\n"); + virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); + if (def->mirror->format) + virBufferEscapeString(buf, "\n", + virStorageFileFormatTypeToString(def->mirror->format)); + if (virDomainDiskSourceFormat(buf, def->mirror, 0, 0) < 0) + return -1; + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "\n"); } virBufferAsprintf(buf, " + QEMUGuest1 + c7a5fdbd-edaf-9455-926a-d65c16db1809 + 219136 + 219136 + 1 + + hvm + + + + destroy + restart + destroy + + /usr/bin/qemu + + + + + +
+ + + + + + +
+ + + + + + + + + + + + + + + + + + diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml index faa0b8c4a1..a937d0a5ca 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml @@ -17,7 +17,9 @@ - + + +
@@ -31,7 +33,10 @@ - + + + + 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 0000000000..b3d8c59dc4 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old-inactive.xml @@ -0,0 +1,41 @@ + + QEMUGuest1 + c7a5fdbd-edaf-9455-926a-d65c16db1809 + 219136 + 219136 + 1 + + hvm + + + + destroy + restart + destroy + + /usr/bin/qemu + + + +
+ + + + + +
+ + + + + + + + + + + + + + + diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml new file mode 100644 index 0000000000..a937d0a5ca --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml @@ -0,0 +1,52 @@ + + QEMUGuest1 + c7a5fdbd-edaf-9455-926a-d65c16db1809 + 219136 + 219136 + 1 + + hvm + + + + destroy + restart + destroy + + /usr/bin/qemu + + + + + + + +
+ + + + + + +
+ + + + + + + + + + + + + + + + + + + + + diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index da528dadf0..200d50fd63 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");