11:31 a.m.
On 07/29/2014 05:20 AM, Peter Krempa wrote:
On 07/29/14 06:20, Eric Blake wrote:
> Doing a blockcopy operation across a libvirtd restart is not very
> robust at the moment. In particular, we are clearing the <mirror>
> element prior to telling qemu to finish the job; and 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.
>
>
> +typedef enum {
> + VIR_DOMAIN_DISK_MIRROR_DEFAULT = 0, /* No job, or job still not synced */
I'd go with _NONE here and ...
> + VIR_DOMAIN_DISK_MIRROR_READY, /* Job in second phase */
> + VIR_DOMAIN_DISK_MIRROR_ABORT, /* Job aborted, waiting for event */
> + VIR_DOMAIN_DISK_MIRROR_PIVOT, /* Job pivoted, waiting for event */
> +
> + VIR_DOMAIN_DISK_MIRROR_LAST
> +} virDomainDiskMirror;
... as this describes the ready state of the mirroring operation I'd go
with virDomainDiskMirrorState and rename those fields accordingly.
Without that it's not clear what the enum describes.
Otherwise looks good. ACK if you change the enum name and default
value
name.
I'm squashing this in, and pushing.
diff --git i/src/conf/domain_conf.c w/src/conf/domain_conf.c
index 1c8f8cf..c2a81f1 100644
--- i/src/conf/domain_conf.c
+++ w/src/conf/domain_conf.c
@@ -747,8 +747,8 @@ VIR_ENUM_IMPL(virDomainDiskDiscard,
VIR_DOMAIN_DISK_DISCARD_LAST,
"unmap",
"ignore")
-VIR_ENUM_IMPL(virDomainDiskMirror, VIR_DOMAIN_DISK_MIRROR_LAST,
- "default",
+VIR_ENUM_IMPL(virDomainDiskMirrorState, VIR_DOMAIN_DISK_MIRROR_STATE_LAST,
+ "none",
"yes",
"abort",
"pivot")
@@ -5488,8 +5488,8 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
}
ready = virXMLPropString(cur, "ready");
if (ready) {
- def->mirrorState =
virDomainDiskMirrorTypeFromString(ready);
- if (def->mirrorState < 0) {
+ if ((def->mirrorState =
+ virDomainDiskMirrorStateTypeFromString(ready))
< 0) {
virReportError(VIR_ERR_XML_ERROR,
_("unknown mirror ready state %s"),
ready);
@@ -15289,7 +15289,7 @@ virDomainDiskDefFormat(virBufferPtr buf,
if (def->mirrorState) {
const char *mirror;
- mirror = virDomainDiskMirrorTypeToString(def->mirrorState);
+ mirror =
virDomainDiskMirrorStateTypeToString(def->mirrorState);
virBufferEscapeString(buf, " ready='%s'", mirror);
}
virBufferAddLit(buf, ">\n");
diff --git i/src/conf/domain_conf.h w/src/conf/domain_conf.h
index bc8966d..2df3ae2 100644
--- i/src/conf/domain_conf.h
+++ w/src/conf/domain_conf.h
@@ -611,13 +611,13 @@ typedef virDomainBlockIoTuneInfo
*virDomainBlockIoTuneInfoPtr;
typedef enum {
- VIR_DOMAIN_DISK_MIRROR_DEFAULT = 0, /* No job, or job still not
synced */
- VIR_DOMAIN_DISK_MIRROR_READY, /* Job in second phase */
- VIR_DOMAIN_DISK_MIRROR_ABORT, /* Job aborted, waiting for event */
- VIR_DOMAIN_DISK_MIRROR_PIVOT, /* Job pivoted, waiting for event */
+ 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_LAST
-} virDomainDiskMirror;
+ VIR_DOMAIN_DISK_MIRROR_STATE_LAST
+} virDomainDiskMirrorState;
/* Stores the virtual disk configuration */
@@ -631,7 +631,7 @@ struct _virDomainDiskDef {
int removable; /* enum virTristateSwitch */
virStorageSourcePtr mirror;
- int mirrorState; /* enum virDomainDiskMirror */
+ int mirrorState; /* enum virDomainDiskMirrorState */
struct {
unsigned int cylinders;
@@ -2566,7 +2566,7 @@ VIR_ENUM_DECL(virDomainDiskIo)
VIR_ENUM_DECL(virDomainDeviceSGIO)
VIR_ENUM_DECL(virDomainDiskTray)
VIR_ENUM_DECL(virDomainDiskDiscard)
-VIR_ENUM_DECL(virDomainDiskMirror)
+VIR_ENUM_DECL(virDomainDiskMirrorState)
VIR_ENUM_DECL(virDomainController)
VIR_ENUM_DECL(virDomainControllerModelPCI)
VIR_ENUM_DECL(virDomainControllerModelSCSI)
diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index acc9ef0..836adba 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -14860,10 +14860,10 @@ qemuDomainBlockPivot(virConnectPtr conn,
}
if (rc == 1 && info.cur == info.end &&
info.type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY)
- disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_READY;
+ disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY;
}
- if (disk->mirrorState != VIR_DOMAIN_DISK_MIRROR_READY) {
+ if (disk->mirrorState != VIR_DOMAIN_DISK_MIRROR_STATE_READY) {
virReportError(VIR_ERR_BLOCK_COPY_ACTIVE,
_("disk '%s' not ready for pivot yet"),
disk->dst);
@@ -14939,7 +14939,7 @@ qemuDomainBlockPivot(virConnectPtr conn,
}
disk->mirror = NULL;
- disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_DEFAULT;
+ disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
cleanup:
/* revert to original disk def on failure */
@@ -15096,7 +15096,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->mirrorState = VIR_DOMAIN_DISK_MIRROR_READY;
+ disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY;
/* A successful block job cancelation stops any mirroring. */
if (mode == BLOCK_JOB_ABORT && disk->mirror) {
@@ -15104,7 +15104,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
* the mirror, and audit that fact, before dropping things. */
virStorageSourceFree(disk->mirror);
disk->mirror = NULL;
- disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_DEFAULT;
+ disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
}
waitjob:
diff --git i/src/qemu/qemu_process.c w/src/qemu/qemu_process.c
index 73ad27d..378c6d3 100644
--- i/src/qemu/qemu_process.c
+++ w/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->mirrorState = VIR_DOMAIN_DISK_MIRROR_READY;
+ disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY;
} else if (status == VIR_DOMAIN_BLOCK_JOB_FAILED) {
virStorageSourceFree(disk->mirror);
disk->mirror = NULL;
- disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_DEFAULT;
+ disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
}
}
}
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org