--- src/qemu/qemu_domain.c | 104 +++++++++++++++++++++++++------------ src/qemu/qemu_domain.h | 25 +++++++--- src/qemu/qemu_driver.c | 82 +++++++++++++++--------------- src/qemu/qemu_migration.c | 123 ++++++++++++++++++++++----------------------- src/qemu/qemu_process.c | 5 +- 5 files changed, 192 insertions(+), 147 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 4b65d87..bbdfdc4 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -79,6 +79,42 @@ void qemuDomainEventQueue(struct qemud_driver *driver, } +static int +qemuDomainObjInitJob(qemuDomainObjPrivatePtr priv) +{ + memset(&priv->job, 0, sizeof(priv->job)); + + if (virCondInit(&priv->job.cond) < 0) + return -1; + + if (virCondInit(&priv->job.signalCond) < 0) { + ignore_value(virCondDestroy(&priv->job.cond)); + return -1; + } + + return 0; +} + +static void +qemuDomainObjResetJob(qemuDomainObjPrivatePtr priv) +{ + struct qemuDomainJobObj *job = &priv->job; + + job->active = QEMU_JOB_NONE; + job->start = 0; + memset(&job->info, 0, sizeof(job->info)); + job->signals = 0; + memset(&job->signalsData, 0, sizeof(job->signalsData)); +} + +static void +qemuDomainObjFreeJob(qemuDomainObjPrivatePtr priv) +{ + ignore_value(virCondDestroy(&priv->job.cond)); + ignore_value(virCondDestroy(&priv->job.signalCond)); +} + + static void *qemuDomainObjPrivateAlloc(void) { qemuDomainObjPrivatePtr priv; @@ -86,19 +122,10 @@ static void *qemuDomainObjPrivateAlloc(void) if (VIR_ALLOC(priv) < 0) return NULL; - if (virCondInit(&priv->jobCond) < 0) - goto initfail; - - if (virCondInit(&priv->signalCond) < 0) { - ignore_value(virCondDestroy(&priv->jobCond)); - goto initfail; - } + if (qemuDomainObjInitJob(priv) < 0) + VIR_FREE(priv); return priv; - -initfail: - VIR_FREE(priv); - return NULL; } static void qemuDomainObjPrivateFree(void *data) @@ -109,9 +136,8 @@ static void qemuDomainObjPrivateFree(void *data) qemuDomainPCIAddressSetFree(priv->pciaddrs); virDomainChrSourceDefFree(priv->monConfig); + qemuDomainObjFreeJob(priv); VIR_FREE(priv->vcpupids); - ignore_value(virCondDestroy(&priv->jobCond)); - ignore_value(virCondDestroy(&priv->signalCond)); VIR_FREE(priv->lockState); /* This should never be non-NULL if we get here, but just in case... */ @@ -473,6 +499,24 @@ void qemuDomainSetNamespaceHooks(virCapsPtr caps) caps->ns.href = qemuDomainDefNamespaceHref; } +void +qemuDomainObjSetJob(virDomainObjPtr obj, + enum qemuDomainJob job) +{ + qemuDomainObjPrivatePtr priv = obj->privateData; + + priv->job.active = job; +} + +void +qemuDomainObjDiscardJob(virDomainObjPtr obj) +{ + qemuDomainObjPrivatePtr priv = obj->privateData; + + qemuDomainObjResetJob(priv); + qemuDomainObjSetJob(obj, QEMU_JOB_NONE); +} + /* * obj must be locked before calling, qemud_driver must NOT be locked * @@ -498,8 +542,8 @@ int qemuDomainObjBeginJob(virDomainObjPtr obj) virDomainObjRef(obj); - while (priv->jobActive) { - if (virCondWaitUntil(&priv->jobCond, &obj->lock, then) < 0) { + while (priv->job.active) { + if (virCondWaitUntil(&priv->job.cond, &obj->lock, then) < 0) { /* Safe to ignore value since ref count was incremented above */ ignore_value(virDomainObjUnref(obj)); if (errno == ETIMEDOUT) @@ -511,11 +555,9 @@ int qemuDomainObjBeginJob(virDomainObjPtr obj) return -1; } } - priv->jobActive = QEMU_JOB_UNSPECIFIED; - priv->jobSignals = 0; - memset(&priv->jobSignalsData, 0, sizeof(priv->jobSignalsData)); - priv->jobStart = now; - memset(&priv->jobInfo, 0, sizeof(priv->jobInfo)); + qemuDomainObjResetJob(priv); + qemuDomainObjSetJob(obj, QEMU_JOB_UNSPECIFIED); + priv->job.start = now; return 0; } @@ -540,8 +582,8 @@ int qemuDomainObjBeginJobWithDriver(struct qemud_driver *driver, virDomainObjRef(obj); qemuDriverUnlock(driver); - while (priv->jobActive) { - if (virCondWaitUntil(&priv->jobCond, &obj->lock, then) < 0) { + while (priv->job.active) { + if (virCondWaitUntil(&priv->job.cond, &obj->lock, then) < 0) { if (errno == ETIMEDOUT) qemuReportError(VIR_ERR_OPERATION_TIMEOUT, "%s", _("cannot acquire state change lock")); @@ -556,11 +598,9 @@ int qemuDomainObjBeginJobWithDriver(struct qemud_driver *driver, return -1; } } - priv->jobActive = QEMU_JOB_UNSPECIFIED; - priv->jobSignals = 0; - memset(&priv->jobSignalsData, 0, sizeof(priv->jobSignalsData)); - priv->jobStart = now; - memset(&priv->jobInfo, 0, sizeof(priv->jobInfo)); + qemuDomainObjResetJob(priv); + qemuDomainObjSetJob(obj, QEMU_JOB_UNSPECIFIED); + priv->job.start = now; virDomainObjUnlock(obj); qemuDriverLock(driver); @@ -582,17 +622,13 @@ int qemuDomainObjEndJob(virDomainObjPtr obj) { qemuDomainObjPrivatePtr priv = obj->privateData; - priv->jobActive = QEMU_JOB_NONE; - priv->jobSignals = 0; - memset(&priv->jobSignalsData, 0, sizeof(priv->jobSignalsData)); - priv->jobStart = 0; - memset(&priv->jobInfo, 0, sizeof(priv->jobInfo)); - virCondSignal(&priv->jobCond); + qemuDomainObjResetJob(priv); + qemuDomainObjSetJob(obj, QEMU_JOB_NONE); + virCondSignal(&priv->job.cond); return virDomainObjUnref(obj); } - /* * obj must be locked before calling, qemud_driver must be unlocked * diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index f282df2..214e578 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -62,19 +62,26 @@ struct qemuDomainJobSignalsData { int *infoRetCode; /* Return code for the blkinfo calls */ }; +struct qemuDomainJobObj { + virCond cond; /* Use in conjunction with main virDomainObjPtr lock */ + virCond signalCond; /* Use to coordinate the safe queries during migration */ + + enum qemuDomainJob active; /* Currently running job */ + + unsigned long long start; /* When the job started */ + virDomainJobInfo info; /* Progress data */ + + unsigned int signals; /* Signals for running job */ + struct qemuDomainJobSignalsData signalsData; /* Signal specific data */ +}; + typedef struct _qemuDomainPCIAddressSet qemuDomainPCIAddressSet; typedef qemuDomainPCIAddressSet *qemuDomainPCIAddressSetPtr; typedef struct _qemuDomainObjPrivate qemuDomainObjPrivate; typedef qemuDomainObjPrivate *qemuDomainObjPrivatePtr; struct _qemuDomainObjPrivate { - virCond jobCond; /* Use in conjunction with main virDomainObjPtr lock */ - virCond signalCond; /* Use to coordinate the safe queries during migration */ - enum qemuDomainJob jobActive; /* Currently running job */ - unsigned int jobSignals; /* Signals for running job */ - struct qemuDomainJobSignalsData jobSignalsData; /* Signal specific data */ - virDomainJobInfo jobInfo; - unsigned long long jobStart; + struct qemuDomainJobObj job; qemuMonitorPtr mon; virDomainChrSourceDefPtr monConfig; @@ -114,6 +121,10 @@ int qemuDomainObjBeginJob(virDomainObjPtr obj) ATTRIBUTE_RETURN_CHECK; int qemuDomainObjBeginJobWithDriver(struct qemud_driver *driver, virDomainObjPtr obj) ATTRIBUTE_RETURN_CHECK; int qemuDomainObjEndJob(virDomainObjPtr obj) ATTRIBUTE_RETURN_CHECK; + +void qemuDomainObjSetJob(virDomainObjPtr obj, enum qemuDomainJob job); +void qemuDomainObjDiscardJob(virDomainObjPtr obj); + void qemuDomainObjEnterMonitor(virDomainObjPtr obj); void qemuDomainObjExitMonitor(virDomainObjPtr obj); void qemuDomainObjEnterMonitorWithDriver(struct qemud_driver *driver, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 52b7dfd..e9cdcbe 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1343,11 +1343,11 @@ static int qemudDomainSuspend(virDomainPtr dom) { priv = vm->privateData; - if (priv->jobActive == QEMU_JOB_MIGRATION_OUT) { + if (priv->job.active == QEMU_JOB_MIGRATION_OUT) { if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_PAUSED) { VIR_DEBUG("Requesting domain pause on %s", vm->def->name); - priv->jobSignals |= QEMU_JOB_SIGNAL_SUSPEND; + priv->job.signals |= QEMU_JOB_SIGNAL_SUSPEND; } ret = 0; goto cleanup; @@ -1878,7 +1878,7 @@ static int qemudDomainGetInfo(virDomainPtr dom, if ((vm->def->memballoon != NULL) && (vm->def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_NONE)) { info->memory = vm->def->mem.max_balloon; - } else if (!priv->jobActive) { + } else if (!priv->job.active) { if (qemuDomainObjBeginJob(vm) < 0) goto cleanup; if (!virDomainObjIsActive(vm)) @@ -1985,12 +1985,12 @@ qemuDomainGetControlInfo(virDomainPtr dom, if (priv->monError) { info->state = VIR_DOMAIN_CONTROL_ERROR; - } else if (priv->jobActive) { + } else if (priv->job.active) { if (!priv->monStart) { info->state = VIR_DOMAIN_CONTROL_JOB; if (virTimeMs(&info->stateTime) < 0) goto cleanup; - info->stateTime -= priv->jobStart; + info->stateTime -= priv->job.start; } else { info->state = VIR_DOMAIN_CONTROL_OCCUPIED; if (virTimeMs(&info->stateTime) < 0) @@ -2125,10 +2125,10 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) goto cleanup; - priv->jobActive = QEMU_JOB_SAVE; + qemuDomainObjSetJob(vm, QEMU_JOB_SAVE); - memset(&priv->jobInfo, 0, sizeof(priv->jobInfo)); - priv->jobInfo.type = VIR_DOMAIN_JOB_UNBOUNDED; + memset(&priv->job.info, 0, sizeof(priv->job.info)); + priv->job.info.type = VIR_DOMAIN_JOB_UNBOUNDED; /* Pause */ if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { @@ -2617,7 +2617,7 @@ static int qemudDomainCoreDump(virDomainPtr dom, goto endjob; } - priv->jobActive = QEMU_JOB_DUMP; + qemuDomainObjSetJob(vm, QEMU_JOB_DUMP); /* Migrate will always stop the VM, so the resume condition is independent of whether the stop command is issued. */ @@ -3847,7 +3847,7 @@ static char *qemuDomainGetXMLDesc(virDomainPtr dom, qemuDomainObjPrivatePtr priv = vm->privateData; /* Don't delay if someone's using the monitor, just use * existing most recent data instead */ - if (!priv->jobActive) { + if (!priv->job.active) { if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) goto cleanup; @@ -6006,19 +6006,19 @@ qemudDomainBlockStats (virDomainPtr dom, } priv = vm->privateData; - if ((priv->jobActive == QEMU_JOB_MIGRATION_OUT) - || (priv->jobActive == QEMU_JOB_SAVE)) { + if ((priv->job.active == QEMU_JOB_MIGRATION_OUT) + || (priv->job.active == QEMU_JOB_SAVE)) { virDomainObjRef(vm); - while (priv->jobSignals & QEMU_JOB_SIGNAL_BLKSTAT) - ignore_value(virCondWait(&priv->signalCond, &vm->lock)); + while (priv->job.signals & QEMU_JOB_SIGNAL_BLKSTAT) + ignore_value(virCondWait(&priv->job.signalCond, &vm->lock)); - priv->jobSignalsData.statDevName = disk->info.alias; - priv->jobSignalsData.blockStat = stats; - priv->jobSignalsData.statRetCode = &ret; - priv->jobSignals |= QEMU_JOB_SIGNAL_BLKSTAT; + priv->job.signalsData.statDevName = disk->info.alias; + priv->job.signalsData.blockStat = stats; + priv->job.signalsData.statRetCode = &ret; + priv->job.signals |= QEMU_JOB_SIGNAL_BLKSTAT; - while (priv->jobSignals & QEMU_JOB_SIGNAL_BLKSTAT) - ignore_value(virCondWait(&priv->signalCond, &vm->lock)); + while (priv->job.signals & QEMU_JOB_SIGNAL_BLKSTAT) + ignore_value(virCondWait(&priv->job.signalCond, &vm->lock)); if (virDomainObjUnref(vm) == 0) vm = NULL; @@ -6453,19 +6453,19 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom, virDomainObjIsActive(vm)) { qemuDomainObjPrivatePtr priv = vm->privateData; - if ((priv->jobActive == QEMU_JOB_MIGRATION_OUT) - || (priv->jobActive == QEMU_JOB_SAVE)) { + if ((priv->job.active == QEMU_JOB_MIGRATION_OUT) + || (priv->job.active == QEMU_JOB_SAVE)) { virDomainObjRef(vm); - while (priv->jobSignals & QEMU_JOB_SIGNAL_BLKINFO) - ignore_value(virCondWait(&priv->signalCond, &vm->lock)); + while (priv->job.signals & QEMU_JOB_SIGNAL_BLKINFO) + ignore_value(virCondWait(&priv->job.signalCond, &vm->lock)); - priv->jobSignalsData.infoDevName = disk->info.alias; - priv->jobSignalsData.blockInfo = info; - priv->jobSignalsData.infoRetCode = &ret; - priv->jobSignals |= QEMU_JOB_SIGNAL_BLKINFO; + priv->job.signalsData.infoDevName = disk->info.alias; + priv->job.signalsData.blockInfo = info; + priv->job.signalsData.infoRetCode = &ret; + priv->job.signals |= QEMU_JOB_SIGNAL_BLKINFO; - while (priv->jobSignals & QEMU_JOB_SIGNAL_BLKINFO) - ignore_value(virCondWait(&priv->signalCond, &vm->lock)); + while (priv->job.signals & QEMU_JOB_SIGNAL_BLKINFO) + ignore_value(virCondWait(&priv->job.signalCond, &vm->lock)); if (virDomainObjUnref(vm) == 0) vm = NULL; @@ -7293,8 +7293,8 @@ static int qemuDomainGetJobInfo(virDomainPtr dom, priv = vm->privateData; if (virDomainObjIsActive(vm)) { - if (priv->jobActive) { - memcpy(info, &priv->jobInfo, sizeof(*info)); + if (priv->job.active) { + memcpy(info, &priv->job.info, sizeof(*info)); /* Refresh elapsed time again just to ensure it * is fully updated. This is primarily for benefit @@ -7303,7 +7303,7 @@ static int qemuDomainGetJobInfo(virDomainPtr dom, */ if (virTimeMs(&info->timeElapsed) < 0) goto cleanup; - info->timeElapsed -= priv->jobStart; + info->timeElapsed -= priv->job.start; } else { memset(info, 0, sizeof(*info)); info->type = VIR_DOMAIN_JOB_NONE; @@ -7343,9 +7343,9 @@ static int qemuDomainAbortJob(virDomainPtr dom) { priv = vm->privateData; if (virDomainObjIsActive(vm)) { - if (priv->jobActive) { + if (priv->job.active) { VIR_DEBUG("Requesting cancellation of job on vm %s", vm->def->name); - priv->jobSignals |= QEMU_JOB_SIGNAL_CANCEL; + priv->job.signals |= QEMU_JOB_SIGNAL_CANCEL; } else { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("no job is active on the domain")); @@ -7397,15 +7397,15 @@ qemuDomainMigrateSetMaxDowntime(virDomainPtr dom, priv = vm->privateData; - if (priv->jobActive != QEMU_JOB_MIGRATION_OUT) { + if (priv->job.active != QEMU_JOB_MIGRATION_OUT) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not being migrated")); goto cleanup; } VIR_DEBUG("Requesting migration downtime change to %llums", downtime); - priv->jobSignalsData.migrateDowntime = downtime; - priv->jobSignals |= QEMU_JOB_SIGNAL_MIGRATE_DOWNTIME; + priv->job.signalsData.migrateDowntime = downtime; + priv->job.signals |= QEMU_JOB_SIGNAL_MIGRATE_DOWNTIME; ret = 0; cleanup: @@ -7446,15 +7446,15 @@ qemuDomainMigrateSetMaxSpeed(virDomainPtr dom, priv = vm->privateData; - if (priv->jobActive != QEMU_JOB_MIGRATION_OUT) { + if (priv->job.active != QEMU_JOB_MIGRATION_OUT) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not being migrated")); goto cleanup; } VIR_DEBUG("Requesting migration speed change to %luMbs", bandwidth); - priv->jobSignalsData.migrateBandwidth = bandwidth; - priv->jobSignals |= QEMU_JOB_SIGNAL_MIGRATE_SPEED; + priv->job.signalsData.migrateBandwidth = bandwidth; + priv->job.signals |= QEMU_JOB_SIGNAL_MIGRATE_SPEED; ret = 0; cleanup: diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index d7b27a0..3634966 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -742,12 +742,12 @@ qemuMigrationProcessJobSignals(struct qemud_driver *driver, qemuReportError(VIR_ERR_INTERNAL_ERROR, _("%s: %s"), job, _("guest unexpectedly quit")); if (cleanup) - priv->jobSignals = 0; + priv->job.signals = 0; return -1; } - if (priv->jobSignals & QEMU_JOB_SIGNAL_CANCEL) { - priv->jobSignals ^= QEMU_JOB_SIGNAL_CANCEL; + if (priv->job.signals & QEMU_JOB_SIGNAL_CANCEL) { + priv->job.signals ^= QEMU_JOB_SIGNAL_CANCEL; VIR_DEBUG("Cancelling job at client request"); qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorMigrateCancel(priv->mon); @@ -755,58 +755,58 @@ qemuMigrationProcessJobSignals(struct qemud_driver *driver, if (ret < 0) { VIR_WARN("Unable to cancel job"); } - } else if (priv->jobSignals & QEMU_JOB_SIGNAL_SUSPEND) { - priv->jobSignals ^= QEMU_JOB_SIGNAL_SUSPEND; + } else if (priv->job.signals & QEMU_JOB_SIGNAL_SUSPEND) { + priv->job.signals ^= QEMU_JOB_SIGNAL_SUSPEND; VIR_DEBUG("Pausing domain for non-live migration"); if (qemuMigrationSetOffline(driver, vm) < 0) VIR_WARN("Unable to pause domain"); - } else if (priv->jobSignals & QEMU_JOB_SIGNAL_MIGRATE_DOWNTIME) { - unsigned long long ms = priv->jobSignalsData.migrateDowntime; + } else if (priv->job.signals & QEMU_JOB_SIGNAL_MIGRATE_DOWNTIME) { + unsigned long long ms = priv->job.signalsData.migrateDowntime; - priv->jobSignals ^= QEMU_JOB_SIGNAL_MIGRATE_DOWNTIME; - priv->jobSignalsData.migrateDowntime = 0; + priv->job.signals ^= QEMU_JOB_SIGNAL_MIGRATE_DOWNTIME; + priv->job.signalsData.migrateDowntime = 0; VIR_DEBUG("Setting migration downtime to %llums", ms); qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorSetMigrationDowntime(priv->mon, ms); qemuDomainObjExitMonitorWithDriver(driver, vm); if (ret < 0) VIR_WARN("Unable to set migration downtime"); - } else if (priv->jobSignals & QEMU_JOB_SIGNAL_MIGRATE_SPEED) { - unsigned long bandwidth = priv->jobSignalsData.migrateBandwidth; + } else if (priv->job.signals & QEMU_JOB_SIGNAL_MIGRATE_SPEED) { + unsigned long bandwidth = priv->job.signalsData.migrateBandwidth; - priv->jobSignals ^= QEMU_JOB_SIGNAL_MIGRATE_SPEED; - priv->jobSignalsData.migrateBandwidth = 0; + priv->job.signals ^= QEMU_JOB_SIGNAL_MIGRATE_SPEED; + priv->job.signalsData.migrateBandwidth = 0; VIR_DEBUG("Setting migration bandwidth to %luMbs", bandwidth); qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorSetMigrationSpeed(priv->mon, bandwidth); qemuDomainObjExitMonitorWithDriver(driver, vm); if (ret < 0) VIR_WARN("Unable to set migration speed"); - } else if (priv->jobSignals & QEMU_JOB_SIGNAL_BLKSTAT) { + } else if (priv->job.signals & QEMU_JOB_SIGNAL_BLKSTAT) { qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorGetBlockStatsInfo(priv->mon, - priv->jobSignalsData.statDevName, - &priv->jobSignalsData.blockStat->rd_req, - &priv->jobSignalsData.blockStat->rd_bytes, - &priv->jobSignalsData.blockStat->wr_req, - &priv->jobSignalsData.blockStat->wr_bytes, - &priv->jobSignalsData.blockStat->errs); + priv->job.signalsData.statDevName, + &priv->job.signalsData.blockStat->rd_req, + &priv->job.signalsData.blockStat->rd_bytes, + &priv->job.signalsData.blockStat->wr_req, + &priv->job.signalsData.blockStat->wr_bytes, + &priv->job.signalsData.blockStat->errs); qemuDomainObjExitMonitorWithDriver(driver, vm); - *priv->jobSignalsData.statRetCode = ret; - priv->jobSignals ^= QEMU_JOB_SIGNAL_BLKSTAT; + *priv->job.signalsData.statRetCode = ret; + priv->job.signals ^= QEMU_JOB_SIGNAL_BLKSTAT; if (ret < 0) VIR_WARN("Unable to get block statistics"); - } else if (priv->jobSignals & QEMU_JOB_SIGNAL_BLKINFO) { + } else if (priv->job.signals & QEMU_JOB_SIGNAL_BLKINFO) { qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorGetBlockExtent(priv->mon, - priv->jobSignalsData.infoDevName, - &priv->jobSignalsData.blockInfo->allocation); + priv->job.signalsData.infoDevName, + &priv->job.signalsData.blockInfo->allocation); qemuDomainObjExitMonitorWithDriver(driver, vm); - *priv->jobSignalsData.infoRetCode = ret; - priv->jobSignals ^= QEMU_JOB_SIGNAL_BLKINFO; + *priv->job.signalsData.infoRetCode = ret; + priv->job.signals ^= QEMU_JOB_SIGNAL_BLKINFO; if (ret < 0) VIR_WARN("Unable to get block information"); @@ -844,44 +844,44 @@ qemuMigrationUpdateJobStatus(struct qemud_driver *driver, &memTotal); qemuDomainObjExitMonitorWithDriver(driver, vm); - if (ret < 0 || virTimeMs(&priv->jobInfo.timeElapsed) < 0) { - priv->jobInfo.type = VIR_DOMAIN_JOB_FAILED; + if (ret < 0 || virTimeMs(&priv->job.info.timeElapsed) < 0) { + priv->job.info.type = VIR_DOMAIN_JOB_FAILED; return -1; } - priv->jobInfo.timeElapsed -= priv->jobStart; + priv->job.info.timeElapsed -= priv->job.start; switch (status) { case QEMU_MONITOR_MIGRATION_STATUS_INACTIVE: - priv->jobInfo.type = VIR_DOMAIN_JOB_NONE; + priv->job.info.type = VIR_DOMAIN_JOB_NONE; qemuReportError(VIR_ERR_OPERATION_FAILED, _("%s: %s"), job, _("is not active")); break; case QEMU_MONITOR_MIGRATION_STATUS_ACTIVE: - priv->jobInfo.dataTotal = memTotal; - priv->jobInfo.dataRemaining = memRemaining; - priv->jobInfo.dataProcessed = memProcessed; + priv->job.info.dataTotal = memTotal; + priv->job.info.dataRemaining = memRemaining; + priv->job.info.dataProcessed = memProcessed; - priv->jobInfo.memTotal = memTotal; - priv->jobInfo.memRemaining = memRemaining; - priv->jobInfo.memProcessed = memProcessed; + priv->job.info.memTotal = memTotal; + priv->job.info.memRemaining = memRemaining; + priv->job.info.memProcessed = memProcessed; ret = 0; break; case QEMU_MONITOR_MIGRATION_STATUS_COMPLETED: - priv->jobInfo.type = VIR_DOMAIN_JOB_COMPLETED; + priv->job.info.type = VIR_DOMAIN_JOB_COMPLETED; ret = 0; break; case QEMU_MONITOR_MIGRATION_STATUS_ERROR: - priv->jobInfo.type = VIR_DOMAIN_JOB_FAILED; + priv->job.info.type = VIR_DOMAIN_JOB_FAILED; qemuReportError(VIR_ERR_OPERATION_FAILED, _("%s: %s"), job, _("unexpectedly failed")); break; case QEMU_MONITOR_MIGRATION_STATUS_CANCELLED: - priv->jobInfo.type = VIR_DOMAIN_JOB_CANCELLED; + priv->job.info.type = VIR_DOMAIN_JOB_CANCELLED; qemuReportError(VIR_ERR_OPERATION_FAILED, _("%s: %s"), job, _("canceled by client")); break; @@ -897,7 +897,7 @@ qemuMigrationWaitForCompletion(struct qemud_driver *driver, virDomainObjPtr vm) qemuDomainObjPrivatePtr priv = vm->privateData; const char *job; - switch (priv->jobActive) { + switch (priv->job.active) { case QEMU_JOB_MIGRATION_OUT: job = _("migration job"); break; @@ -911,17 +911,17 @@ qemuMigrationWaitForCompletion(struct qemud_driver *driver, virDomainObjPtr vm) job = _("job"); } - priv->jobInfo.type = VIR_DOMAIN_JOB_UNBOUNDED; + priv->job.info.type = VIR_DOMAIN_JOB_UNBOUNDED; - while (priv->jobInfo.type == VIR_DOMAIN_JOB_UNBOUNDED) { + while (priv->job.info.type == VIR_DOMAIN_JOB_UNBOUNDED) { /* Poll every 50ms for progress & to allow cancellation */ struct timespec ts = { .tv_sec = 0, .tv_nsec = 50 * 1000 * 1000ull }; - while (priv->jobSignals) { + while (priv->job.signals) { if (qemuMigrationProcessJobSignals(driver, vm, job, false) < 0) goto cleanup; } - virCondSignal(&priv->signalCond); + virCondSignal(&priv->job.signalCond); if (qemuMigrationUpdateJobStatus(driver, vm, job) < 0) goto cleanup; @@ -937,12 +937,12 @@ qemuMigrationWaitForCompletion(struct qemud_driver *driver, virDomainObjPtr vm) } cleanup: - while (priv->jobSignals) { + while (priv->job.signals) { qemuMigrationProcessJobSignals(driver, vm, job, true); } - virCondBroadcast(&priv->signalCond); + virCondBroadcast(&priv->job.signalCond); - if (priv->jobInfo.type == VIR_DOMAIN_JOB_COMPLETED) + if (priv->job.info.type == VIR_DOMAIN_JOB_COMPLETED) return 0; else return -1; @@ -1110,7 +1110,7 @@ qemuMigrationPrepareTunnel(struct qemud_driver *driver, if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) goto cleanup; - priv->jobActive = QEMU_JOB_MIGRATION_IN; + qemuDomainObjSetJob(vm, QEMU_JOB_MIGRATION_IN); /* Domain starts inactive, even if the domain XML had an id field. */ vm->def->id = -1; @@ -1183,9 +1183,9 @@ endjob: */ if (vm && virDomainObjIsActive(vm)) { - priv->jobActive = QEMU_JOB_MIGRATION_IN; - priv->jobInfo.type = VIR_DOMAIN_JOB_UNBOUNDED; - priv->jobStart = now; + qemuDomainObjSetJob(vm, QEMU_JOB_MIGRATION_IN); + priv->job.info.type = VIR_DOMAIN_JOB_UNBOUNDED; + priv->job.start = now; } cleanup: @@ -1345,7 +1345,7 @@ qemuMigrationPrepareDirect(struct qemud_driver *driver, if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) goto cleanup; - priv->jobActive = QEMU_JOB_MIGRATION_IN; + qemuDomainObjSetJob(vm, QEMU_JOB_MIGRATION_IN); /* Domain starts inactive, even if the domain XML had an id field. */ vm->def->id = -1; @@ -1404,9 +1404,9 @@ endjob: */ if (vm && virDomainObjIsActive(vm)) { - priv->jobActive = QEMU_JOB_MIGRATION_IN; - priv->jobInfo.type = VIR_DOMAIN_JOB_UNBOUNDED; - priv->jobStart = now; + qemuDomainObjSetJob(vm, QEMU_JOB_MIGRATION_IN); + priv->job.info.type = VIR_DOMAIN_JOB_UNBOUNDED; + priv->job.start = now; } cleanup: @@ -2286,7 +2286,7 @@ int qemuMigrationPerform(struct qemud_driver *driver, if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) goto cleanup; - priv->jobActive = QEMU_JOB_MIGRATION_OUT; + qemuDomainObjSetJob(vm, QEMU_JOB_MIGRATION_OUT); if (!virDomainObjIsActive(vm)) { qemuReportError(VIR_ERR_OPERATION_INVALID, @@ -2300,8 +2300,8 @@ int qemuMigrationPerform(struct qemud_driver *driver, goto endjob; } - memset(&priv->jobInfo, 0, sizeof(priv->jobInfo)); - priv->jobInfo.type = VIR_DOMAIN_JOB_UNBOUNDED; + memset(&priv->job.info, 0, sizeof(priv->job.info)); + priv->job.info.type = VIR_DOMAIN_JOB_UNBOUNDED; resume = virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING; @@ -2450,13 +2450,12 @@ qemuMigrationFinish(struct qemud_driver *driver, virErrorPtr orig_err = NULL; priv = vm->privateData; - if (priv->jobActive != QEMU_JOB_MIGRATION_IN) { + if (priv->job.active != QEMU_JOB_MIGRATION_IN) { qemuReportError(VIR_ERR_NO_DOMAIN, _("domain '%s' is not processing incoming migration"), vm->def->name); goto cleanup; } - priv->jobActive = QEMU_JOB_NONE; - memset(&priv->jobInfo, 0, sizeof(priv->jobInfo)); + qemuDomainObjDiscardJob(vm); if (!(mig = qemuMigrationEatCookie(driver, vm, cookiein, cookieinlen, 0))) goto cleanup; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index c9145cb..56593c2 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3013,11 +3013,10 @@ static void qemuProcessAutoDestroyDom(void *payload, } priv = dom->privateData; - if (priv->jobActive == QEMU_JOB_MIGRATION_IN) { + if (priv->job.active == QEMU_JOB_MIGRATION_IN) { VIR_DEBUG("vm=%s has incoming migration active, cancelling", dom->def->name); - priv->jobActive = QEMU_JOB_NONE; - memset(&priv->jobInfo, 0, sizeof(priv->jobInfo)); + qemuDomainObjDiscardJob(dom); } if (qemuDomainObjBeginJobWithDriver(data->driver, dom) < 0) -- 1.7.6

This avoids code duplication and also avoids relying on good luck that ignore_value(virDomainObjUnref(obj)) doesn't set errno. --- src/qemu/qemu_domain.c | 93 ++++++++++++++++++++++-------------------------- 1 files changed, 43 insertions(+), 50 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index bbdfdc4..8f3eaa7 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -517,20 +517,17 @@ qemuDomainObjDiscardJob(virDomainObjPtr obj) qemuDomainObjSetJob(obj, QEMU_JOB_NONE); } -/* - * obj must be locked before calling, qemud_driver must NOT be locked - * - * This must be called by anything that will change the VM state - * in any way, or anything that will use the QEMU monitor. - * - * Upon successful return, the object will have its ref count increased, - * successful calls must be followed by EndJob eventually - */ - /* Give up waiting for mutex after 30 seconds */ #define QEMU_JOB_WAIT_TIME (1000ull * 30) -int qemuDomainObjBeginJob(virDomainObjPtr obj) +/* + * obj must be locked before calling; driver_locked says if qemu_driver is + * locked or not. + */ +static int +qemuDomainObjBeginJobInternal(struct qemud_driver *driver, + bool driver_locked, + virDomainObjPtr obj) { qemuDomainObjPrivatePtr priv = obj->privateData; unsigned long long now; @@ -541,17 +538,24 @@ int qemuDomainObjBeginJob(virDomainObjPtr obj) then = now + QEMU_JOB_WAIT_TIME; virDomainObjRef(obj); + if (driver_locked) + qemuDriverUnlock(driver); while (priv->job.active) { if (virCondWaitUntil(&priv->job.cond, &obj->lock, then) < 0) { - /* Safe to ignore value since ref count was incremented above */ - ignore_value(virDomainObjUnref(obj)); if (errno == ETIMEDOUT) qemuReportError(VIR_ERR_OPERATION_TIMEOUT, "%s", _("cannot acquire state change lock")); else virReportSystemError(errno, "%s", _("cannot acquire job mutex")); + if (driver_locked) { + virDomainObjUnlock(obj); + qemuDriverLock(driver); + virDomainObjLock(obj); + } + /* Safe to ignore value since ref count was incremented above */ + ignore_value(virDomainObjUnref(obj)); return -1; } } @@ -559,54 +563,43 @@ int qemuDomainObjBeginJob(virDomainObjPtr obj) qemuDomainObjSetJob(obj, QEMU_JOB_UNSPECIFIED); priv->job.start = now; + if (driver_locked) { + virDomainObjUnlock(obj); + qemuDriverLock(driver); + virDomainObjLock(obj); + } + return 0; } /* - * obj must be locked before calling, qemud_driver must be locked + * obj must be locked before calling, qemud_driver must NOT be locked * * This must be called by anything that will change the VM state * in any way, or anything that will use the QEMU monitor. + * + * Upon successful return, the object will have its ref count increased, + * successful calls must be followed by EndJob eventually + */ +int qemuDomainObjBeginJob(virDomainObjPtr obj) +{ + return qemuDomainObjBeginJobInternal(NULL, false, obj); +} + +/* + * obj must be locked before calling. If qemud_driver is passed, it MUST be + * locked; otherwise it MUST NOT be locked. + * + * This must be called by anything that will change the VM state + * in any way, or anything that will use the QEMU monitor. + * + * Upon successful return, the object will have its ref count increased, + * successful calls must be followed by EndJob eventually */ int qemuDomainObjBeginJobWithDriver(struct qemud_driver *driver, virDomainObjPtr obj) { - qemuDomainObjPrivatePtr priv = obj->privateData; - unsigned long long now; - unsigned long long then; - - if (virTimeMs(&now) < 0) - return -1; - then = now + QEMU_JOB_WAIT_TIME; - - virDomainObjRef(obj); - qemuDriverUnlock(driver); - - while (priv->job.active) { - if (virCondWaitUntil(&priv->job.cond, &obj->lock, then) < 0) { - if (errno == ETIMEDOUT) - qemuReportError(VIR_ERR_OPERATION_TIMEOUT, - "%s", _("cannot acquire state change lock")); - else - virReportSystemError(errno, - "%s", _("cannot acquire job mutex")); - virDomainObjUnlock(obj); - qemuDriverLock(driver); - virDomainObjLock(obj); - /* Safe to ignore value since ref count was incremented above */ - ignore_value(virDomainObjUnref(obj)); - return -1; - } - } - qemuDomainObjResetJob(priv); - qemuDomainObjSetJob(obj, QEMU_JOB_UNSPECIFIED); - priv->job.start = now; - - virDomainObjUnlock(obj); - qemuDriverLock(driver); - virDomainObjLock(obj); - - return 0; + return qemuDomainObjBeginJobInternal(driver, true, obj); } /* -- 1.7.6

EnterMonitor and ExitMonitor methods are very similar to their *WithDriver variants; consolidate them into EnterMonitorInternal and ExitMonitorInternal to avoid (mainly future) code duplication. --- src/qemu/qemu_domain.c | 74 ++++++++++++++++++++++------------------------- 1 files changed, 35 insertions(+), 39 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8f3eaa7..a2e77b6 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -622,16 +622,10 @@ int qemuDomainObjEndJob(virDomainObjPtr obj) return virDomainObjUnref(obj); } -/* - * obj must be locked before calling, qemud_driver must be unlocked - * - * To be called immediately before any QEMU monitor API call - * Must have already called qemuDomainObjBeginJob(), and checked - * that the VM is still active. - * - * To be followed with qemuDomainObjExitMonitor() once complete - */ -void qemuDomainObjEnterMonitor(virDomainObjPtr obj) + +static void +qemuDomainObjEnterMonitorInternal(struct qemud_driver *driver, + virDomainObjPtr obj) { qemuDomainObjPrivatePtr priv = obj->privateData; @@ -639,14 +633,13 @@ void qemuDomainObjEnterMonitor(virDomainObjPtr obj) qemuMonitorRef(priv->mon); ignore_value(virTimeMs(&priv->monStart)); virDomainObjUnlock(obj); + if (driver) + qemuDriverUnlock(driver); } - -/* obj must NOT be locked before calling, qemud_driver must be unlocked - * - * Should be paired with an earlier qemuDomainObjEnterMonitor() call - */ -void qemuDomainObjExitMonitor(virDomainObjPtr obj) +static void +qemuDomainObjExitMonitorInternal(struct qemud_driver *driver, + virDomainObjPtr obj) { qemuDomainObjPrivatePtr priv = obj->privateData; int refs; @@ -656,6 +649,8 @@ void qemuDomainObjExitMonitor(virDomainObjPtr obj) if (refs > 0) qemuMonitorUnlock(priv->mon); + if (driver) + qemuDriverLock(driver); virDomainObjLock(obj); priv->monStart = 0; @@ -664,6 +659,28 @@ void qemuDomainObjExitMonitor(virDomainObjPtr obj) } } +/* + * obj must be locked before calling, qemud_driver must be unlocked + * + * To be called immediately before any QEMU monitor API call + * Must have already called qemuDomainObjBeginJob(), and checked + * that the VM is still active. + * + * To be followed with qemuDomainObjExitMonitor() once complete + */ +void qemuDomainObjEnterMonitor(virDomainObjPtr obj) +{ + qemuDomainObjEnterMonitorInternal(NULL, obj); +} + +/* obj must NOT be locked before calling, qemud_driver must be unlocked + * + * Should be paired with an earlier qemuDomainObjEnterMonitor() call + */ +void qemuDomainObjExitMonitor(virDomainObjPtr obj) +{ + qemuDomainObjExitMonitorInternal(NULL, obj); +} /* * obj must be locked before calling, qemud_driver must be locked @@ -676,16 +693,9 @@ void qemuDomainObjExitMonitor(virDomainObjPtr obj) void qemuDomainObjEnterMonitorWithDriver(struct qemud_driver *driver, virDomainObjPtr obj) { - qemuDomainObjPrivatePtr priv = obj->privateData; - - qemuMonitorLock(priv->mon); - qemuMonitorRef(priv->mon); - ignore_value(virTimeMs(&priv->monStart)); - virDomainObjUnlock(obj); - qemuDriverUnlock(driver); + qemuDomainObjEnterMonitorInternal(driver, obj); } - /* obj must NOT be locked before calling, qemud_driver must be unlocked, * and will be locked after returning * @@ -694,21 +704,7 @@ void qemuDomainObjEnterMonitorWithDriver(struct qemud_driver *driver, void qemuDomainObjExitMonitorWithDriver(struct qemud_driver *driver, virDomainObjPtr obj) { - qemuDomainObjPrivatePtr priv = obj->privateData; - int refs; - - refs = qemuMonitorUnref(priv->mon); - - if (refs > 0) - qemuMonitorUnlock(priv->mon); - - qemuDriverLock(driver); - virDomainObjLock(obj); - - priv->monStart = 0; - if (refs == 0) { - priv->mon = NULL; - } + qemuDomainObjExitMonitorInternal(driver, obj); } void qemuDomainObjEnterRemoteWithDriver(struct qemud_driver *driver, -- 1.7.6

Query commands are safe to be called during long running jobs (such as migration). This patch makes them all work without the need to special-case every single one of them. The patch introduces new job.asyncCond condition and associated job.asyncJob which are dedicated to asynchronous (from qemu monitor point of view) jobs that can take arbitrarily long time to finish while qemu monitor is still usable for other commands. The existing job.active (and job.cond condition) is used all other synchronous jobs (including the commands run during async job). Locking schema is changed to use these two conditions. While asyncJob is active, only allowed set of synchronous jobs is allowed (the set can be different according to a particular asyncJob) so any method that communicates to qemu monitor needs to check if it is allowed to be executed during current asyncJob (if any). Once the check passes, the method needs to normally acquire job.cond to ensure no other command is running. Since domain object lock is released during that time, asyncJob could have been started in the meantime so the method needs to recheck the first condition. Then, normal jobs set job.active and asynchronous jobs set job.asyncJob and optionally change the list of allowed job groups. Since asynchronous jobs only set job.asyncJob, other allowed commands can still be run when domain object is unlocked (when communicating to remote libvirtd or sleeping). To protect its own internal synchronous commands, the asynchronous job needs to start a special nested job before entering qemu monitor. The nested job doesn't check asyncJob, it only acquires job.cond and sets job.active to block other jobs. --- src/qemu/qemu_domain.c | 219 +++++++++++++++++++++++++++++++++++++-------- src/qemu/qemu_domain.h | 82 +++++++++++++---- src/qemu/qemu_driver.c | 122 +++++++++++++------------- src/qemu/qemu_hotplug.c | 38 ++++---- src/qemu/qemu_migration.c | 152 ++++++++++++++++++------------- src/qemu/qemu_process.c | 42 +++++---- 6 files changed, 439 insertions(+), 216 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a2e77b6..1ed5efd 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -87,8 +87,14 @@ qemuDomainObjInitJob(qemuDomainObjPrivatePtr priv) if (virCondInit(&priv->job.cond) < 0) return -1; + if (virCondInit(&priv->job.asyncCond) < 0) { + ignore_value(virCondDestroy(&priv->job.cond)); + return -1; + } + if (virCondInit(&priv->job.signalCond) < 0) { ignore_value(virCondDestroy(&priv->job.cond)); + ignore_value(virCondDestroy(&priv->job.asyncCond)); return -1; } @@ -101,6 +107,15 @@ qemuDomainObjResetJob(qemuDomainObjPrivatePtr priv) struct qemuDomainJobObj *job = &priv->job; job->active = QEMU_JOB_NONE; +} + +static void +qemuDomainObjResetAsyncJob(qemuDomainObjPrivatePtr priv) +{ + struct qemuDomainJobObj *job = &priv->job; + + job->asyncJob = QEMU_ASYNC_JOB_NONE; + job->mask = DEFAULT_JOB_MASK; job->start = 0; memset(&job->info, 0, sizeof(job->info)); job->signals = 0; @@ -111,6 +126,7 @@ static void qemuDomainObjFreeJob(qemuDomainObjPrivatePtr priv) { ignore_value(virCondDestroy(&priv->job.cond)); + ignore_value(virCondDestroy(&priv->job.asyncCond)); ignore_value(virCondDestroy(&priv->job.signalCond)); } @@ -509,12 +525,31 @@ qemuDomainObjSetJob(virDomainObjPtr obj, } void -qemuDomainObjDiscardJob(virDomainObjPtr obj) +qemuDomainObjSetAsyncJobMask(virDomainObjPtr obj, + unsigned long long allowedJobs) { qemuDomainObjPrivatePtr priv = obj->privateData; - qemuDomainObjResetJob(priv); - qemuDomainObjSetJob(obj, QEMU_JOB_NONE); + if (!priv->job.asyncJob) + return; + + priv->job.mask = allowedJobs | JOB_MASK(QEMU_JOB_DESTROY); +} + +void +qemuDomainObjDiscardAsyncJob(virDomainObjPtr obj) +{ + qemuDomainObjPrivatePtr priv = obj->privateData; + + if (priv->job.active == QEMU_JOB_ASYNC_NESTED) + qemuDomainObjResetJob(priv); + qemuDomainObjResetAsyncJob(priv); +} + +static bool +qemuDomainJobAllowed(qemuDomainObjPrivatePtr priv, enum qemuDomainJob job) +{ + return !priv->job.asyncJob || (priv->job.mask & JOB_MASK(job)) != 0; } /* Give up waiting for mutex after 30 seconds */ @@ -527,11 +562,14 @@ qemuDomainObjDiscardJob(virDomainObjPtr obj) static int qemuDomainObjBeginJobInternal(struct qemud_driver *driver, bool driver_locked, - virDomainObjPtr obj) + virDomainObjPtr obj, + enum qemuDomainJob job, + enum qemuDomainAsyncJob asyncJob) { qemuDomainObjPrivatePtr priv = obj->privateData; unsigned long long now; unsigned long long then; + bool nested = job == QEMU_JOB_ASYNC_NESTED; if (virTimeMs(&now) < 0) return -1; @@ -541,27 +579,31 @@ qemuDomainObjBeginJobInternal(struct qemud_driver *driver, if (driver_locked) qemuDriverUnlock(driver); +retry: + while (!nested && !qemuDomainJobAllowed(priv, job)) { + if (virCondWaitUntil(&priv->job.asyncCond, &obj->lock, then) < 0) + goto error; + } + while (priv->job.active) { - if (virCondWaitUntil(&priv->job.cond, &obj->lock, then) < 0) { - if (errno == ETIMEDOUT) - qemuReportError(VIR_ERR_OPERATION_TIMEOUT, - "%s", _("cannot acquire state change lock")); - else - virReportSystemError(errno, - "%s", _("cannot acquire job mutex")); - if (driver_locked) { - virDomainObjUnlock(obj); - qemuDriverLock(driver); - virDomainObjLock(obj); - } - /* Safe to ignore value since ref count was incremented above */ - ignore_value(virDomainObjUnref(obj)); - return -1; - } + if (virCondWaitUntil(&priv->job.cond, &obj->lock, then) < 0) + goto error; } + + /* No job is active but a new async job could have been started while obj + * was unlocked, so we need to recheck it. */ + if (!nested && !qemuDomainJobAllowed(priv, job)) + goto retry; + qemuDomainObjResetJob(priv); - qemuDomainObjSetJob(obj, QEMU_JOB_UNSPECIFIED); - priv->job.start = now; + + if (job != QEMU_JOB_ASYNC) { + priv->job.active = job; + } else { + qemuDomainObjResetAsyncJob(priv); + priv->job.asyncJob = asyncJob; + priv->job.start = now; + } if (driver_locked) { virDomainObjUnlock(obj); @@ -570,6 +612,22 @@ qemuDomainObjBeginJobInternal(struct qemud_driver *driver, } return 0; + +error: + if (errno == ETIMEDOUT) + qemuReportError(VIR_ERR_OPERATION_TIMEOUT, + "%s", _("cannot acquire state change lock")); + else + virReportSystemError(errno, + "%s", _("cannot acquire job mutex")); + if (driver_locked) { + virDomainObjUnlock(obj); + qemuDriverLock(driver); + virDomainObjLock(obj); + } + /* Safe to ignore value since ref count was incremented above */ + ignore_value(virDomainObjUnref(obj)); + return -1; } /* @@ -581,9 +639,17 @@ qemuDomainObjBeginJobInternal(struct qemud_driver *driver, * Upon successful return, the object will have its ref count increased, * successful calls must be followed by EndJob eventually */ -int qemuDomainObjBeginJob(virDomainObjPtr obj) +int qemuDomainObjBeginJob(virDomainObjPtr obj, enum qemuDomainJob job) +{ + return qemuDomainObjBeginJobInternal(NULL, false, obj, job, + QEMU_ASYNC_JOB_NONE); +} + +int qemuDomainObjBeginAsyncJob(virDomainObjPtr obj, + enum qemuDomainAsyncJob asyncJob) { - return qemuDomainObjBeginJobInternal(NULL, false, obj); + return qemuDomainObjBeginJobInternal(NULL, false, obj, QEMU_JOB_ASYNC, + asyncJob); } /* @@ -597,9 +663,49 @@ int qemuDomainObjBeginJob(virDomainObjPtr obj) * successful calls must be followed by EndJob eventually */ int qemuDomainObjBeginJobWithDriver(struct qemud_driver *driver, - virDomainObjPtr obj) + virDomainObjPtr obj, + enum qemuDomainJob job) { - return qemuDomainObjBeginJobInternal(driver, true, obj); + if (job <= QEMU_JOB_NONE || job >= QEMU_JOB_ASYNC) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Attempt to start invalid job")); + return -1; + } + + return qemuDomainObjBeginJobInternal(driver, true, obj, job, + QEMU_ASYNC_JOB_NONE); +} + +int qemuDomainObjBeginAsyncJobWithDriver(struct qemud_driver *driver, + virDomainObjPtr obj, + enum qemuDomainAsyncJob asyncJob) +{ + return qemuDomainObjBeginJobInternal(driver, true, obj, QEMU_JOB_ASYNC, + asyncJob); +} + +/* + * Use this to protect monitor sections within active async job. + * + * The caller must call qemuDomainObjBeginAsyncJob{,WithDriver} before it can + * use this method. Never use this method if you only own non-async job, use + * qemuDomainObjBeginJob{,WithDriver} instead. + */ +int +qemuDomainObjBeginNestedJob(virDomainObjPtr obj) +{ + return qemuDomainObjBeginJobInternal(NULL, false, obj, + QEMU_JOB_ASYNC_NESTED, + QEMU_ASYNC_JOB_NONE); +} + +int +qemuDomainObjBeginNestedJobWithDriver(struct qemud_driver *driver, + virDomainObjPtr obj) +{ + return qemuDomainObjBeginJobInternal(driver, true, obj, + QEMU_JOB_ASYNC_NESTED, + QEMU_ASYNC_JOB_NONE); } /* @@ -616,25 +722,60 @@ int qemuDomainObjEndJob(virDomainObjPtr obj) qemuDomainObjPrivatePtr priv = obj->privateData; qemuDomainObjResetJob(priv); - qemuDomainObjSetJob(obj, QEMU_JOB_NONE); virCondSignal(&priv->job.cond); return virDomainObjUnref(obj); } +int +qemuDomainObjEndAsyncJob(virDomainObjPtr obj) +{ + qemuDomainObjPrivatePtr priv = obj->privateData; -static void + qemuDomainObjResetAsyncJob(priv); + virCondBroadcast(&priv->job.asyncCond); + + return virDomainObjUnref(obj); +} + +void +qemuDomainObjEndNestedJob(virDomainObjPtr obj) +{ + qemuDomainObjPrivatePtr priv = obj->privateData; + + qemuDomainObjResetJob(priv); + virCondSignal(&priv->job.cond); + + /* safe to ignore since the surrounding async job increased the reference + * counter as well */ + ignore_value(virDomainObjUnref(obj)); +} + + +static int qemuDomainObjEnterMonitorInternal(struct qemud_driver *driver, virDomainObjPtr obj) { qemuDomainObjPrivatePtr priv = obj->privateData; + if (priv->job.active == QEMU_JOB_NONE && priv->job.asyncJob) { + if (qemuDomainObjBeginNestedJob(obj) < 0) + return -1; + if (!virDomainObjIsActive(obj)) { + qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("domain is no longer running")); + return -1; + } + } + qemuMonitorLock(priv->mon); qemuMonitorRef(priv->mon); ignore_value(virTimeMs(&priv->monStart)); virDomainObjUnlock(obj); if (driver) qemuDriverUnlock(driver); + + return 0; } static void @@ -657,20 +798,24 @@ qemuDomainObjExitMonitorInternal(struct qemud_driver *driver, if (refs == 0) { priv->mon = NULL; } + + if (priv->job.active == QEMU_JOB_ASYNC_NESTED) + qemuDomainObjEndNestedJob(obj); } /* * obj must be locked before calling, qemud_driver must be unlocked * * To be called immediately before any QEMU monitor API call - * Must have already called qemuDomainObjBeginJob(), and checked - * that the VM is still active. + * Must have already either called qemuDomainObjBeginJob() and checked + * that the VM is still active or called qemuDomainObjBeginAsyncJob, in which + * case this will call qemuDomainObjBeginNestedJob. * * To be followed with qemuDomainObjExitMonitor() once complete */ -void qemuDomainObjEnterMonitor(virDomainObjPtr obj) +int qemuDomainObjEnterMonitor(virDomainObjPtr obj) { - qemuDomainObjEnterMonitorInternal(NULL, obj); + return qemuDomainObjEnterMonitorInternal(NULL, obj); } /* obj must NOT be locked before calling, qemud_driver must be unlocked @@ -686,14 +831,16 @@ void qemuDomainObjExitMonitor(virDomainObjPtr obj) * obj must be locked before calling, qemud_driver must be locked * * To be called immediately before any QEMU monitor API call - * Must have already called qemuDomainObjBeginJob(). + * Must have already either called qemuDomainObjBeginJobWithDriver() and + * checked that the VM is still active or called qemuDomainObjBeginAsyncJob, + * in which case this will call qemuDomainObjBeginNestedJobWithDriver. * * To be followed with qemuDomainObjExitMonitorWithDriver() once complete */ -void qemuDomainObjEnterMonitorWithDriver(struct qemud_driver *driver, - virDomainObjPtr obj) +int qemuDomainObjEnterMonitorWithDriver(struct qemud_driver *driver, + virDomainObjPtr obj) { - qemuDomainObjEnterMonitorInternal(driver, obj); + return qemuDomainObjEnterMonitorInternal(driver, obj); } /* obj must NOT be locked before calling, qemud_driver must be unlocked, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 214e578..85a3c03 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -30,16 +30,35 @@ # include "qemu_conf.h" # include "bitmap.h" +#define JOB_MASK(job) (1 << (job - 1)) +#define DEFAULT_JOB_MASK \ + (JOB_MASK(QEMU_JOB_QUERY) | JOB_MASK(QEMU_JOB_DESTROY)) + /* Only 1 job is allowed at any time * A job includes *all* monitor commands, even those just querying * information, not merely actions */ enum qemuDomainJob { QEMU_JOB_NONE = 0, /* Always set to 0 for easy if (jobActive) conditions */ - QEMU_JOB_UNSPECIFIED, - QEMU_JOB_MIGRATION_OUT, - QEMU_JOB_MIGRATION_IN, - QEMU_JOB_SAVE, - QEMU_JOB_DUMP, + QEMU_JOB_QUERY, /* Doesn't change any state */ + QEMU_JOB_DESTROY, /* Destroys the domain (cannot be masked out) */ + QEMU_JOB_SUSPEND, /* Suspends (stops vCPUs) the domain */ + QEMU_JOB_MODIFY, /* May change state */ + + /* The following two items must always be the last items */ + QEMU_JOB_ASYNC, /* Asynchronous job */ + QEMU_JOB_ASYNC_NESTED, /* Normal job within an async job */ +}; + +/* Async job consists of a series of jobs that may change state. Independent + * jobs that do not change state (and possibly others if explicitly allowed by + * current async job) are allowed to be run even if async job is active. + */ +enum qemuDomainAsyncJob { + QEMU_ASYNC_JOB_NONE = 0, + QEMU_ASYNC_JOB_MIGRATION_OUT, + QEMU_ASYNC_JOB_MIGRATION_IN, + QEMU_ASYNC_JOB_SAVE, + QEMU_ASYNC_JOB_DUMP, }; enum qemuDomainJobSignals { @@ -63,14 +82,16 @@ struct qemuDomainJobSignalsData { }; struct qemuDomainJobObj { - virCond cond; /* Use in conjunction with main virDomainObjPtr lock */ - virCond signalCond; /* Use to coordinate the safe queries during migration */ - - enum qemuDomainJob active; /* Currently running job */ + virCond cond; /* Use to coordinate jobs */ + enum qemuDomainJob active; /* Currently running job */ - unsigned long long start; /* When the job started */ - virDomainJobInfo info; /* Progress data */ + virCond asyncCond; /* Use to coordinate with async jobs */ + enum qemuDomainAsyncJob asyncJob; /* Currently active async job */ + unsigned long long mask; /* Jobs allowed during async job */ + unsigned long long start; /* When the async job started */ + virDomainJobInfo info; /* Async job progress data */ + virCond signalCond; /* Use to coordinate the safe queries during migration */ unsigned int signals; /* Signals for running job */ struct qemuDomainJobSignalsData signalsData; /* Signal specific data */ }; @@ -117,18 +138,43 @@ void qemuDomainEventQueue(struct qemud_driver *driver, void qemuDomainSetPrivateDataHooks(virCapsPtr caps); void qemuDomainSetNamespaceHooks(virCapsPtr caps); -int qemuDomainObjBeginJob(virDomainObjPtr obj) ATTRIBUTE_RETURN_CHECK; +int qemuDomainObjBeginJob(virDomainObjPtr obj, + enum qemuDomainJob job) + ATTRIBUTE_RETURN_CHECK; +int qemuDomainObjBeginAsyncJob(virDomainObjPtr obj, + enum qemuDomainAsyncJob asyncJob) + ATTRIBUTE_RETURN_CHECK; +int qemuDomainObjBeginNestedJob(virDomainObjPtr obj) + ATTRIBUTE_RETURN_CHECK; int qemuDomainObjBeginJobWithDriver(struct qemud_driver *driver, - virDomainObjPtr obj) ATTRIBUTE_RETURN_CHECK; -int qemuDomainObjEndJob(virDomainObjPtr obj) ATTRIBUTE_RETURN_CHECK; + virDomainObjPtr obj, + enum qemuDomainJob job) + ATTRIBUTE_RETURN_CHECK; +int qemuDomainObjBeginAsyncJobWithDriver(struct qemud_driver *driver, + virDomainObjPtr obj, + enum qemuDomainAsyncJob asyncJob) + ATTRIBUTE_RETURN_CHECK; +int qemuDomainObjBeginNestedJobWithDriver(struct qemud_driver *driver, + virDomainObjPtr obj) + ATTRIBUTE_RETURN_CHECK; + +int qemuDomainObjEndJob(virDomainObjPtr obj) + ATTRIBUTE_RETURN_CHECK; +int qemuDomainObjEndAsyncJob(virDomainObjPtr obj) + ATTRIBUTE_RETURN_CHECK; +void qemuDomainObjEndNestedJob(virDomainObjPtr obj); void qemuDomainObjSetJob(virDomainObjPtr obj, enum qemuDomainJob job); -void qemuDomainObjDiscardJob(virDomainObjPtr obj); +void qemuDomainObjSetAsyncJobMask(virDomainObjPtr obj, + unsigned long long allowedJobs); +void qemuDomainObjDiscardAsyncJob(virDomainObjPtr obj); -void qemuDomainObjEnterMonitor(virDomainObjPtr obj); +int qemuDomainObjEnterMonitor(virDomainObjPtr obj) + ATTRIBUTE_RETURN_CHECK; void qemuDomainObjExitMonitor(virDomainObjPtr obj); -void qemuDomainObjEnterMonitorWithDriver(struct qemud_driver *driver, - virDomainObjPtr obj); +int qemuDomainObjEnterMonitorWithDriver(struct qemud_driver *driver, + virDomainObjPtr obj) + ATTRIBUTE_RETURN_CHECK; void qemuDomainObjExitMonitorWithDriver(struct qemud_driver *driver, virDomainObjPtr obj); void qemuDomainObjEnterRemoteWithDriver(struct qemud_driver *driver, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e9cdcbe..96b3737 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -141,7 +141,8 @@ qemuAutostartDomain(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaq virDomainObjLock(vm); virResetLastError(); - if (qemuDomainObjBeginJobWithDriver(data->driver, vm) < 0) { + if (qemuDomainObjBeginJobWithDriver(data->driver, vm, + QEMU_JOB_MODIFY) < 0) { err = virGetLastError(); VIR_ERROR(_("Failed to start job on VM '%s': %s"), vm->def->name, @@ -1274,7 +1275,7 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, def = NULL; - if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) + if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; /* XXXX free the 'vm' we created ? */ if (qemuProcessStart(conn, driver, vm, NULL, @@ -1343,7 +1344,7 @@ static int qemudDomainSuspend(virDomainPtr dom) { priv = vm->privateData; - if (priv->job.active == QEMU_JOB_MIGRATION_OUT) { + if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT) { if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_PAUSED) { VIR_DEBUG("Requesting domain pause on %s", vm->def->name); @@ -1352,7 +1353,7 @@ static int qemudDomainSuspend(virDomainPtr dom) { ret = 0; goto cleanup; } else { - if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) + if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_SUSPEND) < 0) goto cleanup; if (!virDomainObjIsActive(vm)) { @@ -1405,7 +1406,7 @@ static int qemudDomainResume(virDomainPtr dom) { goto cleanup; } - if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) + if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; if (!virDomainObjIsActive(vm)) { @@ -1461,7 +1462,7 @@ static int qemuDomainShutdown(virDomainPtr dom) { goto cleanup; } - if (qemuDomainObjBeginJob(vm) < 0) + if (qemuDomainObjBeginJob(vm, QEMU_JOB_MODIFY) < 0) goto cleanup; if (!virDomainObjIsActive(vm)) { @@ -1471,7 +1472,7 @@ static int qemuDomainShutdown(virDomainPtr dom) { } priv = vm->privateData; - qemuDomainObjEnterMonitor(vm); + ignore_value(qemuDomainObjEnterMonitor(vm)); ret = qemuMonitorSystemPowerdown(priv->mon); qemuDomainObjExitMonitor(vm); @@ -1511,7 +1512,7 @@ static int qemuDomainReboot(virDomainPtr dom, unsigned int flags) { #if HAVE_YAJL if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_MONITOR_JSON)) { - if (qemuDomainObjBeginJob(vm) < 0) + if (qemuDomainObjBeginJob(vm, QEMU_JOB_MODIFY) < 0) goto cleanup; if (!virDomainObjIsActive(vm)) { @@ -1520,7 +1521,7 @@ static int qemuDomainReboot(virDomainPtr dom, unsigned int flags) { goto endjob; } - qemuDomainObjEnterMonitor(vm); + ignore_value(qemuDomainObjEnterMonitor(vm)); ret = qemuMonitorSystemPowerdown(priv->mon); qemuDomainObjExitMonitor(vm); @@ -1571,7 +1572,7 @@ static int qemudDomainDestroy(virDomainPtr dom) { */ qemuProcessKill(vm); - if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) + if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_DESTROY) < 0) goto cleanup; if (!virDomainObjIsActive(vm)) { @@ -1684,7 +1685,7 @@ static int qemudDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, goto cleanup; } - if (qemuDomainObjBeginJob(vm) < 0) + if (qemuDomainObjBeginJob(vm, QEMU_JOB_MODIFY) < 0) goto cleanup; isActive = virDomainObjIsActive(vm); @@ -1749,7 +1750,7 @@ static int qemudDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, if (flags & VIR_DOMAIN_AFFECT_LIVE) { priv = vm->privateData; - qemuDomainObjEnterMonitor(vm); + ignore_value(qemuDomainObjEnterMonitor(vm)); r = qemuMonitorSetBalloon(priv->mon, newmem); qemuDomainObjExitMonitor(vm); qemuAuditMemory(vm, vm->def->mem.cur_balloon, newmem, "update", @@ -1821,9 +1822,9 @@ static int qemuDomainInjectNMI(virDomainPtr domain, unsigned int flags) priv = vm->privateData; - if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) + if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); ret = qemuMonitorInjectNMI(priv->mon); qemuDomainObjExitMonitorWithDriver(driver, vm); if (qemuDomainObjEndJob(vm) == 0) { @@ -1879,12 +1880,12 @@ static int qemudDomainGetInfo(virDomainPtr dom, (vm->def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_NONE)) { info->memory = vm->def->mem.max_balloon; } else if (!priv->job.active) { - if (qemuDomainObjBeginJob(vm) < 0) + if (qemuDomainObjBeginJob(vm, QEMU_JOB_QUERY) < 0) goto cleanup; if (!virDomainObjIsActive(vm)) err = 0; else { - qemuDomainObjEnterMonitor(vm); + ignore_value(qemuDomainObjEnterMonitor(vm)); err = qemuMonitorGetBalloonInfo(priv->mon, &balloon); qemuDomainObjExitMonitor(vm); } @@ -2122,11 +2123,10 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, priv = vm->privateData; - if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) + if (qemuDomainObjBeginAsyncJobWithDriver(driver, vm, + QEMU_ASYNC_JOB_SAVE) < 0) goto cleanup; - qemuDomainObjSetJob(vm, QEMU_JOB_SAVE); - memset(&priv->job.info, 0, sizeof(priv->job.info)); priv->job.info.type = VIR_DOMAIN_JOB_UNBOUNDED; @@ -2294,7 +2294,7 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_SAVED); if (!vm->persistent) { - if (qemuDomainObjEndJob(vm) > 0) + if (qemuDomainObjEndAsyncJob(vm) > 0) virDomainRemoveInactive(&driver->domains, vm); vm = NULL; @@ -2310,7 +2310,7 @@ endjob: VIR_WARN("Unable to resume guest CPUs after save failure"); } } - if (qemuDomainObjEndJob(vm) == 0) + if (qemuDomainObjEndAsyncJob(vm) == 0) vm = NULL; } @@ -2608,7 +2608,8 @@ static int qemudDomainCoreDump(virDomainPtr dom, } priv = vm->privateData; - if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) + if (qemuDomainObjBeginAsyncJobWithDriver(driver, vm, + QEMU_ASYNC_JOB_DUMP) < 0) goto cleanup; if (!virDomainObjIsActive(vm)) { @@ -2617,8 +2618,6 @@ static int qemudDomainCoreDump(virDomainPtr dom, goto endjob; } - qemuDomainObjSetJob(vm, QEMU_JOB_DUMP); - /* Migrate will always stop the VM, so the resume condition is independent of whether the stop command is issued. */ resume = virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING; @@ -2664,7 +2663,7 @@ endjob: } } - if (qemuDomainObjEndJob(vm) == 0) + if (qemuDomainObjEndAsyncJob(vm) == 0) vm = NULL; else if ((ret == 0) && (flags & VIR_DUMP_CRASH) && !vm->persistent) { virDomainRemoveInactive(&driver->domains, @@ -2708,7 +2707,7 @@ qemuDomainScreenshot(virDomainPtr dom, priv = vm->privateData; - if (qemuDomainObjBeginJob(vm) < 0) + if (qemuDomainObjBeginJob(vm, QEMU_JOB_QUERY) < 0) goto cleanup; if (!virDomainObjIsActive(vm)) { @@ -2738,7 +2737,7 @@ qemuDomainScreenshot(virDomainPtr dom, virSecurityManagerSetSavedStateLabel(qemu_driver->securityManager, vm, tmp); - qemuDomainObjEnterMonitor(vm); + ignore_value(qemuDomainObjEnterMonitor(vm)); if (qemuMonitorScreendump(priv->mon, tmp) < 0) { qemuDomainObjExitMonitor(vm); goto endjob; @@ -2793,7 +2792,8 @@ static void processWatchdogEvent(void *data, void *opaque) goto unlock; } - if (qemuDomainObjBeginJobWithDriver(driver, wdEvent->vm) < 0) { + if (qemuDomainObjBeginAsyncJobWithDriver(driver, wdEvent->vm, + QEMU_ASYNC_JOB_DUMP) < 0) { VIR_FREE(dumpfile); goto unlock; } @@ -2831,7 +2831,7 @@ endjob: /* Safe to ignore value since ref count was incremented in * qemuProcessHandleWatchdog(). */ - ignore_value(qemuDomainObjEndJob(wdEvent->vm)); + ignore_value(qemuDomainObjEndAsyncJob(wdEvent->vm)); unlock: if (virDomainObjUnref(wdEvent->vm) > 0) @@ -2848,7 +2848,7 @@ static int qemudDomainHotplugVcpus(virDomainObjPtr vm, unsigned int nvcpus) int oldvcpus = vm->def->vcpus; int vcpus = oldvcpus; - qemuDomainObjEnterMonitor(vm); + ignore_value(qemuDomainObjEnterMonitor(vm)); /* We need different branches here, because we want to offline * in reverse order to onlining, so any partial fail leaves us in a @@ -2934,7 +2934,7 @@ qemudDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, goto cleanup; } - if (qemuDomainObjBeginJob(vm) < 0) + if (qemuDomainObjBeginJob(vm, QEMU_JOB_MODIFY) < 0) goto cleanup; if (!virDomainObjIsActive(vm) && (flags & VIR_DOMAIN_AFFECT_LIVE)) { @@ -3755,7 +3755,7 @@ qemuDomainRestore(virConnectPtr conn, } def = NULL; - if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) + if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; ret = qemuDomainSaveImageStartVM(conn, driver, vm, &fd, &header, path); @@ -3848,10 +3848,10 @@ static char *qemuDomainGetXMLDesc(virDomainPtr dom, /* Don't delay if someone's using the monitor, just use * existing most recent data instead */ if (!priv->job.active) { - if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) + if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_QUERY) < 0) goto cleanup; - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); err = qemuMonitorGetBalloonInfo(priv->mon, &balloon); qemuDomainObjExitMonitorWithDriver(driver, vm); if (qemuDomainObjEndJob(vm) == 0) { @@ -4078,7 +4078,7 @@ qemudDomainStartWithFlags(virDomainPtr dom, unsigned int flags) goto cleanup; } - if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) + if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; if (virDomainObjIsActive(vm)) { @@ -4823,7 +4823,7 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, goto cleanup; } - if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) + if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; if (virDomainObjIsActive(vm)) { @@ -6006,8 +6006,8 @@ qemudDomainBlockStats (virDomainPtr dom, } priv = vm->privateData; - if ((priv->job.active == QEMU_JOB_MIGRATION_OUT) - || (priv->job.active == QEMU_JOB_SAVE)) { + if ((priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT) + || (priv->job.asyncJob == QEMU_ASYNC_JOB_SAVE)) { virDomainObjRef(vm); while (priv->job.signals & QEMU_JOB_SIGNAL_BLKSTAT) ignore_value(virCondWait(&priv->job.signalCond, &vm->lock)); @@ -6023,7 +6023,7 @@ qemudDomainBlockStats (virDomainPtr dom, if (virDomainObjUnref(vm) == 0) vm = NULL; } else { - if (qemuDomainObjBeginJob(vm) < 0) + if (qemuDomainObjBeginJob(vm, QEMU_JOB_QUERY) < 0) goto cleanup; if (!virDomainObjIsActive(vm)) { @@ -6032,7 +6032,7 @@ qemudDomainBlockStats (virDomainPtr dom, goto endjob; } - qemuDomainObjEnterMonitor(vm); + ignore_value(qemuDomainObjEnterMonitor(vm)); ret = qemuMonitorGetBlockStatsInfo(priv->mon, disk->info.alias, &stats->rd_req, @@ -6135,12 +6135,12 @@ qemudDomainMemoryStats (virDomainPtr dom, goto cleanup; } - if (qemuDomainObjBeginJob(vm) < 0) + if (qemuDomainObjBeginJob(vm, QEMU_JOB_QUERY) < 0) goto cleanup; if (virDomainObjIsActive(vm)) { qemuDomainObjPrivatePtr priv = vm->privateData; - qemuDomainObjEnterMonitor(vm); + ignore_value(qemuDomainObjEnterMonitor(vm)); ret = qemuMonitorGetMemoryStats(priv->mon, stats, nr_stats); qemuDomainObjExitMonitor(vm); } else { @@ -6259,7 +6259,7 @@ qemudDomainMemoryPeek (virDomainPtr dom, goto cleanup; } - if (qemuDomainObjBeginJob(vm) < 0) + if (qemuDomainObjBeginJob(vm, QEMU_JOB_QUERY) < 0) goto cleanup; if (!virDomainObjIsActive(vm)) { @@ -6283,7 +6283,7 @@ qemudDomainMemoryPeek (virDomainPtr dom, virSecurityManagerSetSavedStateLabel(qemu_driver->securityManager, vm, tmp); priv = vm->privateData; - qemuDomainObjEnterMonitor(vm); + ignore_value(qemuDomainObjEnterMonitor(vm)); if (flags == VIR_MEMORY_VIRTUAL) { if (qemuMonitorSaveVirtualMemory(priv->mon, offset, size, tmp) < 0) { qemuDomainObjExitMonitor(vm); @@ -6453,8 +6453,8 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom, virDomainObjIsActive(vm)) { qemuDomainObjPrivatePtr priv = vm->privateData; - if ((priv->job.active == QEMU_JOB_MIGRATION_OUT) - || (priv->job.active == QEMU_JOB_SAVE)) { + if ((priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT) + || (priv->job.asyncJob == QEMU_ASYNC_JOB_SAVE)) { virDomainObjRef(vm); while (priv->job.signals & QEMU_JOB_SIGNAL_BLKINFO) ignore_value(virCondWait(&priv->job.signalCond, &vm->lock)); @@ -6470,11 +6470,11 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom, if (virDomainObjUnref(vm) == 0) vm = NULL; } else { - if (qemuDomainObjBeginJob(vm) < 0) + if (qemuDomainObjBeginJob(vm, QEMU_JOB_QUERY) < 0) goto cleanup; if (virDomainObjIsActive(vm)) { - qemuDomainObjEnterMonitor(vm); + ignore_value(qemuDomainObjEnterMonitor(vm)); ret = qemuMonitorGetBlockExtent(priv->mon, disk->info.alias, &info->allocation); @@ -7083,7 +7083,7 @@ qemuDomainMigrateConfirm3(virDomainPtr domain, goto cleanup; } - if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) + if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; ret = qemuMigrationConfirm(driver, domain->conn, vm, @@ -7293,7 +7293,7 @@ static int qemuDomainGetJobInfo(virDomainPtr dom, priv = vm->privateData; if (virDomainObjIsActive(vm)) { - if (priv->job.active) { + if (priv->job.asyncJob) { memcpy(info, &priv->job.info, sizeof(*info)); /* Refresh elapsed time again just to ensure it @@ -7343,7 +7343,7 @@ static int qemuDomainAbortJob(virDomainPtr dom) { priv = vm->privateData; if (virDomainObjIsActive(vm)) { - if (priv->job.active) { + if (priv->job.asyncJob) { VIR_DEBUG("Requesting cancellation of job on vm %s", vm->def->name); priv->job.signals |= QEMU_JOB_SIGNAL_CANCEL; } else { @@ -7397,7 +7397,7 @@ qemuDomainMigrateSetMaxDowntime(virDomainPtr dom, priv = vm->privateData; - if (priv->job.active != QEMU_JOB_MIGRATION_OUT) { + if (priv->job.asyncJob != QEMU_ASYNC_JOB_MIGRATION_OUT) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not being migrated")); goto cleanup; @@ -7446,7 +7446,7 @@ qemuDomainMigrateSetMaxSpeed(virDomainPtr dom, priv = vm->privateData; - if (priv->job.active != QEMU_JOB_MIGRATION_OUT) { + if (priv->job.asyncJob != QEMU_ASYNC_JOB_MIGRATION_OUT) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not being migrated")); goto cleanup; @@ -7639,7 +7639,7 @@ qemuDomainSnapshotCreateActive(virConnectPtr conn, bool resume = false; int ret = -1; - if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) + if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) return -1; if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { @@ -7658,7 +7658,7 @@ qemuDomainSnapshotCreateActive(virConnectPtr conn, } } - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); ret = qemuMonitorCreateSnapshot(priv->mon, snap->def->name); qemuDomainObjExitMonitorWithDriver(driver, vm); @@ -7984,7 +7984,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, vm->current_snapshot = snap; - if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) + if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; if (snap->def->state == VIR_DOMAIN_RUNNING @@ -7992,7 +7992,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, if (virDomainObjIsActive(vm)) { priv = vm->privateData; - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); rc = qemuMonitorLoadSnapshot(priv->mon, snap->def->name); qemuDomainObjExitMonitorWithDriver(driver, vm); if (rc < 0) @@ -8116,7 +8116,7 @@ static int qemuDomainSnapshotDiscard(struct qemud_driver *driver, } else { priv = vm->privateData; - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); /* we continue on even in the face of error */ qemuMonitorDeleteSnapshot(priv->mon, snap->def->name); qemuDomainObjExitMonitorWithDriver(driver, vm); @@ -8255,7 +8255,7 @@ static int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, goto cleanup; } - if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) + if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN) { @@ -8324,9 +8324,9 @@ static int qemuDomainMonitorCommand(virDomainPtr domain, const char *cmd, hmp = !!(flags & VIR_DOMAIN_QEMU_MONITOR_COMMAND_HMP); - if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) + if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); ret = qemuMonitorArbitraryCommand(priv->mon, cmd, result, hmp); qemuDomainObjExitMonitorWithDriver(driver, vm); if (qemuDomainObjEndJob(vm) == 0) { diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index a7f11ab..a7571cd 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -96,7 +96,7 @@ int qemuDomainChangeEjectableMedia(struct qemud_driver *driver, if (!(driveAlias = qemuDeviceDriveHostAlias(origdisk, priv->qemuCaps))) goto error; - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); if (disk->src) { const char *format = NULL; if (disk->type != VIR_DOMAIN_DISK_TYPE_DIR) { @@ -198,7 +198,7 @@ int qemuDomainAttachPciDiskDevice(struct qemud_driver *driver, goto error; } - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { ret = qemuMonitorAddDrive(priv->mon, drivestr); if (ret == 0) { @@ -295,7 +295,7 @@ int qemuDomainAttachPciControllerDevice(struct qemud_driver *driver, goto cleanup; } - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { ret = qemuMonitorAddDevice(priv->mon, devstr); } else { @@ -440,7 +440,7 @@ int qemuDomainAttachSCSIDisk(struct qemud_driver *driver, goto error; } - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { ret = qemuMonitorAddDrive(priv->mon, drivestr); if (ret == 0) { @@ -542,7 +542,7 @@ int qemuDomainAttachUsbMassstorageDevice(struct qemud_driver *driver, goto error; } - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { ret = qemuMonitorAddDrive(priv->mon, drivestr); if (ret == 0) { @@ -675,7 +675,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, goto cleanup; } - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV) && qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { if (qemuMonitorAddNetdev(priv->mon, netstr, tapfd, tapfd_name, @@ -711,7 +711,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, goto try_remove; } - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { if (qemuMonitorAddDevice(priv->mon, nicstr) < 0) { qemuDomainObjExitMonitorWithDriver(driver, vm); @@ -767,7 +767,7 @@ try_remove: char *netdev_name; if (virAsprintf(&netdev_name, "host%s", net->info.alias) < 0) goto no_memory; - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); if (qemuMonitorRemoveNetdev(priv->mon, netdev_name) < 0) VIR_WARN("Failed to remove network backend for netdev %s", netdev_name); @@ -780,7 +780,7 @@ try_remove: char *hostnet_name; if (virAsprintf(&hostnet_name, "host%s", net->info.alias) < 0) goto no_memory; - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); if (qemuMonitorRemoveHostNetwork(priv->mon, vlan, hostnet_name) < 0) VIR_WARN("Failed to remove network backend for vlan %d, net %s", vlan, hostnet_name); @@ -841,14 +841,14 @@ int qemuDomainAttachHostPciDevice(struct qemud_driver *driver, priv->qemuCaps))) goto error; - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); ret = qemuMonitorAddDeviceWithFd(priv->mon, devstr, configfd, configfd_name); qemuDomainObjExitMonitorWithDriver(driver, vm); } else { virDomainDevicePCIAddress guestAddr; - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); ret = qemuMonitorAddPCIHostDevice(priv->mon, &hostdev->source.subsys.u.pci, &guestAddr); @@ -929,7 +929,7 @@ int qemuDomainAttachHostUsbDevice(struct qemud_driver *driver, goto error; } - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) ret = qemuMonitorAddDevice(priv->mon, devstr); else @@ -1237,7 +1237,7 @@ int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver, goto cleanup; } - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { qemuDomainObjExitMonitor(vm); @@ -1333,7 +1333,7 @@ int qemuDomainDetachDiskDevice(struct qemud_driver *driver, goto cleanup; } - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { qemuDomainObjExitMonitor(vm); qemuAuditDisk(vm, detach, NULL, "detach", false); @@ -1471,7 +1471,7 @@ int qemuDomainDetachPciControllerDevice(struct qemud_driver *driver, goto cleanup; } - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { if (qemuMonitorDelDevice(priv->mon, detach->info.alias)) { qemuDomainObjExitMonitor(vm); @@ -1566,7 +1566,7 @@ int qemuDomainDetachNetDevice(struct qemud_driver *driver, goto cleanup; } - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { qemuDomainObjExitMonitor(vm); @@ -1701,7 +1701,7 @@ int qemuDomainDetachHostPciDevice(struct qemud_driver *driver, return -1; } - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { ret = qemuMonitorDelDevice(priv->mon, detach->info.alias); } else { @@ -1804,7 +1804,7 @@ int qemuDomainDetachHostUsbDevice(struct qemud_driver *driver, return -1; } - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); ret = qemuMonitorDelDevice(priv->mon, detach->info.alias); qemuDomainObjExitMonitorWithDriver(driver, vm); qemuAuditHostdev(vm, detach, "detach", ret == 0); @@ -1879,7 +1879,7 @@ qemuDomainChangeGraphicsPasswords(struct qemud_driver *driver, if (!auth->passwd && !driver->vncPassword) return 0; - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); ret = qemuMonitorSetPassword(priv->mon, type, auth->passwd ? auth->passwd : defaultPasswd, diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 3634966..0712d73 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -749,9 +749,11 @@ qemuMigrationProcessJobSignals(struct qemud_driver *driver, if (priv->job.signals & QEMU_JOB_SIGNAL_CANCEL) { priv->job.signals ^= QEMU_JOB_SIGNAL_CANCEL; VIR_DEBUG("Cancelling job at client request"); - qemuDomainObjEnterMonitorWithDriver(driver, vm); - ret = qemuMonitorMigrateCancel(priv->mon); - qemuDomainObjExitMonitorWithDriver(driver, vm); + ret = qemuDomainObjEnterMonitorWithDriver(driver, vm); + if (ret == 0) { + ret = qemuMonitorMigrateCancel(priv->mon); + qemuDomainObjExitMonitorWithDriver(driver, vm); + } if (ret < 0) { VIR_WARN("Unable to cancel job"); } @@ -766,9 +768,11 @@ qemuMigrationProcessJobSignals(struct qemud_driver *driver, priv->job.signals ^= QEMU_JOB_SIGNAL_MIGRATE_DOWNTIME; priv->job.signalsData.migrateDowntime = 0; VIR_DEBUG("Setting migration downtime to %llums", ms); - qemuDomainObjEnterMonitorWithDriver(driver, vm); - ret = qemuMonitorSetMigrationDowntime(priv->mon, ms); - qemuDomainObjExitMonitorWithDriver(driver, vm); + ret = qemuDomainObjEnterMonitorWithDriver(driver, vm); + if (ret == 0) { + ret = qemuMonitorSetMigrationDowntime(priv->mon, ms); + qemuDomainObjExitMonitorWithDriver(driver, vm); + } if (ret < 0) VIR_WARN("Unable to set migration downtime"); } else if (priv->job.signals & QEMU_JOB_SIGNAL_MIGRATE_SPEED) { @@ -777,21 +781,25 @@ qemuMigrationProcessJobSignals(struct qemud_driver *driver, priv->job.signals ^= QEMU_JOB_SIGNAL_MIGRATE_SPEED; priv->job.signalsData.migrateBandwidth = 0; VIR_DEBUG("Setting migration bandwidth to %luMbs", bandwidth); - qemuDomainObjEnterMonitorWithDriver(driver, vm); - ret = qemuMonitorSetMigrationSpeed(priv->mon, bandwidth); - qemuDomainObjExitMonitorWithDriver(driver, vm); + ret = qemuDomainObjEnterMonitorWithDriver(driver, vm); + if (ret == 0) { + ret = qemuMonitorSetMigrationSpeed(priv->mon, bandwidth); + qemuDomainObjExitMonitorWithDriver(driver, vm); + } if (ret < 0) VIR_WARN("Unable to set migration speed"); } else if (priv->job.signals & QEMU_JOB_SIGNAL_BLKSTAT) { - qemuDomainObjEnterMonitorWithDriver(driver, vm); - ret = qemuMonitorGetBlockStatsInfo(priv->mon, - priv->job.signalsData.statDevName, - &priv->job.signalsData.blockStat->rd_req, - &priv->job.signalsData.blockStat->rd_bytes, - &priv->job.signalsData.blockStat->wr_req, - &priv->job.signalsData.blockStat->wr_bytes, - &priv->job.signalsData.blockStat->errs); - qemuDomainObjExitMonitorWithDriver(driver, vm); + ret = qemuDomainObjEnterMonitorWithDriver(driver, vm); + if (ret == 0) { + ret = qemuMonitorGetBlockStatsInfo(priv->mon, + priv->job.signalsData.statDevName, + &priv->job.signalsData.blockStat->rd_req, + &priv->job.signalsData.blockStat->rd_bytes, + &priv->job.signalsData.blockStat->wr_req, + &priv->job.signalsData.blockStat->wr_bytes, + &priv->job.signalsData.blockStat->errs); + qemuDomainObjExitMonitorWithDriver(driver, vm); + } *priv->job.signalsData.statRetCode = ret; priv->job.signals ^= QEMU_JOB_SIGNAL_BLKSTAT; @@ -799,11 +807,13 @@ qemuMigrationProcessJobSignals(struct qemud_driver *driver, if (ret < 0) VIR_WARN("Unable to get block statistics"); } else if (priv->job.signals & QEMU_JOB_SIGNAL_BLKINFO) { - qemuDomainObjEnterMonitorWithDriver(driver, vm); - ret = qemuMonitorGetBlockExtent(priv->mon, - priv->job.signalsData.infoDevName, - &priv->job.signalsData.blockInfo->allocation); - qemuDomainObjExitMonitorWithDriver(driver, vm); + ret = qemuDomainObjEnterMonitorWithDriver(driver, vm); + if (ret == 0) { + ret = qemuMonitorGetBlockExtent(priv->mon, + priv->job.signalsData.infoDevName, + &priv->job.signalsData.blockInfo->allocation); + qemuDomainObjExitMonitorWithDriver(driver, vm); + } *priv->job.signalsData.infoRetCode = ret; priv->job.signals ^= QEMU_JOB_SIGNAL_BLKINFO; @@ -836,13 +846,15 @@ qemuMigrationUpdateJobStatus(struct qemud_driver *driver, return -1; } - qemuDomainObjEnterMonitorWithDriver(driver, vm); - ret = qemuMonitorGetMigrationStatus(priv->mon, - &status, - &memProcessed, - &memRemaining, - &memTotal); - qemuDomainObjExitMonitorWithDriver(driver, vm); + ret = qemuDomainObjEnterMonitorWithDriver(driver, vm); + if (ret == 0) { + ret = qemuMonitorGetMigrationStatus(priv->mon, + &status, + &memProcessed, + &memRemaining, + &memTotal); + qemuDomainObjExitMonitorWithDriver(driver, vm); + } if (ret < 0 || virTimeMs(&priv->job.info.timeElapsed) < 0) { priv->job.info.type = VIR_DOMAIN_JOB_FAILED; @@ -897,14 +909,14 @@ qemuMigrationWaitForCompletion(struct qemud_driver *driver, virDomainObjPtr vm) qemuDomainObjPrivatePtr priv = vm->privateData; const char *job; - switch (priv->job.active) { - case QEMU_JOB_MIGRATION_OUT: + switch (priv->job.asyncJob) { + case QEMU_ASYNC_JOB_MIGRATION_OUT: job = _("migration job"); break; - case QEMU_JOB_SAVE: + case QEMU_ASYNC_JOB_SAVE: job = _("domain save job"); break; - case QEMU_JOB_DUMP: + case QEMU_ASYNC_JOB_DUMP: job = _("domain core dump job"); break; default: @@ -969,14 +981,16 @@ qemuDomainMigrateGraphicsRelocate(struct qemud_driver *driver, if (cookie->graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE) return 0; - qemuDomainObjEnterMonitorWithDriver(driver, vm); - ret = qemuMonitorGraphicsRelocate(priv->mon, - cookie->graphics->type, - cookie->remoteHostname, - cookie->graphics->port, - cookie->graphics->tlsPort, - cookie->graphics->tlsSubject); - qemuDomainObjExitMonitorWithDriver(driver, vm); + ret = qemuDomainObjEnterMonitorWithDriver(driver, vm); + if (ret == 0) { + ret = qemuMonitorGraphicsRelocate(priv->mon, + cookie->graphics->type, + cookie->remoteHostname, + cookie->graphics->port, + cookie->graphics->tlsPort, + cookie->graphics->tlsSubject); + qemuDomainObjExitMonitorWithDriver(driver, vm); + } return ret; } @@ -1108,9 +1122,9 @@ qemuMigrationPrepareTunnel(struct qemud_driver *driver, QEMU_MIGRATION_COOKIE_LOCKSTATE))) goto cleanup; - if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) + if (qemuDomainObjBeginAsyncJobWithDriver(driver, vm, + QEMU_ASYNC_JOB_MIGRATION_IN) < 0) goto cleanup; - qemuDomainObjSetJob(vm, QEMU_JOB_MIGRATION_IN); /* Domain starts inactive, even if the domain XML had an id field. */ vm->def->id = -1; @@ -1144,7 +1158,7 @@ qemuMigrationPrepareTunnel(struct qemud_driver *driver, qemuAuditDomainStart(vm, "migrated", false); qemuProcessStop(driver, vm, 0, VIR_DOMAIN_SHUTOFF_FAILED); if (!vm->persistent) { - if (qemuDomainObjEndJob(vm) > 0) + if (qemuDomainObjEndAsyncJob(vm) > 0) virDomainRemoveInactive(&driver->domains, vm); vm = NULL; } @@ -1173,7 +1187,7 @@ qemuMigrationPrepareTunnel(struct qemud_driver *driver, endjob: if (vm && - qemuDomainObjEndJob(vm) == 0) + qemuDomainObjEndAsyncJob(vm) == 0) vm = NULL; /* We set a fake job active which is held across @@ -1183,7 +1197,7 @@ endjob: */ if (vm && virDomainObjIsActive(vm)) { - qemuDomainObjSetJob(vm, QEMU_JOB_MIGRATION_IN); + priv->job.asyncJob = QEMU_ASYNC_JOB_MIGRATION_IN; priv->job.info.type = VIR_DOMAIN_JOB_UNBOUNDED; priv->job.start = now; } @@ -1343,9 +1357,9 @@ qemuMigrationPrepareDirect(struct qemud_driver *driver, QEMU_MIGRATION_COOKIE_LOCKSTATE))) goto cleanup; - if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) + if (qemuDomainObjBeginAsyncJobWithDriver(driver, vm, + QEMU_ASYNC_JOB_MIGRATION_IN) < 0) goto cleanup; - qemuDomainObjSetJob(vm, QEMU_JOB_MIGRATION_IN); /* Domain starts inactive, even if the domain XML had an id field. */ vm->def->id = -1; @@ -1361,7 +1375,7 @@ qemuMigrationPrepareDirect(struct qemud_driver *driver, * should have already done that. */ if (!vm->persistent) { - if (qemuDomainObjEndJob(vm) > 0) + if (qemuDomainObjEndAsyncJob(vm) > 0) virDomainRemoveInactive(&driver->domains, vm); vm = NULL; } @@ -1394,7 +1408,7 @@ qemuMigrationPrepareDirect(struct qemud_driver *driver, endjob: if (vm && - qemuDomainObjEndJob(vm) == 0) + qemuDomainObjEndAsyncJob(vm) == 0) vm = NULL; /* We set a fake job active which is held across @@ -1404,7 +1418,7 @@ endjob: */ if (vm && virDomainObjIsActive(vm)) { - qemuDomainObjSetJob(vm, QEMU_JOB_MIGRATION_IN); + priv->job.asyncJob = QEMU_ASYNC_JOB_MIGRATION_IN; priv->job.info.type = VIR_DOMAIN_JOB_UNBOUNDED; priv->job.start = now; } @@ -1488,7 +1502,9 @@ static int doNativeMigrate(struct qemud_driver *driver, goto cleanup; } - qemuDomainObjEnterMonitorWithDriver(driver, vm); + if (qemuDomainObjEnterMonitorWithDriver(driver, vm) < 0) + goto cleanup; + if (resource > 0 && qemuMonitorSetMigrationSpeed(priv->mon, resource) < 0) { qemuDomainObjExitMonitorWithDriver(driver, vm); @@ -1747,7 +1763,9 @@ static int doTunnelMigrate(struct qemud_driver *driver, goto cleanup; } - qemuDomainObjEnterMonitorWithDriver(driver, vm); + if (qemuDomainObjEnterMonitorWithDriver(driver, vm) < 0) + goto cleanup; + if (resource > 0 && qemuMonitorSetMigrationSpeed(priv->mon, resource) < 0) { qemuDomainObjExitMonitorWithDriver(driver, vm); @@ -1788,7 +1806,8 @@ static int doTunnelMigrate(struct qemud_driver *driver, /* it is also possible that the migrate didn't fail initially, but * rather failed later on. Check the output of "info migrate" */ - qemuDomainObjEnterMonitorWithDriver(driver, vm); + if (qemuDomainObjEnterMonitorWithDriver(driver, vm) < 0) + goto cancel; if (qemuMonitorGetMigrationStatus(priv->mon, &status, &transferred, @@ -1846,9 +1865,10 @@ cancel: if (ret != 0 && virDomainObjIsActive(vm)) { VIR_FORCE_CLOSE(client_sock); VIR_FORCE_CLOSE(qemu_sock); - qemuDomainObjEnterMonitorWithDriver(driver, vm); - qemuMonitorMigrateCancel(priv->mon); - qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjEnterMonitorWithDriver(driver, vm) == 0) { + qemuMonitorMigrateCancel(priv->mon); + qemuDomainObjExitMonitorWithDriver(driver, vm); + } } cleanup: @@ -2284,9 +2304,9 @@ int qemuMigrationPerform(struct qemud_driver *driver, cookieout, cookieoutlen, flags, NULLSTR(dname), resource, v3proto); - if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) + if (qemuDomainObjBeginAsyncJobWithDriver(driver, vm, + QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) goto cleanup; - qemuDomainObjSetJob(vm, QEMU_JOB_MIGRATION_OUT); if (!virDomainObjIsActive(vm)) { qemuReportError(VIR_ERR_OPERATION_INVALID, @@ -2365,7 +2385,7 @@ endjob: VIR_DOMAIN_EVENT_RESUMED_MIGRATED); } if (vm) { - if (qemuDomainObjEndJob(vm) == 0) { + if (qemuDomainObjEndAsyncJob(vm) == 0) { vm = NULL; } else if (!virDomainObjIsActive(vm) && (!vm->persistent || (flags & VIR_MIGRATE_UNDEFINE_SOURCE))) { @@ -2450,17 +2470,17 @@ qemuMigrationFinish(struct qemud_driver *driver, virErrorPtr orig_err = NULL; priv = vm->privateData; - if (priv->job.active != QEMU_JOB_MIGRATION_IN) { + if (priv->job.asyncJob != QEMU_ASYNC_JOB_MIGRATION_IN) { qemuReportError(VIR_ERR_NO_DOMAIN, _("domain '%s' is not processing incoming migration"), vm->def->name); goto cleanup; } - qemuDomainObjDiscardJob(vm); + qemuDomainObjDiscardAsyncJob(vm); if (!(mig = qemuMigrationEatCookie(driver, vm, cookiein, cookieinlen, 0))) goto cleanup; - if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) + if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; /* Did the migration go as planned? If yes, return the domain @@ -2724,7 +2744,9 @@ qemuMigrationToFile(struct qemud_driver *driver, virDomainObjPtr vm, restoreLabel = true; } - qemuDomainObjEnterMonitorWithDriver(driver, vm); + if (qemuDomainObjEnterMonitorWithDriver(driver, vm) < 0) + goto cleanup; + if (!compressor) { const char *args[] = { "cat", NULL }; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 56593c2..534a8b1 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -374,7 +374,7 @@ qemuProcessFakeReboot(void *opaque) VIR_DEBUG("vm=%p", vm); qemuDriverLock(driver); virDomainObjLock(vm); - if (qemuDomainObjBeginJob(vm) < 0) + if (qemuDomainObjBeginJob(vm, QEMU_JOB_MODIFY) < 0) goto cleanup; if (!virDomainObjIsActive(vm)) { @@ -383,7 +383,7 @@ qemuProcessFakeReboot(void *opaque) goto endjob; } - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); if (qemuMonitorSystemReset(priv->mon) < 0) { qemuDomainObjExitMonitorWithDriver(driver, vm); goto endjob; @@ -814,7 +814,7 @@ qemuConnectMonitor(struct qemud_driver *driver, virDomainObjPtr vm) } - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); ret = qemuMonitorSetCapabilities(priv->mon); qemuDomainObjExitMonitorWithDriver(driver, vm); @@ -1163,7 +1163,7 @@ qemuProcessWaitForMonitor(struct qemud_driver* driver, goto cleanup; priv = vm->privateData; - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); ret = qemuMonitorGetPtyPaths(priv->mon, paths); qemuDomainObjExitMonitorWithDriver(driver, vm); @@ -1216,7 +1216,7 @@ qemuProcessDetectVcpuPIDs(struct qemud_driver *driver, /* What follows is now all KVM specific */ - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); if ((ncpupids = qemuMonitorGetCPUInfo(priv->mon, &cpupids)) < 0) { qemuDomainObjExitMonitorWithDriver(driver, vm); return -1; @@ -1510,7 +1510,7 @@ qemuProcessInitPasswords(virConnectPtr conn, goto cleanup; alias = vm->def->disks[i]->info.alias; - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); ret = qemuMonitorSetDrivePassphrase(priv->mon, alias, secret); VIR_FREE(secret); qemuDomainObjExitMonitorWithDriver(driver, vm); @@ -1901,7 +1901,7 @@ qemuProcessInitPCIAddresses(struct qemud_driver *driver, int ret; qemuMonitorPCIAddress *addrs = NULL; - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); naddrs = qemuMonitorGetAllPCIAddresses(priv->mon, &addrs); qemuDomainObjExitMonitorWithDriver(driver, vm); @@ -2122,7 +2122,7 @@ qemuProcessStartCPUs(struct qemud_driver *driver, virDomainObjPtr vm, } VIR_FREE(priv->lockState); - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); ret = qemuMonitorStartCPUs(priv->mon, conn); qemuDomainObjExitMonitorWithDriver(driver, vm); @@ -2150,9 +2150,11 @@ int qemuProcessStopCPUs(struct qemud_driver *driver, virDomainObjPtr vm, oldState = virDomainObjGetState(vm, &oldReason); virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, reason); - qemuDomainObjEnterMonitorWithDriver(driver, vm); - ret = qemuMonitorStopCPUs(priv->mon); - qemuDomainObjExitMonitorWithDriver(driver, vm); + ret = qemuDomainObjEnterMonitorWithDriver(driver, vm); + if (ret == 0) { + ret = qemuMonitorStopCPUs(priv->mon); + qemuDomainObjExitMonitorWithDriver(driver, vm); + } if (ret == 0) { if (virDomainLockProcessPause(driver->lockManager, vm, &priv->lockState) < 0) @@ -2198,7 +2200,7 @@ qemuProcessUpdateState(struct qemud_driver *driver, virDomainObjPtr vm) bool running; int ret; - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); ret = qemuMonitorGetStatus(priv->mon, &running); qemuDomainObjExitMonitorWithDriver(driver, vm); @@ -2244,6 +2246,9 @@ qemuProcessReconnect(void *payload, const void *name ATTRIBUTE_UNUSED, void *opa priv = obj->privateData; + /* Set fake job so that EnterMonitor* doesn't want to start a new one */ + priv->job.active = QEMU_JOB_MODIFY; + /* Hold an extra reference because we can't allow 'vm' to be * deleted if qemuConnectMonitor() failed */ virDomainObjRef(obj); @@ -2282,6 +2287,8 @@ qemuProcessReconnect(void *payload, const void *name ATTRIBUTE_UNUSED, void *opa if (qemuProcessFiltersInstantiate(conn, obj->def)) goto error; + priv->job.active = QEMU_JOB_NONE; + /* update domain state XML with possibly updated state in virDomainObj */ if (virDomainSaveStatus(driver->caps, driver->stateDir, obj) < 0) goto error; @@ -2695,7 +2702,7 @@ int qemuProcessStart(virConnectPtr conn, VIR_DEBUG("Setting initial memory amount"); cur_balloon = vm->def->mem.cur_balloon; - qemuDomainObjEnterMonitorWithDriver(driver, vm); + ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); if (qemuMonitorSetBalloon(priv->mon, cur_balloon) < 0) { qemuDomainObjExitMonitorWithDriver(driver, vm); goto cleanup; @@ -3013,13 +3020,14 @@ static void qemuProcessAutoDestroyDom(void *payload, } priv = dom->privateData; - if (priv->job.active == QEMU_JOB_MIGRATION_IN) { - VIR_DEBUG("vm=%s has incoming migration active, cancelling", + if (priv->job.asyncJob) { + VIR_DEBUG("vm=%s has long-term job active, cancelling", dom->def->name); - qemuDomainObjDiscardJob(dom); + qemuDomainObjDiscardAsyncJob(dom); } - if (qemuDomainObjBeginJobWithDriver(data->driver, dom) < 0) + if (qemuDomainObjBeginJobWithDriver(data->driver, dom, + QEMU_JOB_DESTROY) < 0) goto cleanup; VIR_DEBUG("Killing domain"); -- 1.7.6

This patch introduces several helper methods to deal with jobs and phases during migration in a simpler manner. --- src/qemu/MIGRATION.txt | 55 +++++++++++++++++++++++++++ src/qemu/qemu_domain.c | 5 ++ src/qemu/qemu_migration.c | 91 +++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_migration.h | 36 ++++++++++++++++++ 4 files changed, 187 insertions(+), 0 deletions(-) create mode 100644 src/qemu/MIGRATION.txt diff --git a/src/qemu/MIGRATION.txt b/src/qemu/MIGRATION.txt new file mode 100644 index 0000000..6c32998 --- /dev/null +++ b/src/qemu/MIGRATION.txt @@ -0,0 +1,55 @@ + QEMU Migration Locking Rules + ============================ + +Migration is a complicated beast which may span across several APIs on both +source and destination side and we need to keep the domain we are migrating in +a consistent state during the whole process. + +To avoid anyone from changing the domain in the middle of migration we need to +keep MIGRATION_OUT job active during migration from Begin to Confirm on the +source side and MIGRATION_IN job has to be active from Prepare to Finish on +the destination side. + +For this purpose we introduce several helper methods to deal with locking +primitives (described in THREADS.txt) in the right way: + +* qemuMigrationJobStart + +* qemuMigrationJobContinue + +* qemuMigrationJobStartPhase + +* qemuMigrationJobSetPhase + +* qemuMigrationJobFinish + +The sequence of calling qemuMigrationJob* helper methods is as follows: + +- The first API of a migration protocol (Prepare or Perform/Begin depending on + migration type and version) has to start migration job and keep it active: + + qemuMigrationJobStart(driver, vm, QEMU_JOB_MIGRATION_{IN,OUT}); + qemuMigrationJobSetPhase(driver, vm, QEMU_MIGRATION_PHASE_*); + ...do work... + qemuMigrationJobContinue(vm); + +- All consequent phases except for the last one have to keep the job active: + + if (!qemuMigrationJobIsActive(vm, QEMU_JOB_MIGRATION_{IN,OUT})) + return; + qemuMigrationJobStartPhase(driver, vm, QEMU_MIGRATION_PHASE_*); + ...do work... + qemuMigrationJobContinue(vm); + +- The last migration phase finally finishes the migration job: + + if (!qemuMigrationJobIsActive(vm, QEMU_JOB_MIGRATION_{IN,OUT})) + return; + qemuMigrationJobStartPhase(driver, vm, QEMU_MIGRATION_PHASE_*); + ...do work... + qemuMigrationJobFinish(driver, vm); + +While migration job is running (i.e., after qemuMigrationJobStart* but before +qemuMigrationJob{Continue,Finish}), migration phase can be advanced using + + qemuMigrationJobSetPhase(driver, vm, QEMU_MIGRATION_PHASE_*); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d0dd764..39cbf0e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -26,6 +26,7 @@ #include "qemu_domain.h" #include "qemu_command.h" #include "qemu_capabilities.h" +#include "qemu_migration.h" #include "memory.h" #include "logging.h" #include "virterror_internal.h" @@ -72,6 +73,8 @@ qemuDomainAsyncJobPhaseToString(enum qemuDomainAsyncJob job, switch (job) { case QEMU_ASYNC_JOB_MIGRATION_OUT: case QEMU_ASYNC_JOB_MIGRATION_IN: + return qemuMigrationJobPhaseTypeToString(phase); + case QEMU_ASYNC_JOB_SAVE: case QEMU_ASYNC_JOB_DUMP: case QEMU_ASYNC_JOB_NONE: @@ -92,6 +95,8 @@ qemuDomainAsyncJobPhaseFromString(enum qemuDomainAsyncJob job, switch (job) { case QEMU_ASYNC_JOB_MIGRATION_OUT: case QEMU_ASYNC_JOB_MIGRATION_IN: + return qemuMigrationJobPhaseTypeFromString(phase); + case QEMU_ASYNC_JOB_SAVE: case QEMU_ASYNC_JOB_DUMP: case QEMU_ASYNC_JOB_NONE: diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index e595596..33aa89b 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -46,6 +46,19 @@ #define VIR_FROM_THIS VIR_FROM_QEMU +VIR_ENUM_IMPL(qemuMigrationJobPhase, QEMU_MIGRATION_PHASE_LAST, + "none", + "preform2", + "begin3", + "perform3", + "perform3_done", + "confirm3_cancelled", + "confirm3", + "prepare", + "finish2", + "finish3", +); + enum qemuMigrationCookieFlags { QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS, QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE, @@ -2749,3 +2762,81 @@ cleanup: } return ret; } + +int +qemuMigrationJobStart(struct qemud_driver *driver, + virDomainObjPtr vm, + enum qemuDomainAsyncJob job) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (qemuDomainObjBeginAsyncJobWithDriver(driver, vm, job) < 0) + return -1; + + if (job == QEMU_ASYNC_JOB_MIGRATION_IN) + qemuDomainObjSetAsyncJobMask(vm, QEMU_JOB_NONE); + else + qemuDomainObjSetAsyncJobMask(vm, DEFAULT_JOB_MASK); + + priv->job.info.type = VIR_DOMAIN_JOB_UNBOUNDED; + + return 0; +} + +void +qemuMigrationJobSetPhase(struct qemud_driver *driver, + virDomainObjPtr vm, + enum qemuMigrationJobPhase phase) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (phase < priv->job.phase) { + VIR_ERROR(_("migration protocol going backwards %s => %s"), + qemuMigrationJobPhaseTypeToString(priv->job.phase), + qemuMigrationJobPhaseTypeToString(phase)); + return; + } + + qemuDomainObjSetJobPhase(driver, vm, phase); +} + +void +qemuMigrationJobStartPhase(struct qemud_driver *driver, + virDomainObjPtr vm, + enum qemuMigrationJobPhase phase) +{ + virDomainObjRef(vm); + qemuMigrationJobSetPhase(driver, vm, phase); +} + +int +qemuMigrationJobContinue(virDomainObjPtr vm) +{ + return virDomainObjUnref(vm); +} + +bool +qemuMigrationJobIsActive(virDomainObjPtr vm, + enum qemuDomainAsyncJob job) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (priv->job.asyncJob != job) { + const char *msg; + + if (job == QEMU_ASYNC_JOB_MIGRATION_IN) + msg = _("domain '%s' is not processing incoming migration"); + else + msg = _("domain '%s' is not being migrated"); + + qemuReportError(VIR_ERR_OPERATION_INVALID, msg, vm->def->name); + return false; + } + return true; +} + +int +qemuMigrationJobFinish(struct qemud_driver *driver, virDomainObjPtr vm) +{ + return qemuDomainObjEndAsyncJob(driver, vm); +} diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index d3a3743..4342173 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -23,7 +23,43 @@ # define __QEMU_MIGRATION_H__ # include "qemu_conf.h" +# include "qemu_domain.h" +enum qemuMigrationJobPhase { + QEMU_MIGRATION_PHASE_NONE = 0, + QEMU_MIGRATION_PHASE_PERFORM2, + QEMU_MIGRATION_PHASE_BEGIN3, + QEMU_MIGRATION_PHASE_PERFORM3, + QEMU_MIGRATION_PHASE_PERFORM3_DONE, + QEMU_MIGRATION_PHASE_CONFIRM3_CANCELLED, + QEMU_MIGRATION_PHASE_CONFIRM3, + QEMU_MIGRATION_PHASE_PREPARE, + QEMU_MIGRATION_PHASE_FINISH2, + QEMU_MIGRATION_PHASE_FINISH3, + + QEMU_MIGRATION_PHASE_LAST +}; +VIR_ENUM_DECL(qemuMigrationJobPhase) + +int qemuMigrationJobStart(struct qemud_driver *driver, + virDomainObjPtr vm, + enum qemuDomainAsyncJob job) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +void qemuMigrationJobSetPhase(struct qemud_driver *driver, + virDomainObjPtr vm, + enum qemuMigrationJobPhase phase) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +void qemuMigrationJobStartPhase(struct qemud_driver *driver, + virDomainObjPtr vm, + enum qemuMigrationJobPhase phase) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +int qemuMigrationJobContinue(virDomainObjPtr obj) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; +bool qemuMigrationJobIsActive(virDomainObjPtr vm, + enum qemuDomainAsyncJob job) + ATTRIBUTE_NONNULL(1); +int qemuMigrationJobFinish(struct qemud_driver *driver, virDomainObjPtr obj) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; bool qemuMigrationIsAllowed(virDomainDefPtr def) ATTRIBUTE_NONNULL(1); -- 1.7.6

Make MIGRATION_OUT use the new helper methods. This also introduces new protection to migration v3 process: the migration job is held from Begin to Confirm to avoid changes to a domain during migration (esp. between Begin and Perform phases). This change, however brings a small issue when Prepare step fails during normal (not p2p or tunneled) migration when old client talks to new libvirtd. The old client doesn't call Confirm (with cancelled == 1) to end the migration job. I'm open to suggestions how to solve this in the best way. My idea is to use flags for Begin3 and Perform3 that would enable this extra protection by moving migration job start from Perform to Begin phase while maintaining the old behavior if the flags are not set. Note that virDomainAbortJob will be able to cancel migration in any phase (i.e., not only in Perform phase). --- src/libvirt.c | 11 ++- src/qemu/qemu_driver.c | 47 ++++++++- src/qemu/qemu_migration.c | 232 +++++++++++++++++++++++++++++++-------------- 3 files changed, 211 insertions(+), 79 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index e00c64f..fa32d83 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -3761,7 +3761,7 @@ virDomainMigrateVersion3(virDomainPtr domain, int ret; virDomainInfo info; virErrorPtr orig_err = NULL; - int cancelled; + int cancelled = 1; VIR_DOMAIN_DEBUG(domain, "dconn=%p xmlin=%s, flags=%lu, " "dname=%s, uri=%s, bandwidth=%lu", dconn, NULLSTR(xmlin), flags, @@ -3798,14 +3798,16 @@ virDomainMigrateVersion3(virDomainPtr domain, (dconn, cookiein, cookieinlen, &cookieout, &cookieoutlen, uri, &uri_out, flags, dname, bandwidth, dom_xml); VIR_FREE (dom_xml); - if (ret == -1) - goto done; + if (ret == -1) { + /* Make sure Confirm doesn't overwrite the error */ + orig_err = virSaveLastError(); + goto confirm; + } if (uri == NULL && uri_out == NULL) { virLibConnError(VIR_ERR_INTERNAL_ERROR, _("domainMigratePrepare3 did not set uri")); virDispatchError(domain->conn); - cancelled = 1; goto finish; } if (uri_out) @@ -3871,6 +3873,7 @@ finish: if (!orig_err) orig_err = virSaveLastError(); +confirm: /* * If cancelled, then src VM will be restarted, else * it will be killed diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9dcb248..e317c5b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6850,12 +6850,42 @@ qemuDomainMigrateBegin3(virDomainPtr domain, goto cleanup; } - xml = qemuMigrationBegin(driver, vm, xmlin, - cookieout, cookieoutlen); + if (qemuMigrationJobStart(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto endjob; + } + + if (!(xml = qemuMigrationBegin(driver, vm, xmlin, + cookieout, cookieoutlen))) + goto endjob; + + /* We keep the job active across API calls until the confirm() call. + * This prevents any other APIs being invoked while migration is taking + * place. + */ + if (qemuMigrationJobContinue(vm) == 0) { + vm = NULL; + qemuReportError(VIR_ERR_OPERATION_FAILED, + "%s", _("domain disappeared")); + VIR_FREE(xml); + if (cookieout) + VIR_FREE(*cookieout); + } cleanup: + if (vm) + virDomainObjUnlock(vm); qemuDriverUnlock(driver); return xml; + +endjob: + if (qemuMigrationJobFinish(driver, vm) == 0) + vm = NULL; + goto cleanup; } static int @@ -7005,7 +7035,6 @@ qemuDomainMigratePerform3(virDomainPtr dom, dconnuri, uri, cookiein, cookieinlen, cookieout, cookieoutlen, flags, dname, resource, true); - cleanup: qemuDriverUnlock(driver); return ret; @@ -7065,6 +7094,7 @@ qemuDomainMigrateConfirm3(virDomainPtr domain, struct qemud_driver *driver = domain->conn->privateData; virDomainObjPtr vm; int ret = -1; + enum qemuMigrationJobPhase phase; virCheckFlags(VIR_MIGRATE_LIVE | VIR_MIGRATE_PEER2PEER | @@ -7085,14 +7115,21 @@ qemuDomainMigrateConfirm3(virDomainPtr domain, goto cleanup; } - if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) + if (!qemuMigrationJobIsActive(vm, QEMU_ASYNC_JOB_MIGRATION_OUT)) goto cleanup; + if (cancelled) + phase = QEMU_MIGRATION_PHASE_CONFIRM3_CANCELLED; + else + phase = QEMU_MIGRATION_PHASE_CONFIRM3; + + qemuMigrationJobStartPhase(driver, vm, phase); + ret = qemuMigrationConfirm(driver, domain->conn, vm, cookiein, cookieinlen, flags, cancelled); - if (qemuDomainObjEndJob(driver, vm) == 0) { + if (qemuMigrationJobFinish(driver, vm) == 0) { vm = NULL; } else if (!virDomainObjIsActive(vm) && (!vm->persistent || (flags & VIR_MIGRATE_UNDEFINE_SOURCE))) { diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 5e31b7d..bcca454 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1009,6 +1009,7 @@ qemuDomainMigrateGraphicsRelocate(struct qemud_driver *driver, } +/* The caller is supposed to lock the vm and start a migration job. */ char *qemuMigrationBegin(struct qemud_driver *driver, virDomainObjPtr vm, const char *xmlin, @@ -1021,11 +1022,7 @@ char *qemuMigrationBegin(struct qemud_driver *driver, VIR_DEBUG("driver=%p, vm=%p, xmlin=%s, cookieout=%p, cookieoutlen=%p", driver, vm, NULLSTR(xmlin), cookieout, cookieoutlen); - if (!virDomainObjIsActive(vm)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); - goto cleanup; - } + qemuMigrationJobSetPhase(driver, vm, QEMU_MIGRATION_PHASE_BEGIN3); if (qemuProcessAutoDestroyActive(driver, vm)) { qemuReportError(VIR_ERR_OPERATION_INVALID, @@ -1062,7 +1059,6 @@ char *qemuMigrationBegin(struct qemud_driver *driver, } cleanup: - virDomainObjUnlock(vm); qemuMigrationCookieFree(mig); virDomainDefFree(def); return rv; @@ -1902,6 +1898,7 @@ static int doPeer2PeerMigrate2(struct qemud_driver *driver, * until the migration is complete. */ VIR_DEBUG("Perform %p", sconn); + qemuMigrationJobSetPhase(driver, vm, QEMU_MIGRATION_PHASE_PERFORM2); if (flags & VIR_MIGRATE_TUNNELLED) ret = doTunnelMigrate(driver, vm, st, NULL, 0, NULL, NULL, @@ -2036,6 +2033,7 @@ static int doPeer2PeerMigrate3(struct qemud_driver *driver, * confirm migration completion. */ VIR_DEBUG("Perform3 %p uri=%s uri_out=%s", sconn, uri, uri_out); + qemuMigrationJobSetPhase(driver, vm, QEMU_MIGRATION_PHASE_PERFORM3); VIR_FREE(cookiein); cookiein = cookieout; cookieinlen = cookieoutlen; @@ -2053,8 +2051,12 @@ static int doPeer2PeerMigrate3(struct qemud_driver *driver, flags, dname, resource); /* Perform failed. Make sure Finish doesn't overwrite the error */ - if (ret < 0) + if (ret < 0) { orig_err = virSaveLastError(); + } else { + qemuMigrationJobSetPhase(driver, vm, + QEMU_MIGRATION_PHASE_PERFORM3_DONE); + } /* If Perform returns < 0, then we need to cancel the VM * startup on the destination @@ -2211,35 +2213,32 @@ cleanup: } -int qemuMigrationPerform(struct qemud_driver *driver, - virConnectPtr conn, - virDomainObjPtr vm, - const char *xmlin, - const char *dconnuri, - const char *uri, - const char *cookiein, - int cookieinlen, - char **cookieout, - int *cookieoutlen, - unsigned long flags, - const char *dname, - unsigned long resource, - bool v3proto) +/* + * This implements perform part of the migration protocol when migration job + * does not need to be active across several APIs, i.e., peer2peer migration or + * perform phase of v2 non-peer2peer migration. + */ +static int +qemuMigrationPerformJob(struct qemud_driver *driver, + virConnectPtr conn, + virDomainObjPtr vm, + const char *xmlin, + const char *dconnuri, + const char *uri, + const char *cookiein, + int cookieinlen, + char **cookieout, + int *cookieoutlen, + unsigned long flags, + const char *dname, + unsigned long resource, + bool v3proto) { virDomainEventPtr event = NULL; int ret = -1; int resume = 0; - qemuDomainObjPrivatePtr priv = vm->privateData; - VIR_DEBUG("driver=%p, conn=%p, vm=%p, xmlin=%s, dconnuri=%s, " - "uri=%s, cookiein=%s, cookieinlen=%d, cookieout=%p, " - "cookieoutlen=%p, flags=%lu, dname=%s, resource=%lu, v3proto=%d", - driver, conn, vm, NULLSTR(xmlin), NULLSTR(dconnuri), - NULLSTR(uri), NULLSTR(cookiein), cookieinlen, - cookieout, cookieoutlen, flags, NULLSTR(dname), - resource, v3proto); - - if (qemuDomainObjBeginAsyncJobWithDriver(driver, vm, - QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) + + if (qemuMigrationJobStart(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT) < 0) goto cleanup; if (!virDomainObjIsActive(vm)) { @@ -2254,52 +2253,33 @@ int qemuMigrationPerform(struct qemud_driver *driver, goto endjob; } - memset(&priv->job.info, 0, sizeof(priv->job.info)); - priv->job.info.type = VIR_DOMAIN_JOB_UNBOUNDED; - resume = virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING; if ((flags & (VIR_MIGRATE_TUNNELLED | VIR_MIGRATE_PEER2PEER))) { - if (cookieinlen) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("received unexpected cookie with P2P migration")); - goto endjob; - } - - if (doPeer2PeerMigrate(driver, conn, vm, xmlin, - dconnuri, uri, flags, dname, - resource, &v3proto) < 0) - /* doPeer2PeerMigrate already set the error, so just get out */ - goto endjob; + ret = doPeer2PeerMigrate(driver, conn, vm, xmlin, + dconnuri, uri, flags, dname, + resource, &v3proto); } else { - if (dconnuri) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Unexpected dconnuri parameter with non-peer2peer migration")); - goto endjob; - } - if (doNativeMigrate(driver, vm, uri, cookiein, cookieinlen, - cookieout, cookieoutlen, - flags, dname, resource) < 0) - goto endjob; + qemuMigrationJobSetPhase(driver, vm, QEMU_MIGRATION_PHASE_PERFORM2); + ret = doNativeMigrate(driver, vm, uri, cookiein, cookieinlen, + cookieout, cookieoutlen, + flags, dname, resource); } + if (ret < 0) + goto endjob; /* * In v3 protocol, the source VM is not killed off until the * confirm step. */ - if (v3proto) { - resume = 0; - } else { + if (!v3proto) { qemuProcessStop(driver, vm, 1, VIR_DOMAIN_SHUTOFF_MIGRATED); qemuAuditDomainStop(vm, "migrated"); - resume = 0; - event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_MIGRATED); } - - ret = 0; + resume = 0; endjob: if (resume && virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) { @@ -2319,10 +2299,11 @@ endjob: VIR_DOMAIN_EVENT_RESUMED_MIGRATED); } if (vm) { - if (qemuDomainObjEndAsyncJob(driver, vm) == 0) { + if (qemuMigrationJobFinish(driver, vm) == 0) { vm = NULL; } else if (!virDomainObjIsActive(vm) && - (!vm->persistent || (flags & VIR_MIGRATE_UNDEFINE_SOURCE))) { + (!vm->persistent || + (ret == 0 && (flags & VIR_MIGRATE_UNDEFINE_SOURCE)))) { if (flags & VIR_MIGRATE_UNDEFINE_SOURCE) virDomainDeleteConfig(driver->configDir, driver->autostartDir, vm); virDomainRemoveInactive(&driver->domains, vm); @@ -2338,6 +2319,118 @@ cleanup: return ret; } +/* + * This implements perform phase of v3 migration protocol. + */ +static int +qemuMigrationPerformPhase(struct qemud_driver *driver, + virConnectPtr conn, + virDomainObjPtr vm, + const char *uri, + const char *cookiein, + int cookieinlen, + char **cookieout, + int *cookieoutlen, + unsigned long flags, + const char *dname, + unsigned long resource) +{ + virDomainEventPtr event = NULL; + int ret = -1; + bool resume; + + qemuMigrationJobStartPhase(driver, vm, QEMU_MIGRATION_PHASE_PERFORM3); + + resume = virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING; + ret = doNativeMigrate(driver, vm, uri, cookiein, cookieinlen, + cookieout, cookieoutlen, + flags, dname, resource); + + if (ret < 0 && resume && + virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) { + /* we got here through some sort of failure; start the domain again */ + if (qemuProcessStartCPUs(driver, vm, conn, + VIR_DOMAIN_RUNNING_MIGRATION_CANCELED) < 0) { + /* Hm, we already know we are in error here. We don't want to + * overwrite the previous error, though, so we just throw something + * to the logs and hope for the best + */ + VIR_ERROR(_("Failed to resume guest %s after failure"), + vm->def->name); + } + + event = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_RESUMED, + VIR_DOMAIN_EVENT_RESUMED_MIGRATED); + } + if (ret == 0) { + qemuMigrationJobSetPhase(driver, vm, + QEMU_MIGRATION_PHASE_PERFORM3_DONE); + } + + if (vm) { + if (qemuMigrationJobContinue(vm) == 0) { + vm = NULL; + } else if (!virDomainObjIsActive(vm) && !vm->persistent) { + virDomainRemoveInactive(&driver->domains, vm); + vm = NULL; + } + } + + if (vm) + virDomainObjUnlock(vm); + if (event) + qemuDomainEventQueue(driver, event); + return ret; +} + +int +qemuMigrationPerform(struct qemud_driver *driver, + virConnectPtr conn, + virDomainObjPtr vm, + const char *xmlin, + const char *dconnuri, + const char *uri, + const char *cookiein, + int cookieinlen, + char **cookieout, + int *cookieoutlen, + unsigned long flags, + const char *dname, + unsigned long resource, + bool v3proto) +{ + if ((flags & (VIR_MIGRATE_TUNNELLED | VIR_MIGRATE_PEER2PEER))) { + if (cookieinlen) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("received unexpected cookie with P2P migration")); + return -1; + } + + return qemuMigrationPerformJob(driver, conn, vm, xmlin, dconnuri, uri, + cookiein, cookieinlen, cookieout, + cookieoutlen, flags, dname, resource, + v3proto); + } else { + if (dconnuri) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Unexpected dconnuri parameter with non-peer2peer migration")); + return -1; + } + + if (v3proto) { + return qemuMigrationPerformPhase(driver, conn, vm, uri, + cookiein, cookieinlen, + cookieout, cookieoutlen, + flags, dname, resource); + } else { + return qemuMigrationPerformJob(driver, conn, vm, xmlin, dconnuri, + uri, cookiein, cookieinlen, + cookieout, cookieoutlen, flags, + dname, resource, v3proto); + } + } +} #if WITH_MACVTAP static void @@ -2559,15 +2652,14 @@ int qemuMigrationConfirm(struct qemud_driver *driver, driver, conn, vm, NULLSTR(cookiein), cookieinlen, flags, retcode); + qemuMigrationJobSetPhase(driver, vm, + retcode == 0 + ? QEMU_MIGRATION_PHASE_CONFIRM3 + : QEMU_MIGRATION_PHASE_CONFIRM3_CANCELLED); + if (!(mig = qemuMigrationEatCookie(driver, vm, cookiein, cookieinlen, 0))) return -1; - if (!virDomainObjIsActive(vm)) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("guest unexpectedly quit")); - goto cleanup; - } - /* Did the migration go as planned? If yes, kill off the * domain object, but if no, resume CPUs */ -- 1.7.6

When monitor is entered with qemuDomainObjEnterMonitorWithDriver, the correct method for leaving and unlocking the monitor is qemuDomainObjExitMonitorWithDriver. --- src/qemu/qemu_hotplug.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 06e2c84..c86f128 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1240,14 +1240,14 @@ int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver, ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { - qemuDomainObjExitMonitor(driver, vm); + qemuDomainObjExitMonitorWithDriver(driver, vm); qemuAuditDisk(vm, detach, NULL, "detach", false); goto cleanup; } } else { if (qemuMonitorRemovePCIDevice(priv->mon, &detach->info.addr.pci) < 0) { - qemuDomainObjExitMonitor(driver, vm); + qemuDomainObjExitMonitorWithDriver(driver, vm); qemuAuditDisk(vm, detach, NULL, "detach", false); goto cleanup; } @@ -1335,7 +1335,7 @@ int qemuDomainDetachDiskDevice(struct qemud_driver *driver, ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { - qemuDomainObjExitMonitor(driver, vm); + qemuDomainObjExitMonitorWithDriver(driver, vm); qemuAuditDisk(vm, detach, NULL, "detach", false); goto cleanup; } @@ -1474,13 +1474,13 @@ int qemuDomainDetachPciControllerDevice(struct qemud_driver *driver, ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { if (qemuMonitorDelDevice(priv->mon, detach->info.alias)) { - qemuDomainObjExitMonitor(driver, vm); + qemuDomainObjExitMonitorWithDriver(driver, vm); goto cleanup; } } else { if (qemuMonitorRemovePCIDevice(priv->mon, &detach->info.addr.pci) < 0) { - qemuDomainObjExitMonitor(driver, vm); + qemuDomainObjExitMonitorWithDriver(driver, vm); goto cleanup; } } @@ -1569,7 +1569,7 @@ int qemuDomainDetachNetDevice(struct qemud_driver *driver, ignore_value(qemuDomainObjEnterMonitorWithDriver(driver, vm)); if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { - qemuDomainObjExitMonitor(driver, vm); + qemuDomainObjExitMonitorWithDriver(driver, vm); qemuAuditNet(vm, detach, NULL, "detach", false); goto cleanup; } -- 1.7.6

Like other query commands, this can now be called directly during migration. --- src/qemu/qemu_domain.h | 4 ---- src/qemu/qemu_driver.c | 42 ++++++++++++------------------------------ src/qemu/qemu_migration.c | 14 -------------- 3 files changed, 12 insertions(+), 48 deletions(-) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 387c64c..0958bf2 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -71,7 +71,6 @@ enum qemuDomainJobSignals { QEMU_JOB_SIGNAL_MIGRATE_DOWNTIME = 1 << 2, /* Request migration downtime change */ QEMU_JOB_SIGNAL_MIGRATE_SPEED = 1 << 3, /* Request migration speed change */ QEMU_JOB_SIGNAL_BLKSTAT = 1 << 4, /* Request blkstat during migration */ - QEMU_JOB_SIGNAL_BLKINFO = 1 << 5, /* Request blkinfo during migration */ }; struct qemuDomainJobSignalsData { @@ -80,9 +79,6 @@ struct qemuDomainJobSignalsData { char *statDevName; /* Device name used by blkstat calls */ virDomainBlockStatsPtr blockStat; /* Block statistics for QEMU_JOB_SIGNAL_BLKSTAT */ int *statRetCode; /* Return code for the blkstat calls */ - char *infoDevName; /* Device name used by blkinfo calls */ - virDomainBlockInfoPtr blockInfo; /* Block information for QEMU_JOB_SIGNAL_BLKINFO */ - int *infoRetCode; /* Return code for the blkinfo calls */ }; struct qemuDomainJobObj { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e317c5b..76dfbd8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6455,39 +6455,21 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom, virDomainObjIsActive(vm)) { qemuDomainObjPrivatePtr priv = vm->privateData; - if ((priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT) - || (priv->job.asyncJob == QEMU_ASYNC_JOB_SAVE)) { - virDomainObjRef(vm); - while (priv->job.signals & QEMU_JOB_SIGNAL_BLKINFO) - ignore_value(virCondWait(&priv->job.signalCond, &vm->lock)); - - priv->job.signalsData.infoDevName = disk->info.alias; - priv->job.signalsData.blockInfo = info; - priv->job.signalsData.infoRetCode = &ret; - priv->job.signals |= QEMU_JOB_SIGNAL_BLKINFO; - - while (priv->job.signals & QEMU_JOB_SIGNAL_BLKINFO) - ignore_value(virCondWait(&priv->job.signalCond, &vm->lock)); + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) + goto cleanup; - if (virDomainObjUnref(vm) == 0) - vm = NULL; + if (virDomainObjIsActive(vm)) { + ignore_value(qemuDomainObjEnterMonitor(driver, vm)); + ret = qemuMonitorGetBlockExtent(priv->mon, + disk->info.alias, + &info->allocation); + qemuDomainObjExitMonitor(driver, vm); } else { - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) - goto cleanup; - - if (virDomainObjIsActive(vm)) { - ignore_value(qemuDomainObjEnterMonitor(driver, vm)); - ret = qemuMonitorGetBlockExtent(priv->mon, - disk->info.alias, - &info->allocation); - qemuDomainObjExitMonitor(driver, vm); - } else { - ret = 0; - } - - if (qemuDomainObjEndJob(driver, vm) == 0) - vm = NULL; + ret = 0; } + + if (qemuDomainObjEndJob(driver, vm) == 0) + vm = NULL; } else { ret = 0; } diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index bcca454..8a14d2e 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -819,20 +819,6 @@ qemuMigrationProcessJobSignals(struct qemud_driver *driver, if (ret < 0) VIR_WARN("Unable to get block statistics"); - } else if (priv->job.signals & QEMU_JOB_SIGNAL_BLKINFO) { - ret = qemuDomainObjEnterMonitorWithDriver(driver, vm); - if (ret == 0) { - ret = qemuMonitorGetBlockExtent(priv->mon, - priv->job.signalsData.infoDevName, - &priv->job.signalsData.blockInfo->allocation); - qemuDomainObjExitMonitorWithDriver(driver, vm); - } - - *priv->job.signalsData.infoRetCode = ret; - priv->job.signals ^= QEMU_JOB_SIGNAL_BLKINFO; - - if (ret < 0) - VIR_WARN("Unable to get block information"); } else { ret = 0; } -- 1.7.6

Call qemu monitor command directly within a special job that is only allowed during outgoing migration. --- src/qemu/qemu_domain.c | 1 + src/qemu/qemu_domain.h | 3 +-- src/qemu/qemu_driver.c | 23 +++++++++++++++-------- src/qemu/qemu_migration.c | 21 +++++---------------- src/qemu/qemu_process.c | 1 + 5 files changed, 23 insertions(+), 26 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2e4228a..c47ab60 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -52,6 +52,7 @@ VIR_ENUM_IMPL(qemuDomainJob, QEMU_JOB_LAST, "destroy", "suspend", "modify", + "migration operation", "none", /* async job is never stored in job.active */ "async nested", ); diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index c2ad456..7e6b522 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -43,6 +43,7 @@ enum qemuDomainJob { QEMU_JOB_DESTROY, /* Destroys the domain (cannot be masked out) */ QEMU_JOB_SUSPEND, /* Suspends (stops vCPUs) the domain */ QEMU_JOB_MODIFY, /* May change state */ + QEMU_JOB_MIGRATION_OP, /* Operation influencing outgoing migration */ /* The following two items must always be the last items before JOB_LAST */ QEMU_JOB_ASYNC, /* Asynchronous job */ @@ -69,12 +70,10 @@ enum qemuDomainJobSignals { QEMU_JOB_SIGNAL_CANCEL = 1 << 0, /* Request job cancellation */ QEMU_JOB_SIGNAL_SUSPEND = 1 << 1, /* Request VM suspend to finish live migration offline */ QEMU_JOB_SIGNAL_MIGRATE_DOWNTIME = 1 << 2, /* Request migration downtime change */ - QEMU_JOB_SIGNAL_MIGRATE_SPEED = 1 << 3, /* Request migration speed change */ }; struct qemuDomainJobSignalsData { unsigned long long migrateDowntime; /* Data for QEMU_JOB_SIGNAL_MIGRATE_DOWNTIME */ - unsigned long migrateBandwidth; /* Data for QEMU_JOB_SIGNAL_MIGRATE_SPEED */ }; struct qemuDomainJobObj { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index cd64f85..1235787 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7432,19 +7432,23 @@ qemuDomainMigrateSetMaxSpeed(virDomainPtr dom, qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(dom->uuid, uuidstr); qemuReportError(VIR_ERR_NO_DOMAIN, _("no domain with matching uuid '%s'"), uuidstr); - goto cleanup; + return -1; } + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MIGRATION_OP) < 0) + goto cleanup; + if (!virDomainObjIsActive(vm)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); - goto cleanup; + goto endjob; } priv = vm->privateData; @@ -7452,18 +7456,21 @@ qemuDomainMigrateSetMaxSpeed(virDomainPtr dom, if (priv->job.asyncJob != QEMU_ASYNC_JOB_MIGRATION_OUT) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not being migrated")); - goto cleanup; + goto endjob; } - VIR_DEBUG("Requesting migration speed change to %luMbs", bandwidth); - priv->job.signalsData.migrateBandwidth = bandwidth; - priv->job.signals |= QEMU_JOB_SIGNAL_MIGRATE_SPEED; - ret = 0; + VIR_DEBUG("Setting migration bandwidth to %luMbs", bandwidth); + ignore_value(qemuDomainObjEnterMonitor(driver, vm)); + ret = qemuMonitorSetMigrationSpeed(priv->mon, bandwidth); + qemuDomainObjExitMonitor(driver, vm); + +endjob: + if (qemuDomainObjEndJob(driver, vm) == 0) + vm = NULL; cleanup: if (vm) virDomainObjUnlock(vm); - qemuDriverUnlock(driver); return ret; } diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index a7566f4..d7ad97f 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -788,19 +788,6 @@ qemuMigrationProcessJobSignals(struct qemud_driver *driver, } if (ret < 0) VIR_WARN("Unable to set migration downtime"); - } else if (priv->job.signals & QEMU_JOB_SIGNAL_MIGRATE_SPEED) { - unsigned long bandwidth = priv->job.signalsData.migrateBandwidth; - - priv->job.signals ^= QEMU_JOB_SIGNAL_MIGRATE_SPEED; - priv->job.signalsData.migrateBandwidth = 0; - VIR_DEBUG("Setting migration bandwidth to %luMbs", bandwidth); - ret = qemuDomainObjEnterMonitorWithDriver(driver, vm); - if (ret == 0) { - ret = qemuMonitorSetMigrationSpeed(priv->mon, bandwidth); - qemuDomainObjExitMonitorWithDriver(driver, vm); - } - if (ret < 0) - VIR_WARN("Unable to set migration speed"); } else { ret = 0; } @@ -2823,10 +2810,12 @@ qemuMigrationJobStart(struct qemud_driver *driver, if (qemuDomainObjBeginAsyncJobWithDriver(driver, vm, job) < 0) return -1; - if (job == QEMU_ASYNC_JOB_MIGRATION_IN) + if (job == QEMU_ASYNC_JOB_MIGRATION_IN) { qemuDomainObjSetAsyncJobMask(vm, QEMU_JOB_NONE); - else - qemuDomainObjSetAsyncJobMask(vm, DEFAULT_JOB_MASK); + } else { + qemuDomainObjSetAsyncJobMask(vm, DEFAULT_JOB_MASK | + JOB_MASK(QEMU_JOB_MIGRATION_OP)); + } priv->job.info.type = VIR_DOMAIN_JOB_UNBOUNDED; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ddb29c1..9126c71 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2394,6 +2394,7 @@ qemuProcessRecoverJob(struct qemud_driver *driver, */ break; + case QEMU_JOB_MIGRATION_OP: case QEMU_JOB_ASYNC: case QEMU_JOB_ASYNC_NESTED: /* async job was already handled above */ -- 1.7.6

--- src/qemu/qemu_domain.h | 1 - src/qemu/qemu_driver.c | 46 ++++++++++++++++++++++---------------------- src/qemu/qemu_migration.c | 6 +---- 3 files changed, 24 insertions(+), 29 deletions(-) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 71cefd9..3da7931 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -68,7 +68,6 @@ enum qemuDomainAsyncJob { enum qemuDomainJobSignals { QEMU_JOB_SIGNAL_CANCEL = 1 << 0, /* Request job cancellation */ - QEMU_JOB_SIGNAL_SUSPEND = 1 << 1, /* Request VM suspend to finish live migration offline */ }; struct qemuDomainJobObj { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d62e99d..8b186f7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1319,6 +1319,8 @@ static int qemudDomainSuspend(virDomainPtr dom) { int ret = -1; virDomainEventPtr event = NULL; qemuDomainObjPrivatePtr priv; + virDomainPausedReason reason; + int eventDetail; qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -1345,34 +1347,32 @@ static int qemudDomainSuspend(virDomainPtr dom) { priv = vm->privateData; if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT) { - if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_PAUSED) { - VIR_DEBUG("Requesting domain pause on %s", - vm->def->name); - priv->job.signals |= QEMU_JOB_SIGNAL_SUSPEND; - } - ret = 0; - goto cleanup; + reason = VIR_DOMAIN_PAUSED_MIGRATION; + eventDetail = VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED; } else { - if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_SUSPEND) < 0) - goto cleanup; + reason = VIR_DOMAIN_PAUSED_USER; + eventDetail = VIR_DOMAIN_EVENT_SUSPENDED_PAUSED; + } - if (!virDomainObjIsActive(vm)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); + if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_SUSPEND) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto endjob; + } + if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_PAUSED) { + if (qemuProcessStopCPUs(driver, vm, reason) < 0) { goto endjob; } - if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_PAUSED) { - if (qemuProcessStopCPUs(driver, vm, VIR_DOMAIN_PAUSED_USER) < 0) { - goto endjob; - } - event = virDomainEventNewFromObj(vm, - VIR_DOMAIN_EVENT_SUSPENDED, - VIR_DOMAIN_EVENT_SUSPENDED_PAUSED); - } - if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) - goto endjob; - ret = 0; + event = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_SUSPENDED, + eventDetail); } + if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) + goto endjob; + ret = 0; endjob: if (qemuDomainObjEndJob(driver, vm) == 0) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index ee2c5a0..de00811 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -770,11 +770,6 @@ qemuMigrationProcessJobSignals(struct qemud_driver *driver, if (ret < 0) { VIR_WARN("Unable to cancel job"); } - } else if (priv->job.signals & QEMU_JOB_SIGNAL_SUSPEND) { - priv->job.signals ^= QEMU_JOB_SIGNAL_SUSPEND; - VIR_DEBUG("Pausing domain for non-live migration"); - if (qemuMigrationSetOffline(driver, vm) < 0) - VIR_WARN("Unable to pause domain"); } else { ret = 0; } @@ -2801,6 +2796,7 @@ qemuMigrationJobStart(struct qemud_driver *driver, qemuDomainObjSetAsyncJobMask(vm, QEMU_JOB_NONE); } else { qemuDomainObjSetAsyncJobMask(vm, DEFAULT_JOB_MASK | + JOB_MASK(QEMU_JOB_SUSPEND) | JOB_MASK(QEMU_JOB_MIGRATION_OP)); } -- 1.7.6