On Fri, Feb 13, 2015 at 16:24:32 +0100, Michal Privoznik wrote:
Instead of unlocking and locking the domain object every 50ms lets just wait on blockJob condition and run the loop body if and BLOCK_JOB even occurred.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_migration.c | 15 +++++---------- src/qemu/qemu_process.c | 4 ++++ 2 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 14a4ec6..998e8f5 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1768,9 +1768,6 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver,
/* wait for completion */ while (true) { - /* Poll every 500ms for progress & to allow cancellation */ - struct timespec ts = { .tv_sec = 0, .tv_nsec = 500 * 1000 * 1000ull }; - /* Explicitly check if domain is still alive. Maybe qemu * died meanwhile so we won't see any event at all. */ if (!virDomainObjIsActive(vm)) { @@ -1800,13 +1797,11 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, goto error; }
- /* XXX Turn this into virCond someday. */ - - virObjectUnlock(vm); - - nanosleep(&ts, NULL); - - virObjectLock(vm); + if (virCondWait(&priv->blockJob, &vm->parent.lock) < 0) {
Hmm, you'd also need to signal this condition on EOF, otherwise this could be waiting forever when the domain dies during drive mirror job. But see below...
+ virReportSystemError(errno, "%s", + _("Unable to wait on blockJob condition")); + goto error; + } } }
The main problem with this patch is hidden in the (missing) context: if (priv->job.asyncAbort) { priv->job.current->type = VIR_DOMAIN_JOB_CANCELLED; virReportError(VIR_ERR_OPERATION_ABORTED, _("%s: %s"), qemuDomainAsyncJobTypeToString(priv->job.asyncJob), _("canceled by client")); goto error; } The loop would never break when priv->job.asyncAbort is set because it would still be waiting for priv->blockJob to be signalled. So we need to somehow turn this into a condition too. And because there's no way we could wait for more conditions at once, we'll need to think more what to do. One possibility is to create a general jobCond which would be signalled in several places: when asyncAbort is set, when block job changes its state, on monitor EOF, ... Jirka