[libvirt] [PATCH v3 04/24] qemu: Use domain condition for synchronous block jobs

By switching block jobs to use domain conditions, we can drop some pretty complicated code in NBD storage migration. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- Notes: Version 3: - split into 3 patches Version 2: - slightly modified to use domain conditions po/POTFILES.in | 1 - src/qemu/qemu_blockjob.c | 137 +++------------------------------------------- src/qemu/qemu_blockjob.h | 12 +--- src/qemu/qemu_domain.c | 17 +----- src/qemu/qemu_domain.h | 1 - src/qemu/qemu_driver.c | 24 ++++---- src/qemu/qemu_migration.c | 112 +++++++++++++++++-------------------- src/qemu/qemu_process.c | 13 ++--- 8 files changed, 76 insertions(+), 241 deletions(-) diff --git a/po/POTFILES.in b/po/POTFILES.in index bb0f6e1..dd06ab3 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -112,7 +112,6 @@ 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 eb05cef..3aa6118 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -214,19 +214,17 @@ qemuBlockJobEventProcess(virQEMUDriverPtr driver, * * 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. + * processed only when qemuBlockJobUpdate or qemuBlockJobSyncEnd + * is called. */ void qemuBlockJobSyncBegin(virDomainDiskDefPtr disk) { qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); - if (diskPriv->blockJobSync) - VIR_WARN("Disk %s already has synchronous block job", - disk->dst); - + VIR_DEBUG("disk=%s", disk->dst); diskPriv->blockJobSync = true; + diskPriv->blockJobStatus = -1; } @@ -235,135 +233,16 @@ qemuBlockJobSyncBegin(virDomainDiskDefPtr disk) * @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. + * for the disk is processed. */ void qemuBlockJobSyncEnd(virQEMUDriverPtr driver, virDomainObjPtr vm, - virDomainDiskDefPtr disk, - virConnectDomainEventBlockJobStatus *ret_status) + virDomainDiskDefPtr disk) { - qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); - - if (diskPriv->blockJobSync && diskPriv->blockJobStatus != -1) { - if (ret_status) - *ret_status = diskPriv->blockJobStatus; - qemuBlockJobUpdate(driver, vm, disk); - diskPriv->blockJobStatus = -1; - } - diskPriv->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) -{ - qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); - - if (!diskPriv->blockJobSync) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("No current synchronous block job")); - return -1; - } - - while (diskPriv->blockJobSync && diskPriv->blockJobStatus == -1) { - int r; - - if (!virDomainObjIsActive(vm)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("guest unexpectedly quit")); - diskPriv->blockJobSync = false; - return -1; - } - - if (timeout == (unsigned long long)-1) { - r = virCondWait(&diskPriv->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(&diskPriv->blockJobSyncCond, - &vm->parent.lock, - now + timeout); - if (r < 0 && errno == ETIMEDOUT) - return 0; - } else { - errno = ETIMEDOUT; - return 0; - } - - if (r < 0) { - diskPriv->blockJobSync = false; - virReportSystemError(errno, "%s", - _("Unable to wait on block job sync " - "condition")); - return -1; - } - } - - if (ret_status) - *ret_status = diskPriv->blockJobStatus; + VIR_DEBUG("disk=%s", disk->dst); qemuBlockJobUpdate(driver, vm, disk); - diskPriv->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); + QEMU_DOMAIN_DISK_PRIVATE(disk)->blockJobSync = false; } diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h index 81e893e..775ce95 100644 --- a/src/qemu/qemu_blockjob.h +++ b/src/qemu/qemu_blockjob.h @@ -37,16 +37,6 @@ void qemuBlockJobEventProcess(virQEMUDriverPtr driver, 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); + virDomainDiskDefPtr disk); #endif /* __QEMU_BLOCKJOB_H__ */ diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0682390..0b5ebe1 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -413,7 +413,6 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, static virClassPtr qemuDomainDiskPrivateClass; -static void qemuDomainDiskPrivateDispose(void *obj); static int qemuDomainDiskPrivateOnceInit(void) @@ -421,7 +420,7 @@ qemuDomainDiskPrivateOnceInit(void) qemuDomainDiskPrivateClass = virClassNew(virClassForObject(), "qemuDomainDiskPrivate", sizeof(qemuDomainDiskPrivate), - qemuDomainDiskPrivateDispose); + NULL); if (!qemuDomainDiskPrivateClass) return -1; else @@ -441,23 +440,9 @@ qemuDomainDiskPrivateNew(void) if (!(priv = virObjectNew(qemuDomainDiskPrivateClass))) return NULL; - if (virCondInit(&priv->blockJobSyncCond) < 0) { - virReportSystemError(errno, "%s", _("Failed to initialize condition")); - virObjectUnref(priv); - return NULL; - } - return (virObjectPtr) priv; } -static void -qemuDomainDiskPrivateDispose(void *obj) -{ - qemuDomainDiskPrivatePtr priv = obj; - - virCondDestroy(&priv->blockJobSyncCond); -} - static void * qemuDomainObjPrivateAlloc(void) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 053607f..9003c9b 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -214,7 +214,6 @@ struct _qemuDomainDiskPrivate { bool blockjob; /* for some synchronous block jobs, we need to notify the owner */ - virCond blockJobSyncCond; int blockJobType; /* type of the block job from the event */ int blockJobStatus; /* status of the finished block job */ bool blockJobSync; /* the block job needs synchronized termination */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 34e5581..0214e6b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16450,10 +16450,8 @@ qemuDomainBlockJobAbort(virDomainPtr dom, goto endjob; } - if (modern && !async) { - /* prepare state for event delivery */ + if (modern && !async) qemuBlockJobSyncBegin(disk); - } if (pivot) { if ((ret = qemuDomainBlockPivot(driver, vm, device, disk)) < 0) @@ -16501,21 +16499,21 @@ qemuDomainBlockJobAbort(virDomainPtr dom, VIR_DOMAIN_BLOCK_JOB_TYPE_PULL, VIR_DOMAIN_BLOCK_JOB_CANCELED); } else { - 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; + qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + qemuBlockJobUpdate(driver, vm, disk); + while (diskPriv->blockjob) { + if (virDomainObjWait(vm) < 0) { + ret = -1; + goto endjob; + } + qemuBlockJobUpdate(driver, vm, disk); } } } endjob: - if (disk && QEMU_DOMAIN_DISK_PRIVATE(disk)->blockJobSync) - qemuBlockJobSyncEnd(driver, vm, disk, NULL); + if (disk) + qemuBlockJobSyncEnd(driver, vm, disk); qemuDomainObjEndJob(driver, vm); cleanup: diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 8d01468..83d6c22 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1720,7 +1720,7 @@ qemuMigrationStopNBDServer(virQEMUDriverPtr driver, /** - * qemuMigrationCheckDriveMirror: + * qemuMigrationDriveMirrorReady: * @driver: qemu driver * @vm: domain * @@ -1733,37 +1733,39 @@ qemuMigrationStopNBDServer(virQEMUDriverPtr driver, * -1 on error. */ static int -qemuMigrationCheckDriveMirror(virQEMUDriverPtr driver, +qemuMigrationDriveMirrorReady(virQEMUDriverPtr driver, virDomainObjPtr vm) { size_t i; - int ret = 1; + size_t notReady = 0; + int status; for (i = 0; i < vm->def->ndisks; i++) { virDomainDiskDefPtr disk = vm->def->disks[i]; qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); - if (!diskPriv->migrating || !diskPriv->blockJobSync) + if (!diskPriv->migrating) 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: + status = qemuBlockJobUpdate(driver, vm, disk); + if (status == VIR_DOMAIN_BLOCK_JOB_FAILED) { virReportError(VIR_ERR_OPERATION_FAILED, _("migration of disk %s failed"), disk->dst); return -1; } + + if (disk->mirrorState != VIR_DOMAIN_DISK_MIRROR_STATE_READY) + notReady++; } - return ret; + if (notReady) { + VIR_DEBUG("Waiting for %zu disk mirrors to get ready", notReady); + return 0; + } else { + VIR_DEBUG("All disk mirrors are ready"); + return 1; + } } @@ -1823,18 +1825,17 @@ qemuMigrationCancelOneDriveMirror(virQEMUDriverPtr driver, /* 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); + while (1) { + status = qemuBlockJobUpdate(driver, vm, disk); + if (status != -1 && status != VIR_DOMAIN_BLOCK_JOB_READY) + break; + if ((ret = virDomainObjWait(vm)) < 0) goto endjob; - } - } while (status == VIR_DOMAIN_BLOCK_JOB_READY); + } } endjob: - qemuBlockJobSyncEnd(driver, vm, disk, NULL); + qemuBlockJobSyncEnd(driver, vm, disk); if (disk->mirrorState == VIR_DOMAIN_DISK_MIRROR_STATE_ABORT) disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; @@ -1924,6 +1925,9 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, char *nbd_dest = NULL; char *hoststr = NULL; unsigned int mirror_flags = VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT; + int rv; + + VIR_DEBUG("Starting drive mirrors for domain %s", vm->def->name); /* steal NBD port and thus prevent its propagation back to destination */ port = mig->nbd->port; @@ -1950,60 +1954,46 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, !virDomainDiskGetSource(disk)) continue; - VIR_FREE(diskAlias); - VIR_FREE(nbd_dest); if ((virAsprintf(&diskAlias, "%s%s", QEMU_DRIVE_HOST_PREFIX, disk->info.alias) < 0) || (virAsprintf(&nbd_dest, "nbd:%s:%d:exportname=%s", hoststr, port, diskAlias) < 0)) goto cleanup; + if (qemuDomainObjEnterMonitorAsync(driver, vm, + QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) + goto cleanup; + qemuBlockJobSyncBegin(disk); - - if (qemuDomainObjEnterMonitorAsync(driver, vm, - 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); + VIR_FREE(diskAlias); + VIR_FREE(nbd_dest); if (qemuDomainObjExitMonitor(driver, vm) < 0 || mon_ret < 0) { - qemuBlockJobSyncEnd(driver, vm, disk, NULL); + qemuBlockJobSyncEnd(driver, vm, disk); goto cleanup; } diskPriv->migrating = true; } - /* 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]; - qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); - - if (!diskPriv->migrating) - 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. */ - 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 cleanup; - } - - if (qemuBlockJobSyncWaitWithTimeout(driver, vm, disk, - 500ull, NULL) < 0) - goto cleanup; - - if (qemuMigrationCheckDriveMirror(driver, vm) < 0) - goto cleanup; + while ((rv = qemuMigrationDriveMirrorReady(driver, vm)) != 1) { + unsigned long long now; + + if (rv < 0) + goto cleanup; + + 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 cleanup; } + + if (virTimeMillisNow(&now) < 0 || + virDomainObjWaitUntil(vm, now + 500) < 0) + goto cleanup; } /* Okay, all disks are ready. Modify migrate_flags */ @@ -4092,7 +4082,7 @@ qemuMigrationRun(virQEMUDriverPtr driver, /* Confirm state of drive mirrors */ if (mig->nbd) { - if (qemuMigrationCheckDriveMirror(driver, vm) != 1) { + if (qemuMigrationDriveMirrorReady(driver, vm) != 1) { ret = -1; goto cancel; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 64ee049..3c9d4bc 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1001,10 +1001,10 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); if (diskPriv->blockJobSync) { + /* We have a SYNC API waiting for this event, dispatch it back */ diskPriv->blockJobType = type; diskPriv->blockJobStatus = status; - /* We have an SYNC API waiting for this event, dispatch it back */ - virCondSignal(&diskPriv->blockJobSyncCond); + virDomainObjSignal(vm); } else { /* there is no waiting SYNC API, dispatch the update to a thread */ if (VIR_ALLOC(processEvent) < 0) @@ -5055,13 +5055,8 @@ 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++) { - qemuDomainDiskPrivatePtr diskPriv = - QEMU_DOMAIN_DISK_PRIVATE(vm->def->disks[i]); - if (diskPriv->blockJobSync && diskPriv->blockJobStatus == -1) - virCondSignal(&diskPriv->blockJobSyncCond); - } + /* Wake up anything waiting on domain condition */ + virDomainObjBroadcast(vm); if ((logfile = qemuDomainCreateLog(driver, vm, true)) < 0) { /* To not break the normal domain shutdown process, skip the -- 2.4.3