[libvirt] [PATCH 0/5] Use synchronous block job events in qemu_migration

This patch series converts qemu_migration to use the synchronous block job event code introduced in commit 630ee5ac. It fixes two problems: - Drive mirroring has been broken since that commit, since the event indicating mirror readiness isn't processed while the VM object is locked. - Migration did not wait until the drive mirrors were properly cancelled, and this could cause disk corruption. Patch 1 moves qemuBlockJobEventProcess into a separate source file so it can be used by both qemu_driver and qemu_migration. Patch 2 introduces new qemuBlockJobSync* help functions to manage a synchronous block job. Patch 3 ensures that a thread waiting on a synchronous block job event is woken up should the domain crash. Patches 4 and 5 use the new synchronous block job helpers in qemu_driver and qemu_migration respectively. Michael Chapman (5): qemuBlockJobEventProcess: move to new source file qemuBlockJobSync*: introduce sync block job helpers qemuProcessStop: wake up pending sync block jobs qemuDomainBlockJobAbort: use sync block job helpers qemu: migration: use sync block job helpers po/POTFILES.in | 1 + src/Makefile.am | 1 + src/qemu/qemu_blockjob.c | 331 ++++++++++++++++++++++++++++++++++ src/qemu/qemu_blockjob.h | 49 ++++++ src/qemu/qemu_driver.c | 174 +++--------------- src/qemu/qemu_migration.c | 439 ++++++++++++++++++++++++++++------------------ src/qemu/qemu_process.c | 7 + 7 files changed, 676 insertions(+), 326 deletions(-) create mode 100644 src/qemu/qemu_blockjob.c create mode 100644 src/qemu/qemu_blockjob.h -- 2.1.0

We will want to use synchronous block jobs from qemu_migration as well, so split this function out into a new source file. Signed-off-by: Michael Chapman <mike@very.puzzling.org> --- src/Makefile.am | 1 + src/qemu/qemu_blockjob.c | 167 +++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_blockjob.h | 33 ++++++++++ src/qemu/qemu_driver.c | 118 +-------------------------------- 4 files changed, 202 insertions(+), 117 deletions(-) create mode 100644 src/qemu/qemu_blockjob.c create mode 100644 src/qemu/qemu_blockjob.h diff --git a/src/Makefile.am b/src/Makefile.am index 3c9eac6..196b0f5 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -714,6 +714,7 @@ VBOX_DRIVER_EXTRA_DIST = \ QEMU_DRIVER_SOURCES = \ qemu/qemu_agent.c qemu/qemu_agent.h \ + qemu/qemu_blockjob.c qemu/qemu_blockjob.h \ qemu/qemu_capabilities.c qemu/qemu_capabilities.h \ qemu/qemu_command.c qemu/qemu_command.h \ qemu/qemu_domain.c qemu/qemu_domain.h \ diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c new file mode 100644 index 0000000..e61ad8c --- /dev/null +++ b/src/qemu/qemu_blockjob.c @@ -0,0 +1,167 @@ +/* + * qemu_blockjob.c: helper functions for QEMU block jobs + * + * Copyright (C) 2006-2015 Red Hat, Inc. + * Copyright (C) 2006 Daniel P. Berrange + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#include <config.h> + +#include "internal.h" + +#include "qemu_blockjob.h" +#include "qemu_domain.h" + +#include "conf/domain_conf.h" +#include "conf/domain_event.h" + +#include "virlog.h" +#include "virstoragefile.h" + +#define VIR_FROM_THIS VIR_FROM_QEMU + +VIR_LOG_INIT("qemu.qemu_blockjob"); + +/** + * qemuBlockJobEventProcess: + * @driver: qemu driver + * @vm: domain + * @disk: domain disk + * @type: block job type + * @status: block job status + * + * Update disk's mirror state in response to a block job event + * from QEMU. For mirror state's that must survive libvirt + * restart, also update the domain's status XML. + * + * Returns 0 on success, -1 otherwise. + */ +void +qemuBlockJobEventProcess(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk, + int type, + int status) +{ + virObjectEventPtr event = NULL; + virObjectEventPtr event2 = NULL; + const char *path; + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + virDomainDiskDefPtr persistDisk = NULL; + bool save = false; + + /* Have to generate two variants of the event for old vs. new + * client callbacks */ + if (type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT && + disk->mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT) + type = disk->mirrorJob; + path = virDomainDiskGetSource(disk); + event = virDomainEventBlockJobNewFromObj(vm, path, type, status); + event2 = virDomainEventBlockJob2NewFromObj(vm, disk->dst, type, status); + + /* If we completed a block pull or commit, then update the XML + * to match. */ + switch ((virConnectDomainEventBlockJobStatus) status) { + case VIR_DOMAIN_BLOCK_JOB_COMPLETED: + if (disk->mirrorState == VIR_DOMAIN_DISK_MIRROR_STATE_PIVOT) { + if (vm->newDef) { + int indx = virDomainDiskIndexByName(vm->newDef, disk->dst, false); + virStorageSourcePtr copy = NULL; + + if (indx >= 0) { + persistDisk = vm->newDef->disks[indx]; + copy = virStorageSourceCopy(disk->mirror, false); + if (virStorageSourceInitChainElement(copy, + persistDisk->src, + true) < 0) { + VIR_WARN("Unable to update persistent definition " + "on vm %s after block job", + vm->def->name); + virStorageSourceFree(copy); + copy = NULL; + persistDisk = NULL; + } + } + if (copy) { + virStorageSourceFree(persistDisk->src); + persistDisk->src = copy; + } + } + + /* 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; + disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; + ignore_value(qemuDomainDetermineDiskChain(driver, vm, disk, + true, true)); + disk->blockjob = false; + break; + + case VIR_DOMAIN_BLOCK_JOB_READY: + disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY; + save = true; + break; + + case VIR_DOMAIN_BLOCK_JOB_FAILED: + case VIR_DOMAIN_BLOCK_JOB_CANCELED: + virStorageSourceFree(disk->mirror); + disk->mirror = NULL; + disk->mirrorState = status == VIR_DOMAIN_BLOCK_JOB_FAILED ? + VIR_DOMAIN_DISK_MIRROR_STATE_ABORT : VIR_DOMAIN_DISK_MIRROR_STATE_NONE; + disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; + save = true; + disk->blockjob = false; + break; + + case VIR_DOMAIN_BLOCK_JOB_LAST: + break; + } + + 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); + if (persistDisk && virDomainSaveConfig(cfg->configDir, + vm->newDef) < 0) + VIR_WARN("Unable to update persistent definition on vm %s " + "after block job", vm->def->name); + } + + if (event) + qemuDomainEventQueue(driver, event); + if (event2) + qemuDomainEventQueue(driver, event2); + + virObjectUnref(cfg); +} diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h new file mode 100644 index 0000000..abe1aa5 --- /dev/null +++ b/src/qemu/qemu_blockjob.h @@ -0,0 +1,33 @@ +/* + * qemu_blockjob.h: helper functions for QEMU block jobs + * + * Copyright (C) 2006-2015 Red Hat, Inc. + * Copyright (C) 2006 Daniel P. Berrange + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#ifndef __QEMU_BLOCKJOB_H__ +# define __QEMU_BLOCKJOB_H__ + +# include "qemu_conf.h" + +void qemuBlockJobEventProcess(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk, + int type, + int status); + +#endif /* __QEMU_BLOCKJOB_H__ */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 01e0122..00a4fb1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -55,6 +55,7 @@ #include "qemu_monitor.h" #include "qemu_process.h" #include "qemu_migration.h" +#include "qemu_blockjob.h" #include "virerror.h" #include "virlog.h" @@ -4451,123 +4452,6 @@ processSerialChangedEvent(virQEMUDriverPtr driver, static void -qemuBlockJobEventProcess(virQEMUDriverPtr driver, - virDomainObjPtr vm, - virDomainDiskDefPtr disk, - int type, - int status) -{ - virObjectEventPtr event = NULL; - virObjectEventPtr event2 = NULL; - const char *path; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); - virDomainDiskDefPtr persistDisk = NULL; - bool save = false; - - /* Have to generate two variants of the event for old vs. new - * client callbacks */ - if (type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT && - disk->mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT) - type = disk->mirrorJob; - path = virDomainDiskGetSource(disk); - event = virDomainEventBlockJobNewFromObj(vm, path, type, status); - event2 = virDomainEventBlockJob2NewFromObj(vm, disk->dst, type, status); - - /* If we completed a block pull or commit, then update the XML - * to match. */ - switch ((virConnectDomainEventBlockJobStatus) status) { - case VIR_DOMAIN_BLOCK_JOB_COMPLETED: - if (disk->mirrorState == VIR_DOMAIN_DISK_MIRROR_STATE_PIVOT) { - if (vm->newDef) { - int indx = virDomainDiskIndexByName(vm->newDef, disk->dst, false); - virStorageSourcePtr copy = NULL; - - if (indx >= 0) { - persistDisk = vm->newDef->disks[indx]; - copy = virStorageSourceCopy(disk->mirror, false); - if (virStorageSourceInitChainElement(copy, - persistDisk->src, - true) < 0) { - VIR_WARN("Unable to update persistent definition " - "on vm %s after block job", - vm->def->name); - virStorageSourceFree(copy); - copy = NULL; - persistDisk = NULL; - } - } - if (copy) { - virStorageSourceFree(persistDisk->src); - persistDisk->src = copy; - } - } - - /* 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; - disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; - ignore_value(qemuDomainDetermineDiskChain(driver, vm, disk, - true, true)); - disk->blockjob = false; - break; - - case VIR_DOMAIN_BLOCK_JOB_READY: - disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY; - save = true; - break; - - case VIR_DOMAIN_BLOCK_JOB_FAILED: - case VIR_DOMAIN_BLOCK_JOB_CANCELED: - virStorageSourceFree(disk->mirror); - disk->mirror = NULL; - disk->mirrorState = status == VIR_DOMAIN_BLOCK_JOB_FAILED ? - VIR_DOMAIN_DISK_MIRROR_STATE_ABORT : VIR_DOMAIN_DISK_MIRROR_STATE_NONE; - disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; - save = true; - disk->blockjob = false; - break; - - case VIR_DOMAIN_BLOCK_JOB_LAST: - break; - } - - 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); - if (persistDisk && virDomainSaveConfig(cfg->configDir, - vm->newDef) < 0) - VIR_WARN("Unable to update persistent definition on vm %s " - "after block job", vm->def->name); - } - - if (event) - qemuDomainEventQueue(driver, event); - if (event2) - qemuDomainEventQueue(driver, event2); - - virObjectUnref(cfg); -} - - -static void processBlockJobEvent(virQEMUDriverPtr driver, virDomainObjPtr vm, char *diskAlias, -- 2.1.0

qemuBlockJobSyncBegin and qemuBlockJobSyncEnd delimit a region of code where block job events are processed "synchronously". qemuBlockJobSyncWait and qemuBlockJobSyncWaitWithTimeout wait for an event generated by a block job. The Wait* functions may be called multiple times while the synchronous block job is active. Any pending block job event will be processed by only when Wait* or End is called. disk->blockJobStatus is reset by these functions, so if it is needed a pointer to a virConnectDomainEventBlockJobStatus variable should be passed as the last argument. It is safe to pass NULL if you do not care about the block job status. All functions assume the VM object is locked. The Wait* functions will unlock the object for as long as they are waiting. They will return -1 and report an error if the domain exits before an event is received. Typical use is as follows: virQEMUDriverPtr driver; virDomainObjPtr vm; /* locked */ virDomainDiskDefPtr disk; virConnectDomainEventBlockJobStatus status; qemuBlockJobSyncBegin(disk); ... start block job ... if (qemuBlockJobSyncWait(driver, vm, disk, &status) < 0) { /* domain died while waiting for event */ ret = -1; goto error; } ... possibly start other block jobs or wait for further events ... qemuBlockJobSyncEnd(driver, vm, disk, NULL); To perform other tasks periodically while waiting for an event: virQEMUDriverPtr driver; virDomainObjPtr vm; /* locked */ virDomainDiskDefPtr disk; virConnectDomainEventBlockJobStatus status; unsigned long long timeout = 500 * 1000ull; /* milliseconds */ qemuBlockJobSyncBegin(disk); ... start block job ... do { ... do other task ... if (qemuBlockJobSyncWaitWithTimeout(driver, vm, disk, timeout, &status) < 0) { /* domain died while waiting for event */ ret = -1; goto error; } } while (status == -1); qemuBlockJobSyncEnd(driver, vm, disk, NULL); Signed-off-by: Michael Chapman <mike@very.puzzling.org> --- po/POTFILES.in | 1 + src/qemu/qemu_blockjob.c | 164 +++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_blockjob.h | 16 +++++ 3 files changed, 181 insertions(+) diff --git a/po/POTFILES.in b/po/POTFILES.in index dd06ab3..bb0f6e1 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -112,6 +112,7 @@ src/parallels/parallels_utils.h src/parallels/parallels_storage.c src/phyp/phyp_driver.c src/qemu/qemu_agent.c +src/qemu/qemu_blockjob.c src/qemu/qemu_capabilities.c src/qemu/qemu_cgroup.c src/qemu/qemu_command.c diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index e61ad8c..729928a 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -31,6 +31,8 @@ #include "virlog.h" #include "virstoragefile.h" +#include "virthread.h" +#include "virtime.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -165,3 +167,165 @@ qemuBlockJobEventProcess(virQEMUDriverPtr driver, virObjectUnref(cfg); } + + +/** + * qemuBlockJobSyncBegin: + * @disk: domain disk + * + * Begin a new synchronous block job for @disk. The synchronous + * block job is ended by a call to qemuBlockJobSyncEnd, or by + * the guest quitting. + * + * During a synchronous block job, a block job event for @disk + * will not be processed asynchronously. Instead, it will be + * processed only when qemuBlockJobSyncWait* or + * qemuBlockJobSyncEnd is called. + */ +void +qemuBlockJobSyncBegin(virDomainDiskDefPtr disk) +{ + if (disk->blockJobSync) + VIR_WARN("Disk %s already has synchronous block job", + disk->dst); + + disk->blockJobSync = true; +} + + +/** + * qemuBlockJobSyncEnd: + * @driver: qemu driver + * @vm: domain + * @disk: domain disk + * @ret_status: pointer to virConnectDomainEventBlockJobStatus + * + * End a synchronous block job for @disk. Any pending block job event + * for the disk is processed, and its status is recorded in the + * virConnectDomainEventBlockJobStatus field pointed to by + * @ret_status. + */ +void +qemuBlockJobSyncEnd(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk, + virConnectDomainEventBlockJobStatus *ret_status) +{ + if (disk->blockJobSync && disk->blockJobStatus != -1) { + if (ret_status) + *ret_status = disk->blockJobStatus; + qemuBlockJobEventProcess(driver, vm, disk, + disk->blockJobType, + disk->blockJobStatus); + disk->blockJobStatus = -1; + } + disk->blockJobSync = false; +} + + +/** + * qemuBlockJobSyncWaitWithTimeout: + * @driver: qemu driver + * @vm: domain + * @disk: domain disk + * @timeout: timeout in milliseconds + * @ret_status: pointer to virConnectDomainEventBlockJobStatus + * + * Wait up to @timeout milliseconds for a block job event for @disk. + * If an event is received it is processed, and its status is recorded + * in the virConnectDomainEventBlockJobStatus field pointed to by + * @ret_status. + * + * If @timeout is not 0, @vm will be unlocked while waiting for the event. + * + * Returns 0 if an event was received or the timeout expired, + * -1 otherwise. + */ +int +qemuBlockJobSyncWaitWithTimeout(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk, + unsigned long long timeout, + virConnectDomainEventBlockJobStatus *ret_status) +{ + if (!disk->blockJobSync) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("No current synchronous block job")); + return -1; + } + + while (disk->blockJobSync && disk->blockJobStatus == -1) { + int r; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + disk->blockJobSync = false; + return -1; + } + + if (timeout == (unsigned long long)-1) { + r = virCondWait(&disk->blockJobSyncCond, &vm->parent.lock); + } else if (timeout) { + unsigned long long now; + if (virTimeMillisNow(&now) < 0) { + virReportSystemError(errno, "%s", + _("Unable to get current time")); + return -1; + } + r = virCondWaitUntil(&disk->blockJobSyncCond, &vm->parent.lock, + now + timeout); + if (r < 0 && errno == ETIMEDOUT) + return 0; + } else { + errno = ETIMEDOUT; + return 0; + } + + if (r < 0) { + disk->blockJobSync = false; + virReportSystemError(errno, "%s", + _("Unable to wait on block job sync " + "condition")); + return -1; + } + } + + if (ret_status) + *ret_status = disk->blockJobStatus; + qemuBlockJobEventProcess(driver, vm, disk, + disk->blockJobType, + disk->blockJobStatus); + disk->blockJobStatus = -1; + + return 0; +} + + +/** + * qemuBlockJobSyncWait: + * @driver: qemu driver + * @vm: domain + * @disk: domain disk + * @ret_status: pointer to virConnectDomainEventBlockJobStatus + * + * Wait for a block job event for @disk. If an event is received it + * is processed, and its status is recorded in the + * virConnectDomainEventBlockJobStatus field pointed to by + * @ret_status. + * + * @vm will be unlocked while waiting for the event. + * + * Returns 0 if an event was received, + * -1 otherwise. + */ +int +qemuBlockJobSyncWait(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk, + virConnectDomainEventBlockJobStatus *ret_status) +{ + return qemuBlockJobSyncWaitWithTimeout(driver, vm, disk, + (unsigned long long)-1, + ret_status); +} diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h index abe1aa5..ba372a2 100644 --- a/src/qemu/qemu_blockjob.h +++ b/src/qemu/qemu_blockjob.h @@ -22,6 +22,7 @@ #ifndef __QEMU_BLOCKJOB_H__ # define __QEMU_BLOCKJOB_H__ +# include "internal.h" # include "qemu_conf.h" void qemuBlockJobEventProcess(virQEMUDriverPtr driver, @@ -30,4 +31,19 @@ void qemuBlockJobEventProcess(virQEMUDriverPtr driver, int type, int status); +void qemuBlockJobSyncBegin(virDomainDiskDefPtr disk); +void qemuBlockJobSyncEnd(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk, + virConnectDomainEventBlockJobStatus *ret_status); +int qemuBlockJobSyncWaitWithTimeout(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk, + unsigned long long timeout, + virConnectDomainEventBlockJobStatus *ret_status); +int qemuBlockJobSyncWait(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk, + virConnectDomainEventBlockJobStatus *ret_status); + #endif /* __QEMU_BLOCKJOB_H__ */ -- 2.1.0

Other threads may be blocked in qemuBlockJobSyncWait. Ensure that they're woken up when the domain is stopped. Signed-off-by: Michael Chapman <mike@very.puzzling.org> --- src/qemu/qemu_process.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 276837e..5390bc8 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5086,6 +5086,13 @@ void qemuProcessStop(virQEMUDriverPtr driver, if (virAtomicIntDecAndTest(&driver->nactive) && driver->inhibitCallback) driver->inhibitCallback(false, driver->inhibitOpaque); + /* Wake up anything waiting on synchronous block jobs */ + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr disk = vm->def->disks[i]; + if (disk->blockJobSync && disk->blockJobStatus == -1) + virCondSignal(&disk->blockJobSyncCond); + } + if ((logfile = qemuDomainCreateLog(driver, vm, true)) < 0) { /* To not break the normal domain shutdown process, skip the * timestamp log writing if failed on opening log file. */ -- 2.1.0

The !modern code path needs to call qemuBlockJobEventProcess directly. the modern code path will call it via qemuBlockJobSyncWait. Signed-off-by: Michael Chapman <mike@very.puzzling.org> --- src/qemu/qemu_driver.c | 56 ++++++++++++++++++-------------------------------- 1 file changed, 20 insertions(+), 36 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 00a4fb1..84ea817 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16269,7 +16269,7 @@ qemuDomainBlockJobAbort(virDomainPtr dom, { virQEMUDriverPtr driver = dom->conn->privateData; char *device = NULL; - virDomainDiskDefPtr disk; + virDomainDiskDefPtr disk = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); bool save = false; int idx; @@ -16304,14 +16304,6 @@ qemuDomainBlockJobAbort(virDomainPtr dom, goto endjob; disk = vm->def->disks[idx]; - if (modern && !async) { - /* prepare state for event delivery. Since qemuDomainBlockPivot is - * synchronous, but the event is delivered asynchronously we need to - * wait too */ - disk->blockJobStatus = -1; - disk->blockJobSync = true; - } - if (disk->mirrorState != VIR_DOMAIN_DISK_MIRROR_STATE_NONE && disk->mirrorState != VIR_DOMAIN_DISK_MIRROR_STATE_READY) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -16320,11 +16312,14 @@ qemuDomainBlockJobAbort(virDomainPtr dom, goto endjob; } + if (modern && !async) { + /* prepare state for event delivery */ + qemuBlockJobSyncBegin(disk); + } + if (pivot) { - if ((ret = qemuDomainBlockPivot(driver, vm, device, disk)) < 0) { - disk->blockJobSync = false; + if ((ret = qemuDomainBlockPivot(driver, vm, device, disk)) < 0) goto endjob; - } } else { if (disk->mirror) { disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_ABORT; @@ -16364,36 +16359,25 @@ qemuDomainBlockJobAbort(virDomainPtr dom, * blockcopy and active commit, so we can hardcode the * event to pull and let qemuBlockJobEventProcess() handle * the rest as usual */ - disk->blockJobType = VIR_DOMAIN_BLOCK_JOB_TYPE_PULL; - disk->blockJobStatus = VIR_DOMAIN_BLOCK_JOB_CANCELED; + qemuBlockJobEventProcess(driver, vm, disk, + VIR_DOMAIN_BLOCK_JOB_TYPE_PULL, + VIR_DOMAIN_BLOCK_JOB_CANCELED); } else { - while (disk->blockJobStatus == -1 && disk->blockJobSync) { - if (virCondWait(&disk->blockJobSyncCond, &vm->parent.lock) < 0) { - virReportSystemError(errno, "%s", - _("Unable to wait on block job sync " - "condition")); - disk->blockJobSync = false; - goto endjob; - } + virConnectDomainEventBlockJobStatus status = -1; + if (qemuBlockJobSyncWait(driver, vm, disk, &status) < 0) { + ret = -1; + } else if (status == VIR_DOMAIN_BLOCK_JOB_FAILED) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("failed to terminate block job on disk '%s'"), + disk->dst); + ret = -1; } } - - qemuBlockJobEventProcess(driver, vm, disk, - disk->blockJobType, - disk->blockJobStatus); - - /* adjust the return code if we've got an explicit failure */ - if (disk->blockJobStatus == VIR_DOMAIN_BLOCK_JOB_FAILED) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("failed to terminate block job on disk '%s'"), - disk->dst); - ret = -1; - } - - disk->blockJobSync = false; } endjob: + if (disk && disk->blockJobSync) + qemuBlockJobSyncEnd(driver, vm, disk, NULL); qemuDomainObjEndJob(driver, vm); cleanup: -- 2.1.0

In qemuMigrationDriveMirror we can start all disk mirrors in parallel. We wait until they are all ready, or one of them aborts. In qemuMigrationCancelDriveMirror, we wait until all mirrors are properly stopped. This is necessary to ensure that destination VM is fully in sync with the (paused) source VM. If a drive mirror can not be cancelled, then the destination is not in a consistent state. In this case it is not safe to continue with the migration. Signed-off-by: Michael Chapman <mike@very.puzzling.org> --- src/qemu/qemu_migration.c | 439 ++++++++++++++++++++++++++++------------------ 1 file changed, 266 insertions(+), 173 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 611f53a..752097a 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -39,6 +39,7 @@ #include "qemu_command.h" #include "qemu_cgroup.h" #include "qemu_hotplug.h" +#include "qemu_blockjob.h" #include "domain_audit.h" #include "virlog.h" @@ -1679,6 +1680,200 @@ qemuMigrationStartNBDServer(virQEMUDriverPtr driver, goto cleanup; } + +static int +qemuMigrationStopNBDServer(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuMigrationCookiePtr mig) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (!mig->nbd) + return 0; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, + QEMU_ASYNC_JOB_MIGRATION_IN) < 0) + return -1; + + if (qemuMonitorNBDServerStop(priv->mon) < 0) + VIR_WARN("Unable to stop NBD server"); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + return -1; + + virPortAllocatorRelease(driver->migrationPorts, priv->nbdPort); + priv->nbdPort = 0; + return 0; +} + + +/** + * qemuMigrationCheckDriveMirror: + * @driver: qemu driver + * @vm: domain + * + * Check the status of all drive-mirrors started by + * qemuMigrationDriveMirror. Any pending block job events + * for the mirrored disks will be processed. + * + * Returns 1 if all mirrors are "ready", + * 0 if some mirrors are still performing initial sync, + * -1 on error. + */ +static int +qemuMigrationCheckDriveMirror(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + size_t i; + int ret = 1; + + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr disk = vm->def->disks[i]; + + /* skip shared, RO and source-less disks */ + if (disk->src->shared || disk->src->readonly || + !virDomainDiskGetSource(disk)) + continue; + + /* skip disks that didn't start mirroring */ + if (!disk->blockJobSync) + continue; + + /* process any pending event */ + if (qemuBlockJobSyncWaitWithTimeout(driver, vm, disk, + 0ull, NULL) < 0) + return -1; + + switch (disk->mirrorState) { + case VIR_DOMAIN_DISK_MIRROR_STATE_NONE: + ret = 0; + break; + case VIR_DOMAIN_DISK_MIRROR_STATE_ABORT: + virReportError(VIR_ERR_OPERATION_FAILED, + _("migration of disk %s failed"), + disk->dst); + return -1; + } + } + + return ret; +} + + +/** + * qemuMigrationCancelOneDriveMirror: + * @driver: qemu driver + * @vm: domain + * + * Cancel all drive-mirrors started by qemuMigrationDriveMirror. + * Any pending block job events for the mirrored disks will be + * processed. + * + * Returns 0 on success, -1 otherwise. + */ +static int +qemuMigrationCancelOneDriveMirror(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + char *diskAlias = NULL; + int ret = -1; + + /* No need to cancel if mirror already aborted */ + if (disk->mirrorState == VIR_DOMAIN_DISK_MIRROR_STATE_ABORT) { + ret = 0; + } else { + virConnectDomainEventBlockJobStatus status = -1; + + if (virAsprintf(&diskAlias, "%s%s", + QEMU_DRIVE_HOST_PREFIX, disk->info.alias) < 0) + goto cleanup; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, + QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) + goto endjob; + ret = qemuMonitorBlockJobCancel(priv->mon, diskAlias, true); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto endjob; + + if (ret < 0) { + virDomainBlockJobInfo info; + + /* block-job-cancel can fail if QEMU simultaneously + * aborted the job; probe for it again to detect this */ + if (qemuMonitorBlockJobInfo(priv->mon, diskAlias, + &info, NULL) == 0) { + ret = 0; + } else { + virReportError(VIR_ERR_OPERATION_FAILED, + _("could not cancel migration of disk %s"), + disk->dst); + } + + goto endjob; + } + + /* Mirror may become ready before cancellation takes + * effect; loop if we get that event first */ + do { + ret = qemuBlockJobSyncWait(driver, vm, disk, &status); + if (ret < 0) { + VIR_WARN("Unable to wait for block job on %s to cancel", + diskAlias); + goto endjob; + } + } while (status == VIR_DOMAIN_BLOCK_JOB_READY); + } + + endjob: + qemuBlockJobSyncEnd(driver, vm, disk, NULL); + + if (disk->mirrorState == VIR_DOMAIN_DISK_MIRROR_STATE_ABORT) + disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; + + cleanup: + VIR_FREE(diskAlias); + return ret; +} + + +/** + * qemuMigrationCancelDriveMirror: + * @driver: qemu driver + * @vm: domain + * + * Cancel all drive-mirrors started by qemuMigrationDriveMirror. + * Any pending block job events for the affected disks will be + * processed. + * + * Returns 0 on success, -1 otherwise. + */ +static int +qemuMigrationCancelDriveMirror(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + size_t i; + + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr disk = vm->def->disks[i]; + + /* skip shared, RO and source-less disks */ + if (disk->src->shared || disk->src->readonly || + !virDomainDiskGetSource(disk)) + continue; + + /* skip disks that didn't start mirroring */ + if (!disk->blockJobSync) + continue; + + if (qemuMigrationCancelOneDriveMirror(driver, vm, disk) < 0) + return -1; + } + + return 0; +} + + /** * qemuMigrationDriveMirror: * @driver: qemu driver @@ -1690,10 +1885,11 @@ qemuMigrationStartNBDServer(virQEMUDriverPtr driver, * * Run drive-mirror to feed NBD server running on dst and wait * till the process switches into another phase where writes go - * simultaneously to both source and destination. And this switch - * is what we are waiting for before proceeding with the next - * disk. On success, update @migrate_flags so we don't tell - * 'migrate' command to do the very same operation. + * simultaneously to both source and destination. On success, + * update @migrate_flags so we don't tell 'migrate' command + * to do the very same operation. On failure, the caller is + * expected to call qemuMigrationCancelDriveMirror to stop all + * running mirrors. * * Returns 0 on success (@migrate_flags updated), * -1 otherwise. @@ -1708,26 +1904,12 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, { qemuDomainObjPrivatePtr priv = vm->privateData; int ret = -1; - int mon_ret; int port; - size_t i, lastGood = 0; + size_t i; char *diskAlias = NULL; char *nbd_dest = NULL; char *hoststr = NULL; unsigned int mirror_flags = VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT; - virErrorPtr err = NULL; - - if (!(*migrate_flags & (QEMU_MONITOR_MIGRATE_NON_SHARED_DISK | - QEMU_MONITOR_MIGRATE_NON_SHARED_INC))) - return 0; - - if (!mig->nbd) { - /* Destination doesn't support NBD server. - * Fall back to previous implementation. */ - VIR_DEBUG("Destination doesn't support NBD server " - "Falling back to previous implementation."); - return 0; - } /* steal NBD port and thus prevent its propagation back to destination */ port = mig->nbd->port; @@ -1736,9 +1918,9 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, /* escape literal IPv6 address */ if (strchr(host, ':')) { if (virAsprintf(&hoststr, "[%s]", host) < 0) - goto error; + goto cleanup; } else if (VIR_STRDUP(hoststr, host) < 0) { - goto error; + goto cleanup; } if (*migrate_flags & QEMU_MONITOR_MIGRATE_NON_SHARED_INC) @@ -1746,6 +1928,7 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, for (i = 0; i < vm->def->ndisks; i++) { virDomainDiskDefPtr disk = vm->def->disks[i]; + int mon_ret; /* skip shared, RO and source-less disks */ if (disk->src->shared || disk->src->readonly || @@ -1758,34 +1941,36 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, QEMU_DRIVE_HOST_PREFIX, disk->info.alias) < 0) || (virAsprintf(&nbd_dest, "nbd:%s:%d:exportname=%s", hoststr, port, diskAlias) < 0)) - goto error; + goto cleanup; + + qemuBlockJobSyncBegin(disk); if (qemuDomainObjEnterMonitorAsync(driver, vm, - QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) - goto error; + QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) { + qemuBlockJobSyncEnd(driver, vm, disk, NULL); + goto cleanup; + } + mon_ret = qemuMonitorDriveMirror(priv->mon, diskAlias, nbd_dest, NULL, speed, 0, 0, mirror_flags); - if (qemuDomainObjExitMonitor(driver, vm) < 0) - goto error; - - if (mon_ret < 0) - goto error; - lastGood = i; + if (qemuDomainObjExitMonitor(driver, vm) < 0 || mon_ret < 0) { + qemuBlockJobSyncEnd(driver, vm, disk, NULL); + goto cleanup; + } + } - /* wait for completion */ - while (true) { - /* Poll every 500ms for progress & to allow cancellation */ - struct timespec ts = { .tv_sec = 0, .tv_nsec = 500 * 1000 * 1000ull }; + /* Wait for each disk to become ready in turn, but check the status + * for *all* mirrors to determine if any have aborted. */ + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr disk = vm->def->disks[i]; - /* Explicitly check if domain is still alive. Maybe qemu - * died meanwhile so we won't see any event at all. */ - if (!virDomainObjIsActive(vm)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("guest unexpectedly quit")); - goto error; - } + /* skip shared, RO and source-less disks */ + if (disk->src->shared || disk->src->readonly || + !virDomainDiskGetSource(disk)) + continue; + while (disk->mirrorState != VIR_DOMAIN_DISK_MIRROR_STATE_READY) { /* The following check should be race free as long as the variable * is set only with domain object locked. And here we have the * domain object locked too. */ @@ -1794,30 +1979,19 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, virReportError(VIR_ERR_OPERATION_ABORTED, _("%s: %s"), qemuDomainAsyncJobTypeToString(priv->job.asyncJob), _("canceled by client")); - goto error; - } - - if (disk->mirrorState == VIR_DOMAIN_DISK_MIRROR_STATE_READY) { - VIR_DEBUG("Drive mirroring of '%s' completed", diskAlias); - break; - } else if (disk->mirrorState == VIR_DOMAIN_DISK_MIRROR_STATE_ABORT) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("migration of disk %s failed"), - disk->dst); - goto error; + goto cleanup; } - /* XXX Turn this into virCond someday. */ - - virObjectUnlock(vm); - - nanosleep(&ts, NULL); + if (qemuBlockJobSyncWaitWithTimeout(driver, vm, disk, + 500ull, NULL) < 0) + goto cleanup; - virObjectLock(vm); + if (qemuMigrationCheckDriveMirror(driver, vm) < 0) + goto cleanup; } } - /* Okay, copied. Modify migrate_flags */ + /* Okay, all disks are ready. Modify migrate_flags */ *migrate_flags &= ~(QEMU_MONITOR_MIGRATE_NON_SHARED_DISK | QEMU_MONITOR_MIGRATE_NON_SHARED_INC); ret = 0; @@ -1827,115 +2001,9 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, VIR_FREE(nbd_dest); VIR_FREE(hoststr); return ret; - - error: - /* don't overwrite any errors */ - err = virSaveLastError(); - /* cancel any outstanding jobs */ - while (lastGood) { - virDomainDiskDefPtr disk = vm->def->disks[--lastGood]; - - /* skip shared, RO disks */ - if (disk->src->shared || disk->src->readonly || - !virDomainDiskGetSource(disk)) - continue; - - VIR_FREE(diskAlias); - if (virAsprintf(&diskAlias, "%s%s", - QEMU_DRIVE_HOST_PREFIX, disk->info.alias) < 0) - continue; - if (qemuDomainObjEnterMonitorAsync(driver, vm, - QEMU_ASYNC_JOB_MIGRATION_OUT) == 0) { - if (qemuMonitorBlockJobCancel(priv->mon, diskAlias, true) < 0) - VIR_WARN("Unable to cancel block-job on '%s'", diskAlias); - - if (qemuDomainObjExitMonitor(driver, vm) < 0) - break; - } else { - VIR_WARN("Unable to enter monitor. No block job cancelled"); - } - - /* If disk mirror is already aborted, clear the mirror state now */ - if (disk->mirrorState == VIR_DOMAIN_DISK_MIRROR_STATE_ABORT) - disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; - } - if (err) - virSetError(err); - virFreeError(err); - goto cleanup; } -static int -qemuMigrationStopNBDServer(virQEMUDriverPtr driver, - virDomainObjPtr vm, - qemuMigrationCookiePtr mig) -{ - qemuDomainObjPrivatePtr priv = vm->privateData; - - if (!mig->nbd) - return 0; - - if (qemuDomainObjEnterMonitorAsync(driver, vm, - QEMU_ASYNC_JOB_MIGRATION_IN) < 0) - return -1; - - if (qemuMonitorNBDServerStop(priv->mon) < 0) - VIR_WARN("Unable to stop NBD server"); - if (qemuDomainObjExitMonitor(driver, vm) < 0) - return -1; - - virPortAllocatorRelease(driver->migrationPorts, priv->nbdPort); - priv->nbdPort = 0; - return 0; -} - -static int -qemuMigrationCancelDriveMirror(qemuMigrationCookiePtr mig, - virQEMUDriverPtr driver, - virDomainObjPtr vm) -{ - qemuDomainObjPrivatePtr priv = vm->privateData; - size_t i; - char *diskAlias = NULL; - int ret = 0; - - VIR_DEBUG("mig=%p nbdPort=%d", mig->nbd, priv->nbdPort); - - for (i = 0; i < vm->def->ndisks; i++) { - virDomainDiskDefPtr disk = vm->def->disks[i]; - - /* skip shared, RO and source-less disks */ - if (disk->src->shared || disk->src->readonly || - !virDomainDiskGetSource(disk)) - continue; - - VIR_FREE(diskAlias); - if (virAsprintf(&diskAlias, "%s%s", - QEMU_DRIVE_HOST_PREFIX, disk->info.alias) < 0) - goto cleanup; - - if (qemuDomainObjEnterMonitorAsync(driver, vm, - QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) - goto cleanup; - - if (qemuMonitorBlockJobCancel(priv->mon, diskAlias, true) < 0) - VIR_WARN("Unable to stop block job on %s", diskAlias); - if (qemuDomainObjExitMonitor(driver, vm) < 0) { - ret = -1; - goto cleanup; - } - - /* If disk mirror is already aborted, clear the mirror state now */ - if (disk->mirrorState == VIR_DOMAIN_DISK_MIRROR_STATE_ABORT) - disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; - } - - cleanup: - VIR_FREE(diskAlias); - return ret; -} - /* Validate whether the domain is safe to migrate. If vm is NULL, * then this is being run in the v2 Prepare stage on the destination * (where we only have the target xml); if vm is provided, then this @@ -3481,9 +3549,13 @@ qemuMigrationConfirmPhase(virQEMUDriverPtr driver, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_MIGRATED); } else { + virErrorPtr orig_err = virSaveLastError(); /* cancel any outstanding NBD jobs */ - qemuMigrationCancelDriveMirror(mig, driver, vm); + qemuMigrationCancelDriveMirror(driver, vm); + + virSetError(orig_err); + virFreeError(orig_err); if (qemuMigrationRestoreDomainState(conn, vm)) { event = virDomainEventLifecycleNewFromObj(vm, @@ -3886,11 +3958,22 @@ qemuMigrationRun(virQEMUDriverPtr driver, if (qemuDomainMigrateGraphicsRelocate(driver, vm, mig, graphicsuri) < 0) VIR_WARN("unable to provide data for graphics client relocation"); - /* this will update migrate_flags on success */ - if (qemuMigrationDriveMirror(driver, vm, mig, spec->dest.host.name, - migrate_speed, &migrate_flags) < 0) { - /* error reported by helper func */ - goto cleanup; + if (migrate_flags & (QEMU_MONITOR_MIGRATE_NON_SHARED_DISK | + QEMU_MONITOR_MIGRATE_NON_SHARED_INC)) { + if (mig->nbd) { + /* This will update migrate_flags on success */ + if (qemuMigrationDriveMirror(driver, vm, mig, + spec->dest.host.name, + migrate_speed, + &migrate_flags) < 0) { + goto cleanup; + } + } else { + /* Destination doesn't support NBD server. + * Fall back to previous implementation. */ + VIR_DEBUG("Destination doesn't support NBD server " + "Falling back to previous implementation."); + } } /* Before EnterMonitor, since qemuMigrationSetOffline already does that */ @@ -4017,6 +4100,14 @@ qemuMigrationRun(virQEMUDriverPtr driver, else if (rc == -1) goto cleanup; + /* Confirm state of drive mirrors */ + if (mig->nbd) { + if (qemuMigrationCheckDriveMirror(driver, vm) != 1) { + ret = -1; + goto cancel; + } + } + /* When migration completed, QEMU will have paused the * CPUs for us, but unless we're using the JSON monitor * we won't have been notified of this, so might still @@ -4039,8 +4130,10 @@ qemuMigrationRun(virQEMUDriverPtr driver, orig_err = virSaveLastError(); /* cancel any outstanding NBD jobs */ - if (mig) - ignore_value(qemuMigrationCancelDriveMirror(mig, driver, vm)); + if (mig && mig->nbd) { + if (qemuMigrationCancelDriveMirror(driver, vm) < 0) + ret = -1; + } if (spec->fwdType != MIGRATION_FWD_DIRECT) { if (iothread && qemuMigrationStopTunnel(iothread, ret < 0) < 0) -- 2.1.0

On 16.04.2015 11:24, Michael Chapman wrote:
This patch series converts qemu_migration to use the synchronous block job event code introduced in commit 630ee5ac.
It fixes two problems:
- Drive mirroring has been broken since that commit, since the event indicating mirror readiness isn't processed while the VM object is locked. - Migration did not wait until the drive mirrors were properly cancelled, and this could cause disk corruption.
Patch 1 moves qemuBlockJobEventProcess into a separate source file so it can be used by both qemu_driver and qemu_migration. Patch 2 introduces new qemuBlockJobSync* help functions to manage a synchronous block job. Patch 3 ensures that a thread waiting on a synchronous block job event is woken up should the domain crash. Patches 4 and 5 use the new synchronous block job helpers in qemu_driver and qemu_migration respectively.
Michael Chapman (5): qemuBlockJobEventProcess: move to new source file qemuBlockJobSync*: introduce sync block job helpers qemuProcessStop: wake up pending sync block jobs qemuDomainBlockJobAbort: use sync block job helpers qemu: migration: use sync block job helpers
po/POTFILES.in | 1 + src/Makefile.am | 1 + src/qemu/qemu_blockjob.c | 331 ++++++++++++++++++++++++++++++++++ src/qemu/qemu_blockjob.h | 49 ++++++ src/qemu/qemu_driver.c | 174 +++--------------- src/qemu/qemu_migration.c | 439 ++++++++++++++++++++++++++++------------------ src/qemu/qemu_process.c | 7 + 7 files changed, 676 insertions(+), 326 deletions(-) create mode 100644 src/qemu/qemu_blockjob.c create mode 100644 src/qemu/qemu_blockjob.h
ACK series. Even though this is rather big patch set, it fixes important bug. Therefore I'm pushing this now. Thanks for chasing the problem down. Michal
participants (2)
-
Michael Chapman
-
Michal Privoznik