From 9a212d6708b7d4dc33a78f1887445a9ac73ef9aa Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Mon, 28 Jul 2014 16:25:28 -0600 Subject: [PATCH] blockcopy: add more XML for state tracking Doing a blockcopy operation across a libvirtd restart is not very robust at the moment. In particular, we are clearing the element prior to telling qemu to finish the job. Also, thanks to the ability to request async completion, the user can easily regain control prior to qemu actually finishing the effort, and they should be able to poll the domain XML to see if the job is still going. A future patch will fix things to actually wait until qemu is done before modifying the XML to reflect the job completion. But since qemu issues identical BLOCK_JOB_COMPLETE events regardless of whether the job was cancelled (kept the original disk) or completed (pivoted to the new disk), we have to track which of the two operations were used to end the job. Furthermore, we'd like to avoid attempts to end a job where we are already waiting on an earlier request to qemu to end the job. Likewise, if we miss the qemu event (perhaps because it arrived during a libvirtd restart), we still need enough state recorded to be able to determine how to modify the domain XML once we reconnect to qemu and manually learn whether the job still exists. Although this patch doesn't actually fix the problem, it is a preliminary step that makes it possible to track whether a job has already begun steps towards completion. * src/conf/domain_conf.h (virDomainDiskMirrorState): New enum. (_virDomainDiskDef): Convert bool mirroring to new enum. * src/conf/domain_conf.c (virDomainDiskDefParseXML) (virDomainDiskDefFormat): Handle new values. * src/qemu/qemu_process.c (qemuProcessHandleBlockJob): Adjust client. * src/qemu/qemu_driver.c (qemuDomainBlockPivot) (qemuDomainBlockJobImpl): Likewise. * docs/schemas/domaincommon.rng (diskMirror): Expose new values. * docs/formatdomain.html.in (elementsDisks): Document it. * tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml: Test it. Signed-off-by: Eric Blake --- docs/formatdomain.html.in | 10 +++++--- docs/schemas/domaincommon.rng | 6 ++++- src/conf/domain_conf.c | 23 ++++++++++++++++--- src/conf/domain_conf.h | 13 ++++++++++- src/qemu/qemu_driver.c | 12 +++++----- src/qemu/qemu_process.c | 4 ++-- .../qemuxml2argv-disk-mirror.xml | 4 ++++ 7 files changed, 56 insertions(+), 16 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index bce58859d1..2f0be20a6e 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1921,9 +1921,13 @@ 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 + overall disk device element. The + attribute ready, if present, tracks progress of + the job: yes if the disk is known to be ready to + pivot, or, since + 1.2.7, abort or pivot if the + job is in the process of completing. If ready is + not present, the disk is probably still copying. For now, this element only valid in output; it is ignored on input. The source sub-element exists for all two-phase jobs since 1.2.6. diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index aa0dcd5814..7cb5a9eb11 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4309,7 +4309,11 @@ - yes + + yes + abort + pivot + diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 65b1857f90..2a8cdeb03b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -747,6 +747,12 @@ VIR_ENUM_IMPL(virDomainDiskDiscard, VIR_DOMAIN_DISK_DISCARD_LAST, "unmap", "ignore") +VIR_ENUM_IMPL(virDomainDiskMirrorState, VIR_DOMAIN_DISK_MIRROR_STATE_LAST, + "none", + "yes", + "abort", + "pivot") + #define VIR_DOMAIN_XML_WRITE_FLAGS VIR_DOMAIN_XML_SECURE #define VIR_DOMAIN_XML_READ_FLAGS VIR_DOMAIN_XML_INACTIVE @@ -5482,7 +5488,14 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, } ready = virXMLPropString(cur, "ready"); if (ready) { - def->mirroring = true; + if ((def->mirrorState = + virDomainDiskMirrorStateTypeFromString(ready)) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("unknown mirror ready state %s"), + ready); + VIR_FREE(ready); + goto error; + } VIR_FREE(ready); } } else if (xmlStrEqual(cur->name, BAD_CAST "auth")) { @@ -15392,8 +15405,12 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferEscapeString(buf, " file='%s'", def->mirror->path); virBufferEscapeString(buf, " format='%s'", formatStr); } - if (def->mirroring) - virBufferAddLit(buf, " ready='yes'"); + if (def->mirrorState) { + const char *mirror; + + mirror = virDomainDiskMirrorStateTypeToString(def->mirrorState); + virBufferEscapeString(buf, " ready='%s'", mirror); + } virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); virBufferEscapeString(buf, "\n", formatStr); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 85e1da29ee..c77c85c470 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -610,6 +610,16 @@ struct _virDomainBlockIoTuneInfo { typedef virDomainBlockIoTuneInfo *virDomainBlockIoTuneInfoPtr; +typedef enum { + VIR_DOMAIN_DISK_MIRROR_STATE_NONE = 0, /* No job, or job still not synced */ + VIR_DOMAIN_DISK_MIRROR_STATE_READY, /* Job in second phase */ + VIR_DOMAIN_DISK_MIRROR_STATE_ABORT, /* Job aborted, waiting for event */ + VIR_DOMAIN_DISK_MIRROR_STATE_PIVOT, /* Job pivoted, waiting for event */ + + VIR_DOMAIN_DISK_MIRROR_STATE_LAST +} virDomainDiskMirrorState; + + /* Stores the virtual disk configuration */ struct _virDomainDiskDef { virStorageSourcePtr src; /* non-NULL. XXX Allow NULL for empty cdrom? */ @@ -621,7 +631,7 @@ struct _virDomainDiskDef { int removable; /* enum virTristateSwitch */ virStorageSourcePtr mirror; - bool mirroring; + int mirrorState; /* enum virDomainDiskMirrorState */ struct { unsigned int cylinders; @@ -2567,6 +2577,7 @@ VIR_ENUM_DECL(virDomainDiskIo) VIR_ENUM_DECL(virDomainDeviceSGIO) VIR_ENUM_DECL(virDomainDiskTray) VIR_ENUM_DECL(virDomainDiskDiscard) +VIR_ENUM_DECL(virDomainDiskMirrorState) VIR_ENUM_DECL(virDomainController) VIR_ENUM_DECL(virDomainControllerModelPCI) VIR_ENUM_DECL(virDomainControllerModelSCSI) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c807539473..b581d34565 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14841,7 +14841,7 @@ qemuDomainBlockPivot(virConnectPtr conn, format = virStorageFileFormatTypeToString(disk->mirror->format); /* Probe the status, if needed. */ - if (!disk->mirroring) { + if (!disk->mirrorState) { qemuDomainObjEnterMonitor(driver, vm); rc = qemuMonitorBlockJob(priv->mon, device, NULL, NULL, 0, &info, BLOCK_JOB_INFO, true); @@ -14855,10 +14855,10 @@ qemuDomainBlockPivot(virConnectPtr conn, } if (rc == 1 && info.cur == info.end && info.type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) - disk->mirroring = true; + disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY; } - if (!disk->mirroring) { + if (disk->mirrorState != VIR_DOMAIN_DISK_MIRROR_STATE_READY) { virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, _("disk '%s' not ready for pivot yet"), disk->dst); @@ -14934,7 +14934,7 @@ qemuDomainBlockPivot(virConnectPtr conn, } disk->mirror = NULL; - disk->mirroring = false; + disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; cleanup: /* revert to original disk def on failure */ @@ -15091,7 +15091,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, * avoid checking if pivot is safe. */ if (mode == BLOCK_JOB_INFO && ret == 1 && disk->mirror && info->cur == info->end && info->type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) - disk->mirroring = true; + disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY; /* A successful block job cancelation stops any mirroring. */ if (mode == BLOCK_JOB_ABORT && disk->mirror) { @@ -15099,7 +15099,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, * the mirror, and audit that fact, before dropping things. */ virStorageSourceFree(disk->mirror); disk->mirror = NULL; - disk->mirroring = false; + disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; } waitjob: diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 36922cbd26..c66326539e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1040,11 +1040,11 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, qemuDomainDetermineDiskChain(driver, vm, disk, true); if (disk->mirror && type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) { if (status == VIR_DOMAIN_BLOCK_JOB_READY) { - disk->mirroring = true; + disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY; } else if (status == VIR_DOMAIN_BLOCK_JOB_FAILED) { virStorageSourceFree(disk->mirror); disk->mirror = NULL; - disk->mirroring = false; + disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; } } } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml index 72b03c96b0..fa7af31d3f 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml @@ -42,6 +42,10 @@ + + + +