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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list