qemu: check for vm after starting a job

https://bugzilla.redhat.com/show_bug.cgi?id=638285 - when migrating
a guest, it was very easy to provoke a race where an application
could query block information on a VM that had just been migrated
away.  Any time qemu code obtains a job lock, it must also check
that the VM was not taken down in the time where it was waiting
for the lock.

* src/qemu/qemu_driver.c (qemudDomainSetMemory)
(qemudDomainGetInfo, qemuDomainGetBlockInfo): Check that vm still
exists after obtaining job lock, before starting monitor action.
This commit is contained in:
Eric Blake 2010-10-26 09:31:19 -06:00
parent 0111cebb5a
commit 054d43f570

View File

@ -442,7 +442,7 @@ static int qemuDomainObjBeginJobWithDriver(struct qemud_driver *driver,
* obj must be locked before calling, qemud_driver does not matter * obj must be locked before calling, qemud_driver does not matter
* *
* To be called after completing the work associated with the * To be called after completing the work associated with the
* earlier qemuDomainBeginJob() call * earlier qemuDomainBeginJob() call
* *
* Returns remaining refcount on 'obj', maybe 0 to indicated it * Returns remaining refcount on 'obj', maybe 0 to indicated it
* was deleted * was deleted
@ -466,7 +466,8 @@ static int ATTRIBUTE_RETURN_CHECK qemuDomainObjEndJob(virDomainObjPtr obj)
* obj must be locked before calling, qemud_driver must be unlocked * obj must be locked before calling, qemud_driver must be unlocked
* *
* To be called immediately before any QEMU monitor API call * To be called immediately before any QEMU monitor API call
* Must have alrady called qemuDomainObjBeginJob(). * Must have already called qemuDomainObjBeginJob(), and checked
* that the VM is still active.
* *
* To be followed with qemuDomainObjExitMonitor() once complete * To be followed with qemuDomainObjExitMonitor() once complete
*/ */
@ -482,7 +483,7 @@ static void qemuDomainObjEnterMonitor(virDomainObjPtr obj)
/* obj must NOT be locked before calling, qemud_driver must be unlocked /* obj must NOT be locked before calling, qemud_driver must be unlocked
* *
* Should be paired with an earlier qemuDomainObjEnterMonitor() call * Should be paired with an earlier qemuDomainObjEnterMonitor() call
*/ */
static void qemuDomainObjExitMonitor(virDomainObjPtr obj) static void qemuDomainObjExitMonitor(virDomainObjPtr obj)
{ {
@ -507,7 +508,7 @@ static void qemuDomainObjExitMonitor(virDomainObjPtr obj)
* obj must be locked before calling, qemud_driver must be locked * obj must be locked before calling, qemud_driver must be locked
* *
* To be called immediately before any QEMU monitor API call * To be called immediately before any QEMU monitor API call
* Must have alrady called qemuDomainObjBeginJob(). * Must have already called qemuDomainObjBeginJob().
* *
* To be followed with qemuDomainObjExitMonitorWithDriver() once complete * To be followed with qemuDomainObjExitMonitorWithDriver() once complete
*/ */
@ -525,7 +526,7 @@ static void qemuDomainObjEnterMonitorWithDriver(struct qemud_driver *driver, vir
/* obj must NOT be locked before calling, qemud_driver must be unlocked, /* obj must NOT be locked before calling, qemud_driver must be unlocked,
* and will be locked after returning * and will be locked after returning
* *
* Should be paired with an earlier qemuDomainObjEnterMonitor() call * Should be paired with an earlier qemuDomainObjEnterMonitorWithDriver() call
*/ */
static void qemuDomainObjExitMonitorWithDriver(struct qemud_driver *driver, virDomainObjPtr obj) static void qemuDomainObjExitMonitorWithDriver(struct qemud_driver *driver, virDomainObjPtr obj)
{ {
@ -5077,12 +5078,6 @@ static int qemudDomainSetMemory(virDomainPtr dom, unsigned long newmem) {
goto cleanup; goto cleanup;
} }
if (!virDomainObjIsActive(vm)) {
qemuReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("domain is not running"));
goto cleanup;
}
if (newmem > vm->def->mem.max_balloon) { if (newmem > vm->def->mem.max_balloon) {
qemuReportError(VIR_ERR_INVALID_ARG, qemuReportError(VIR_ERR_INVALID_ARG,
"%s", _("cannot set memory higher than max memory")); "%s", _("cannot set memory higher than max memory"));
@ -5092,6 +5087,12 @@ static int qemudDomainSetMemory(virDomainPtr dom, unsigned long newmem) {
if (qemuDomainObjBeginJob(vm) < 0) if (qemuDomainObjBeginJob(vm) < 0)
goto cleanup; goto cleanup;
if (!virDomainObjIsActive(vm)) {
qemuReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("domain is not running"));
goto endjob;
}
priv = vm->privateData; priv = vm->privateData;
qemuDomainObjEnterMonitor(vm); qemuDomainObjEnterMonitor(vm);
r = qemuMonitorSetBalloon(priv->mon, newmem); r = qemuMonitorSetBalloon(priv->mon, newmem);
@ -5158,26 +5159,25 @@ static int qemudDomainGetInfo(virDomainPtr dom,
} else if (!priv->jobActive) { } else if (!priv->jobActive) {
if (qemuDomainObjBeginJob(vm) < 0) if (qemuDomainObjBeginJob(vm) < 0)
goto cleanup; goto cleanup;
if (!virDomainObjIsActive(vm))
qemuDomainObjEnterMonitor(vm); err = 0;
err = qemuMonitorGetBalloonInfo(priv->mon, &balloon); else {
qemuDomainObjExitMonitor(vm); qemuDomainObjEnterMonitor(vm);
if (err < 0) { err = qemuMonitorGetBalloonInfo(priv->mon, &balloon);
if (qemuDomainObjEndJob(vm) == 0) qemuDomainObjExitMonitor(vm);
vm = NULL; }
if (qemuDomainObjEndJob(vm) == 0) {
vm = NULL;
goto cleanup; goto cleanup;
} }
if (err < 0)
goto cleanup;
if (err == 0) if (err == 0)
/* Balloon not supported, so maxmem is always the allocation */ /* Balloon not supported, so maxmem is always the allocation */
info->memory = vm->def->mem.max_balloon; info->memory = vm->def->mem.max_balloon;
else else
info->memory = balloon; info->memory = balloon;
if (qemuDomainObjEndJob(vm) == 0) {
vm = NULL;
goto cleanup;
}
} else { } else {
info->memory = vm->def->mem.cur_balloon; info->memory = vm->def->mem.cur_balloon;
} }
@ -10510,19 +10510,21 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom,
/* ..but if guest is running & not using raw /* ..but if guest is running & not using raw
disk format and on a block device, then query disk format and on a block device, then query
highest allocated extent from QEMU */ highest allocated extent from QEMU */
if (virDomainObjIsActive(vm) && if (disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK &&
disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK &&
format != VIR_STORAGE_FILE_RAW && format != VIR_STORAGE_FILE_RAW &&
S_ISBLK(sb.st_mode)) { S_ISBLK(sb.st_mode)) {
qemuDomainObjPrivatePtr priv = vm->privateData; qemuDomainObjPrivatePtr priv = vm->privateData;
if (qemuDomainObjBeginJob(vm) < 0) if (qemuDomainObjBeginJob(vm) < 0)
goto cleanup; goto cleanup;
if (!virDomainObjIsActive(vm))
qemuDomainObjEnterMonitor(vm); ret = 0;
ret = qemuMonitorGetBlockExtent(priv->mon, else {
disk->info.alias, qemuDomainObjEnterMonitor(vm);
&info->allocation); ret = qemuMonitorGetBlockExtent(priv->mon,
qemuDomainObjExitMonitor(vm); disk->info.alias,
&info->allocation);
qemuDomainObjExitMonitor(vm);
}
if (qemuDomainObjEndJob(vm) == 0) if (qemuDomainObjEndJob(vm) == 0)
vm = NULL; vm = NULL;