1
0

qemu: don't 'remove' hostdev objects from domain if operation fails

There were certain paths through the hostdev detach code that could
lead to the lower level function failing (and not removing the object
from the domain's hostdevs list), but the higher level function
free'ing the hostdev object anyway. This would leave a stale
hostdevdef pointer in the list, which would surely cause a problem
eventually.

This patch relocates virDomainHostdevRemove from the lower level
functions qemuDomainDetachThisHostDevice and
qemuDomainDetachHostPciDevice, to their caller
qemuDomainDetachThisHostDevice, placing it just before the call to
virDomainHostdevDefFree. This makes it easy to verify that either both
operations are done, or neither.

NB: The "dangling pointer" part of this problem was introduced in
commit 13d5a6, so it is not present in libvirt versions prior to
0.9.9. Earlier versions would return failure in certain cases even
though the the device object was removed/deleted, but the removal and
deletion operations would always both happen or neither.
This commit is contained in:
Laine Stump 2012-03-06 20:43:22 -05:00
parent 8845d29375
commit b59e59845f

View File

@ -1852,8 +1852,7 @@ cleanup:
static int static int
qemuDomainDetachHostPciDevice(struct qemud_driver *driver, qemuDomainDetachHostPciDevice(struct qemud_driver *driver,
virDomainObjPtr vm, virDomainObjPtr vm,
virDomainHostdevDefPtr detach, virDomainHostdevDefPtr detach)
int idx)
{ {
qemuDomainObjPrivatePtr priv = vm->privateData; qemuDomainObjPrivatePtr priv = vm->privateData;
virDomainHostdevSubsysPtr subsys = &detach->source.subsys; virDomainHostdevSubsysPtr subsys = &detach->source.subsys;
@ -1914,15 +1913,13 @@ qemuDomainDetachHostPciDevice(struct qemud_driver *driver,
detach->info->addr.pci.slot) < 0) detach->info->addr.pci.slot) < 0)
VIR_WARN("Unable to release PCI address on host device"); VIR_WARN("Unable to release PCI address on host device");
virDomainHostdevRemove(vm->def, idx);
return ret; return ret;
} }
static int static int
qemuDomainDetachHostUsbDevice(struct qemud_driver *driver, qemuDomainDetachHostUsbDevice(struct qemud_driver *driver,
virDomainObjPtr vm, virDomainObjPtr vm,
virDomainHostdevDefPtr detach, virDomainHostdevDefPtr detach)
int idx)
{ {
qemuDomainObjPrivatePtr priv = vm->privateData; qemuDomainObjPrivatePtr priv = vm->privateData;
virDomainHostdevSubsysPtr subsys = &detach->source.subsys; virDomainHostdevSubsysPtr subsys = &detach->source.subsys;
@ -1956,8 +1953,6 @@ qemuDomainDetachHostUsbDevice(struct qemud_driver *driver,
VIR_WARN("Unable to find device %03d.%03d in list of used USB devices", VIR_WARN("Unable to find device %03d.%03d in list of used USB devices",
subsys->u.usb.bus, subsys->u.usb.device); subsys->u.usb.bus, subsys->u.usb.device);
} }
virDomainHostdevRemove(vm->def, idx);
return ret; return ret;
} }
@ -1987,10 +1982,10 @@ int qemuDomainDetachThisHostDevice(struct qemud_driver *driver,
switch (detach->source.subsys.type) { switch (detach->source.subsys.type) {
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
ret = qemuDomainDetachHostPciDevice(driver, vm, detach, idx); ret = qemuDomainDetachHostPciDevice(driver, vm, detach);
break; break;
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
ret = qemuDomainDetachHostUsbDevice(driver, vm, detach, idx); ret = qemuDomainDetachHostUsbDevice(driver, vm, detach);
break; break;
default: default:
qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@ -1999,13 +1994,14 @@ int qemuDomainDetachThisHostDevice(struct qemud_driver *driver,
return -1; return -1;
} }
if (ret == 0 && if (!ret) {
virSecurityManagerRestoreHostdevLabel(driver->securityManager, if (virSecurityManagerRestoreHostdevLabel(driver->securityManager,
vm->def, detach) < 0) { vm->def, detach) < 0) {
VIR_WARN("Failed to restore host device labelling"); VIR_WARN("Failed to restore host device labelling");
}
virDomainHostdevRemove(vm->def, idx);
virDomainHostdevDefFree(detach);
} }
virDomainHostdevDefFree(detach);
return ret; return ret;
} }