diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b581d34565..883f10d22d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14906,38 +14906,43 @@ qemuDomainBlockPivot(virConnectPtr conn,
virSecurityManagerSetDiskLabel(driver->securityManager, vm->def, disk) < 0))
goto cleanup;
- /* Attempt the pivot. */
+ /* Attempt the pivot. Record the attempt now, to prevent duplicate
+ * attempts; but the actual disk change will be made when emitting
+ * the event.
+ * XXX On libvirtd restarts, if we missed the qemu event, we need
+ * to double check what state qemu is in.
+ * XXX We should be using qemu's rerror flag to make sure the job
+ * remains alive until we know it's final state.
+ * XXX If the abort command is synchronous but the qemu event says
+ * that pivot failed, we need to reflect that failure into the
+ * overall return value. */
+ disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_PIVOT;
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorDrivePivot(priv->mon, device, disk->mirror->path, format);
qemuDomainObjExitMonitor(driver, vm);
- if (ret == 0) {
- /* XXX We want to revoke security labels and disk lease, as
- * well as audit that revocation, before dropping the original
- * source. But it gets tricky if both source and mirror share
- * common backing files (we want to only revoke the non-shared
- * portion of the chain, and is made more difficult by the
- * fact that we aren't tracking the full chain ourselves; so
- * for now, we leak the access to the original. */
- virStorageSourceFree(oldsrc);
- oldsrc = NULL;
- } else {
+ if (ret < 0) {
/* On failure, qemu abandons the mirror, and reverts back to
* the source disk (RHEL 6.3 has a bug where the revert could
* cause catastrophic failure in qemu, but we don't need to
* worry about it here as it is not an upstream qemu problem. */
/* XXX should we be parsing the exact qemu error, or calling
* 'query-block', to see what state we really got left in
- * before killing the mirroring job? And just as on the
- * success case, there's security labeling to worry about. */
+ * before killing the mirroring job?
+ * XXX We want to revoke security labels and disk lease, as
+ * well as audit that revocation, before dropping the original
+ * source. But it gets tricky if both source and mirror share
+ * common backing files (we want to only revoke the non-shared
+ * portion of the chain); so for now, we leak the access to
+ * the original. */
virStorageSourceFree(disk->mirror);
+ disk->mirror = NULL;
+ disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
}
-
- disk->mirror = NULL;
- disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
+ if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
+ ret = -1;
cleanup:
- /* revert to original disk def on failure */
if (oldsrc)
disk->src = oldsrc;
@@ -14980,6 +14985,8 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
unsigned int baseIndex = 0;
char *basePath = NULL;
char *backingPath = NULL;
+ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+ bool save = false;
if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
@@ -15033,19 +15040,30 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
disk->dst);
goto endjob;
}
- if (mode == BLOCK_JOB_ABORT &&
- (flags & VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT) &&
- !(async && disk->mirror)) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- _("pivot of disk '%s' requires an active copy job"),
- disk->dst);
- goto endjob;
- }
+ if (mode == BLOCK_JOB_ABORT) {
+ if ((flags & VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT) &&
+ !(async && disk->mirror)) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ _("pivot of disk '%s' requires an active copy job"),
+ disk->dst);
+ goto endjob;
+ }
+ if (disk->mirrorState != VIR_DOMAIN_DISK_MIRROR_STATE_NONE &&
+ disk->mirrorState != VIR_DOMAIN_DISK_MIRROR_STATE_READY) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ _("another job on disk '%s' is still being ended"),
+ disk->dst);
+ goto endjob;
+ }
- if (disk->mirror && mode == BLOCK_JOB_ABORT &&
- (flags & VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT)) {
- ret = qemuDomainBlockPivot(conn, driver, vm, device, disk);
- goto waitjob;
+ if (disk->mirror && (flags & VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT)) {
+ ret = qemuDomainBlockPivot(conn, driver, vm, device, disk);
+ goto waitjob;
+ }
+ if (disk->mirror) {
+ disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_ABORT;
+ save = true;
+ }
}
if (base &&
@@ -15084,36 +15102,40 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
ret = qemuMonitorBlockJob(priv->mon, device, basePath, backingPath,
bandwidth, info, mode, async);
qemuDomainObjExitMonitor(driver, vm);
- if (ret < 0)
+ if (ret < 0) {
+ if (mode == BLOCK_JOB_ABORT && disk->mirror)
+ disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
goto endjob;
+ }
/* Snoop block copy operations, so future cancel operations can
* 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)
+ info->cur == info->end && !disk->mirrorState) {
disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY;
-
- /* A successful block job cancelation stops any mirroring. */
- if (mode == BLOCK_JOB_ABORT && disk->mirror) {
- /* XXX We should also revoke security labels and disk lease on
- * the mirror, and audit that fact, before dropping things. */
- virStorageSourceFree(disk->mirror);
- disk->mirror = NULL;
- disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
+ save = true;
}
waitjob:
+ /* If we have made changes to XML due to a copy job, make a best
+ * effort to save it now. But we can ignore failure, since there
+ * will be further changes when the event marks completion. */
+ if (save)
+ ignore_value(virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm));
+
/* With synchronous block cancel, we must synthesize an event, and
* we silently ignore the ABORT_ASYNC flag. With asynchronous
- * block cancel, the event will come from qemu, but without the
- * ABORT_ASYNC flag, we must block to guarantee synchronous
- * operation. We do the waiting while still holding the VM job,
- * to prevent newly scheduled block jobs from confusing us. */
+ * block cancel, the event will come from qemu and will update the
+ * XML as appropriate, but without the ABORT_ASYNC flag, we must
+ * block to guarantee synchronous operation. We do the waiting
+ * while still holding the VM job, to prevent newly scheduled
+ * block jobs from confusing us. */
if (mode == BLOCK_JOB_ABORT) {
if (!async) {
/* Older qemu that lacked async reporting also lacked
- * active commit, so we can hardcode the event to pull.
- * We have to generate two variants of the event. */
+ * blockcopy and active commit, so we can hardcode the
+ * event to pull, and we know the XML doesn't need
+ * updating. We have to generate two event variants. */
int type = VIR_DOMAIN_BLOCK_JOB_TYPE_PULL;
int status = VIR_DOMAIN_BLOCK_JOB_CANCELED;
event = virDomainEventBlockJobNewFromObj(vm, disk->src->path, type,
@@ -15121,6 +15143,8 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
event2 = virDomainEventBlockJob2NewFromObj(vm, disk->dst, type,
status);
} else if (!(flags & VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC)) {
+ /* XXX If the event reports failure, we should reflect
+ * that back into the return status of this API call. */
while (1) {
/* Poll every 50ms */
static struct timespec ts = { .tv_sec = 0,
@@ -15158,6 +15182,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
}
cleanup:
+ virObjectUnref(cfg);
VIR_FREE(basePath);
VIR_FREE(backingPath);
VIR_FREE(device);
@@ -15389,6 +15414,10 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
disk->mirror = mirror;
mirror = NULL;
+ if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
+ VIR_WARN("Unable to save status on vm %s after state change",
+ vm->def->name);
+
endjob:
if (need_unlink && unlink(dest))
VIR_WARN("unable to unlink just-created %s", dest);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index c66326539e..d0132e3b66 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1016,6 +1016,8 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
virObjectEventPtr event2 = NULL;
const char *path;
virDomainDiskDefPtr disk;
+ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+ bool save = false;
virObjectLock(vm);
disk = qemuProcessFindDomainDiskByAlias(vm, diskAlias);
@@ -1027,29 +1029,52 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
event = virDomainEventBlockJobNewFromObj(vm, path, type, status);
event2 = virDomainEventBlockJob2NewFromObj(vm, disk->dst, type,
status);
- /* XXX If we completed a block pull or commit, then recompute
- * the cached backing chain to match. Better would be storing
- * the chain ourselves rather than reprobing, but this
- * requires modifying domain_conf and our XML to fully track
- * the chain across libvirtd restarts. For that matter, if
- * qemu gains support for committing the active layer, we have
- * to update disk->src. */
- if ((type == VIR_DOMAIN_BLOCK_JOB_TYPE_PULL ||
- type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT) &&
- status == VIR_DOMAIN_BLOCK_JOB_COMPLETED)
+
+ /* If we completed a block pull or commit, then update the XML
+ * to match. */
+ if (status == VIR_DOMAIN_BLOCK_JOB_COMPLETED) {
+ if (disk->mirrorState == VIR_DOMAIN_DISK_MIRROR_STATE_PIVOT) {
+ /* XXX We want to revoke security labels and disk
+ * lease, as well as audit that revocation, before
+ * dropping the original source. But it gets tricky
+ * if both source and mirror share common backing
+ * files (we want to only revoke the non-shared
+ * portion of the chain); so for now, we leak the
+ * access to the original. */
+ virStorageSourceFree(disk->src);
+ disk->src = disk->mirror;
+ } else {
+ virStorageSourceFree(disk->mirror);
+ }
+
+ /* Recompute the cached backing chain to match our
+ * updates. Better would be storing the chain ourselves
+ * rather than reprobing, but we haven't quite completed
+ * that conversion to use our XML tracking. */
+ disk->mirror = NULL;
+ save = disk->mirrorState != VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
+ disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
qemuDomainDetermineDiskChain(driver, vm, disk, true);
- if (disk->mirror && type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) {
+ } else if (disk->mirror && type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) {
if (status == VIR_DOMAIN_BLOCK_JOB_READY) {
disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY;
+ save = true;
} else if (status == VIR_DOMAIN_BLOCK_JOB_FAILED) {
virStorageSourceFree(disk->mirror);
disk->mirror = NULL;
disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
+ save = true;
}
}
}
+ if (save) {
+ if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
+ VIR_WARN("Unable to save status on vm %s after block job",
+ vm->def->name);
+ }
virObjectUnlock(vm);
+ virObjectUnref(cfg);
if (event)
qemuDomainEventQueue(driver, event);