From ae6aa8c3965e9aaa245b8e669c6324d44312ac1b Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Wed, 28 Mar 2012 18:10:18 -0600 Subject: [PATCH] blockjob: enhance xml to track mirrors across libvirtd restart In order to track a block copy job across libvirtd restarts, we need to save internal XML that tracks the name of the file holding the mirror. Displaying this name in dumpxml might also be useful to the user, even if we don't yet have a way to (re-) start a domain with mirroring enabled up front. This is done with a new sub-element to , as in: ... For now, the element is output-only, in live domains; it is ignored when defining a domain or hot-plugging a disk (since those contexts use VIR_DOMAIN_XML_INACTIVE in parsing). The 'ready' attribute appears when libvirt knows that the job has changed from the initial pulling phase over to the mirroring phase, although absence of the attribute is not a sure indicator of the current phase. If we come up with a way to make qemu start with mirroring enabled, we can relax the xml restriction, and allow (but not attribute 'ready') on input. Testing active-only XML meant tweaking the testsuite slightly, but it was worth it. * docs/schemas/domaincommon.rng (diskspec): Add diskMirror. * docs/formatdomain.html.in (elementsDisks): Document it. * src/conf/domain_conf.h (_virDomainDiskDef): New members. * src/conf/domain_conf.c (virDomainDiskDefFree): Clean them. (virDomainDiskDefParseXML): Parse them, but only internally. (virDomainDiskDefFormat): Output them. * tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml: New test file. * tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror.xml: Likewise. * tests/qemuxml2xmltest.c (testInfo): Alter members. (testCompareXMLToXMLHelper): Allow more test control. (mymain): Run new test. --- docs/formatdomain.html.in | 13 ++++++ docs/schemas/domaincommon.rng | 24 ++++++++-- src/conf/domain_conf.c | 45 +++++++++++++++++++ src/conf/domain_conf.h | 4 ++ .../qemuxml2argv-disk-mirror.xml | 42 +++++++++++++++++ .../qemuxml2xmlout-disk-mirror.xml | 40 +++++++++++++++++ tests/qemuxml2xmltest.c | 42 +++++++++-------- 7 files changed, 189 insertions(+), 21 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 6f19da05a5..e1fe0c4a17 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1296,6 +1296,19 @@ Since 0.9.7 +
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 + 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 +
target
The target element controls the bus / device under which the disk is exposed to the guest diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index f116710f34..8419ccc4e7 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -771,6 +771,9 @@ + + + @@ -1013,9 +1016,7 @@ @@ -3018,6 +3019,23 @@ + + + + + + + + + + + + + yes + + + + diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ac877f9676..184ff2391a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -933,6 +933,8 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def) VIR_FREE(def->dst); VIR_FREE(def->driverName); VIR_FREE(def->driverType); + VIR_FREE(def->mirror); + VIR_FREE(def->mirrorFormat); VIR_FREE(def->auth.username); if (def->auth.secretType == VIR_DOMAIN_DISK_SECRET_TYPE_USAGE) VIR_FREE(def->auth.secret.usage); @@ -3319,6 +3321,9 @@ virDomainDiskDefParseXML(virCapsPtr caps, char *ioeventfd = NULL; char *event_idx = NULL; char *copy_on_read = NULL; + char *mirror = NULL; + char *mirrorFormat = NULL; + bool mirroring = false; char *devaddr = NULL; virStorageEncryptionPtr encryption = NULL; char *serial = NULL; @@ -3454,6 +3459,21 @@ virDomainDiskDefParseXML(virCapsPtr caps, ioeventfd = virXMLPropString(cur, "ioeventfd"); event_idx = virXMLPropString(cur, "event_idx"); copy_on_read = virXMLPropString(cur, "copy_on_read"); + } else if (!mirror && xmlStrEqual(cur->name, BAD_CAST "mirror") && + !(flags & VIR_DOMAIN_XML_INACTIVE)) { + char *ready; + mirror = virXMLPropString(cur, "file"); + if (!mirror) { + virDomainReportError(VIR_ERR_XML_ERROR, "%s", + _("mirror requires file name")); + goto error; + } + mirrorFormat = virXMLPropString(cur, "format"); + ready = virXMLPropString(cur, "ready"); + if (ready) { + mirroring = true; + VIR_FREE(ready); + } } else if (xmlStrEqual(cur->name, BAD_CAST "auth")) { authUsername = virXMLPropString(cur, "username"); if (authUsername == NULL) { @@ -3868,6 +3888,11 @@ virDomainDiskDefParseXML(virCapsPtr caps, driverName = NULL; def->driverType = driverType; driverType = NULL; + def->mirror = mirror; + mirror = NULL; + def->mirrorFormat = mirrorFormat; + mirrorFormat = NULL; + def->mirroring = mirroring; def->encryption = encryption; encryption = NULL; def->serial = serial; @@ -3883,6 +3908,12 @@ virDomainDiskDefParseXML(virCapsPtr caps, !(def->driverName = strdup(caps->defaultDiskDriverName))) goto no_memory; + + if (def->mirror && !def->mirrorFormat && + caps->defaultDiskDriverType && + !(def->mirrorFormat = strdup(caps->defaultDiskDriverType))) + goto no_memory; + if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && virDomainDiskDefAssignAddress(caps, def) < 0) goto error; @@ -3908,6 +3939,8 @@ cleanup: VIR_FREE(authUsage); VIR_FREE(driverType); VIR_FREE(driverName); + VIR_FREE(mirror); + VIR_FREE(mirrorFormat); VIR_FREE(cachetag); VIR_FREE(error_policy); VIR_FREE(rerror_policy); @@ -10839,6 +10872,18 @@ 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. */ + if (def->mirror && !(flags & VIR_DOMAIN_XML_INACTIVE)) { + virBufferEscapeString(buf, " mirror); + if (def->mirrorFormat) + virBufferAsprintf(buf, " format='%s'", def->mirrorFormat); + if (def->mirroring) + virBufferAddLit(buf, " ready='yes'"); + virBufferAddLit(buf, "/>\n"); + } + virBufferAsprintf(buf, " dst, bus); if ((def->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY || diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0eed60e02a..5aa8fc1530 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -563,6 +563,10 @@ struct _virDomainDiskDef { char *driverName; char *driverType; + char *mirror; + char *mirrorFormat; + bool mirroring; + virDomainBlockIoTuneInfo blkdeviotune; char *serial; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml new file mode 100644 index 0000000000..0c9572429f --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml @@ -0,0 +1,42 @@ + + QEMUGuest1 + c7a5fdbd-edaf-9455-926a-d65c16db1809 + 219136 + 219136 + 1 + + hvm + + + + destroy + restart + destroy + + /usr/bin/qemu + + + + +
+ + + + + +
+ + + + + + + + + + + + + + + diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror.xml new file mode 100644 index 0000000000..00111bef58 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror.xml @@ -0,0 +1,40 @@ + + 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 42e4d9df58..9bca066e64 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -54,10 +54,16 @@ testCompareXMLToXMLFiles(const char *inxml, const char *outxml, bool live) return ret; } +enum { + WHEN_INACTIVE = 1, + WHEN_ACTIVE = 2, + WHEN_EITHER = 3, +}; + struct testInfo { const char *name; - int different; - bool inactive_only; + bool different; + int when; }; static int @@ -74,17 +80,15 @@ testCompareXMLToXMLHelper(const void *data) abs_srcdir, info->name) < 0) goto cleanup; - if (info->different) { - ret = testCompareXMLToXMLFiles(xml_in, xml_out, false); - } else { - ret = testCompareXMLToXMLFiles(xml_in, xml_in, false); + if (info->when & WHEN_INACTIVE) { + ret = testCompareXMLToXMLFiles(xml_in, + info->different ? xml_out : xml_in, + false); } - if (!info->inactive_only) { - if (info->different) { - ret = testCompareXMLToXMLFiles(xml_in, xml_out, true); - } else { - ret = testCompareXMLToXMLFiles(xml_in, xml_in, true); - } + if (info->when & WHEN_ACTIVE) { + ret = testCompareXMLToXMLFiles(xml_in, + info->different ? xml_out : xml_in, + true); } cleanup: @@ -102,19 +106,19 @@ mymain(void) if ((driver.caps = testQemuCapsInit()) == NULL) return EXIT_FAILURE; -# define DO_TEST_FULL(name, is_different, inactive) \ +# define DO_TEST_FULL(name, is_different, when) \ do { \ - const struct testInfo info = {name, is_different, inactive}; \ + const struct testInfo info = {name, is_different, when}; \ if (virtTestRun("QEMU XML-2-XML " name, \ 1, testCompareXMLToXMLHelper, &info) < 0) \ ret = -1; \ } while (0) # define DO_TEST(name) \ - DO_TEST_FULL(name, 0, false) + DO_TEST_FULL(name, false, WHEN_EITHER) # define DO_TEST_DIFFERENT(name) \ - DO_TEST_FULL(name, 1, false) + DO_TEST_FULL(name, true, WHEN_EITHER) /* Unset or set all envvars here that are copied in qemudBuildCommandLine * using ADD_ENV_COPY, otherwise these tests may fail due to unexpected @@ -151,6 +155,8 @@ mymain(void) DO_TEST("disk-scsi-device"); DO_TEST("disk-scsi-vscsi"); DO_TEST("disk-scsi-virtio-scsi"); + DO_TEST_FULL("disk-mirror", false, WHEN_ACTIVE); + DO_TEST_FULL("disk-mirror", true, WHEN_INACTIVE); DO_TEST("graphics-listen-network"); DO_TEST("graphics-vnc"); DO_TEST("graphics-vnc-sasl"); @@ -208,8 +214,8 @@ mymain(void) DO_TEST("usb-redir"); DO_TEST("blkdeviotune"); - DO_TEST_FULL("seclabel-dynamic-baselabel", false, true); - DO_TEST_FULL("seclabel-dynamic-override", false, true); + DO_TEST_FULL("seclabel-dynamic-baselabel", false, WHEN_INACTIVE); + DO_TEST_FULL("seclabel-dynamic-override", false, WHEN_INACTIVE); DO_TEST("seclabel-static"); DO_TEST("seclabel-none");