[libvirt] [PATCH 12/13] Protect USB/PCI device list access in QEMU with dedicated locks
Daniel P. Berrange
berrange at redhat.com
Wed Jan 16 14:06:23 UTC 2013
From: "Daniel P. Berrange" <berrange at redhat.com>
Currently the activePciHostdevs, inactivePciHostdevsd and
activeUsbHostdevs lists are all implicitly protected by the
QEMU driver lock. Now that the lists all inherit from the
virObjectLockable, we can make the locking explicit, removing
the dependency on the QEMU driver lock for correctness.
Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
---
src/qemu/qemu_driver.c | 15 ++++++++++++-
src/qemu/qemu_hostdev.c | 57 ++++++++++++++++++++++++++++++++++++-------------
src/qemu/qemu_hotplug.c | 6 ++++++
3 files changed, 62 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 21e2dab..e1d1217 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10032,6 +10032,8 @@ qemuNodeDeviceDettach(virNodeDevicePtr dev)
return -1;
qemuDriverLock(driver);
+ virObjectLock(driver->activePciHostdevs);
+ virObjectLock(driver->inactivePciHostdevs);
in_inactive_list = virPCIDeviceListFind(driver->inactivePciHostdevs, pci);
if (virPCIDeviceDettach(pci, driver->activePciHostdevs,
@@ -10040,6 +10042,8 @@ qemuNodeDeviceDettach(virNodeDevicePtr dev)
ret = 0;
out:
+ virObjectUnlock(driver->inactivePciHostdevs);
+ virObjectUnlock(driver->activePciHostdevs);
qemuDriverUnlock(driver);
if (in_inactive_list)
virPCIDeviceFree(pci);
@@ -10062,6 +10066,9 @@ qemuNodeDeviceReAttach(virNodeDevicePtr dev)
if (!pci)
return -1;
+ qemuDriverLock(driver);
+ virObjectLock(driver->activePciHostdevs);
+ virObjectLock(driver->inactivePciHostdevs);
other = virPCIDeviceListFind(driver->activePciHostdevs, pci);
if (other) {
const char *other_name = virPCIDeviceGetUsedBy(other);
@@ -10074,17 +10081,19 @@ qemuNodeDeviceReAttach(virNodeDevicePtr dev)
virReportError(VIR_ERR_OPERATION_INVALID,
_("PCI device %s is still in use"),
virPCIDeviceGetName(pci));
+ goto out;
}
virPCIDeviceReAttachInit(pci);
- qemuDriverLock(driver);
if (virPCIDeviceReAttach(pci, driver->activePciHostdevs,
driver->inactivePciHostdevs, "pci-stub") < 0)
goto out;
ret = 0;
out:
+ virObjectUnlock(driver->inactivePciHostdevs);
+ virObjectUnlock(driver->activePciHostdevs);
qemuDriverUnlock(driver);
virPCIDeviceFree(pci);
return ret;
@@ -10106,6 +10115,8 @@ qemuNodeDeviceReset(virNodeDevicePtr dev)
return -1;
qemuDriverLock(driver);
+ virObjectLock(driver->activePciHostdevs);
+ virObjectLock(driver->inactivePciHostdevs);
if (virPCIDeviceReset(pci, driver->activePciHostdevs,
driver->inactivePciHostdevs) < 0)
@@ -10113,6 +10124,8 @@ qemuNodeDeviceReset(virNodeDevicePtr dev)
ret = 0;
out:
+ virObjectUnlock(driver->inactivePciHostdevs);
+ virObjectUnlock(driver->activePciHostdevs);
qemuDriverUnlock(driver);
virPCIDeviceFree(pci);
return ret;
diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index 5f14d02..97d78bf 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -72,6 +72,9 @@ qemuGetPciHostDeviceList(virDomainHostdevDefPtr *hostdevs, int nhostdevs)
return list;
}
+/*
+ * Pre-condition: driver->activePciHostdevs is locked
+ */
static virPCIDeviceListPtr
qemuGetActivePciHostDeviceList(virQEMUDriverPtr driver,
virDomainHostdevDefPtr *hostdevs,
@@ -121,10 +124,14 @@ int qemuUpdateActivePciHostdevs(virQEMUDriverPtr driver,
{
virDomainHostdevDefPtr hostdev = NULL;
int i;
+ int ret = -1;
if (!def->nhostdevs)
return 0;
+ virObjectLock(driver->activePciHostdevs);
+ virObjectLock(driver->inactivePciHostdevs);
+
for (i = 0; i < def->nhostdevs; i++) {
virPCIDevicePtr dev = NULL;
hostdev = def->hostdevs[i];
@@ -140,7 +147,7 @@ int qemuUpdateActivePciHostdevs(virQEMUDriverPtr driver,
hostdev->source.subsys.u.pci.function);
if (!dev)
- return -1;
+ goto cleanup;
virPCIDeviceSetManaged(dev, hostdev->managed);
virPCIDeviceSetUsedBy(dev, def->name);
@@ -152,11 +159,14 @@ int qemuUpdateActivePciHostdevs(virQEMUDriverPtr driver,
if (virPCIDeviceListAdd(driver->activePciHostdevs, dev) < 0) {
virPCIDeviceFree(dev);
- return -1;
+ goto cleanup;
}
}
- return 0;
+cleanup:
+ virObjectUnlock(driver->activePciHostdevs);
+ virObjectUnlock(driver->inactivePciHostdevs);
+ return ret;
}
int
@@ -165,10 +175,12 @@ qemuUpdateActiveUsbHostdevs(virQEMUDriverPtr driver,
{
virDomainHostdevDefPtr hostdev = NULL;
int i;
+ int ret = -1;
if (!def->nhostdevs)
return 0;
+ virObjectLock(driver->activeUsbHostdevs);
for (i = 0; i < def->nhostdevs; i++) {
virUSBDevicePtr usb = NULL;
hostdev = def->hostdevs[i];
@@ -193,11 +205,13 @@ qemuUpdateActiveUsbHostdevs(virQEMUDriverPtr driver,
if (virUSBDeviceListAdd(driver->activeUsbHostdevs, usb) < 0) {
virUSBDeviceFree(usb);
- return -1;
+ goto cleanup;
}
}
-
- return 0;
+ ret = 0;
+cleanup:
+ virObjectLock(driver->activeUsbHostdevs);
+ return ret;
}
static int
@@ -412,6 +426,9 @@ int qemuPrepareHostdevPCIDevices(virQEMUDriverPtr driver,
int ret = -1;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+ virObjectLock(driver->activePciHostdevs);
+ virObjectLock(driver->inactivePciHostdevs);
+
if (!(pcidevs = qemuGetPciHostDeviceList(hostdevs, nhostdevs)))
goto cleanup;
@@ -579,19 +596,13 @@ reattachdevs:
}
cleanup:
+ virObjectUnlock(driver->activePciHostdevs);
+ virObjectUnlock(driver->inactivePciHostdevs);
virObjectUnref(pcidevs);
virObjectUnref(cfg);
return ret;
}
-static int
-qemuPrepareHostPCIDevices(virQEMUDriverPtr driver,
- virDomainDefPtr def)
-{
- return qemuPrepareHostdevPCIDevices(driver, def->name, def->uuid,
- def->hostdevs, def->nhostdevs);
-}
-
int
qemuPrepareHostdevUSBDevices(virQEMUDriverPtr driver,
const char *name,
@@ -601,6 +612,7 @@ qemuPrepareHostdevUSBDevices(virQEMUDriverPtr driver,
unsigned int count;
virUSBDevicePtr tmp;
+ virObjectLock(driver->activeUsbHostdevs);
count = virUSBDeviceListCount(list);
for (i = 0; i < count; i++) {
@@ -630,6 +642,8 @@ qemuPrepareHostdevUSBDevices(virQEMUDriverPtr driver,
if (virUSBDeviceListAdd(driver->activeUsbHostdevs, usb) < 0)
goto error;
}
+
+ virObjectUnlock(driver->activeUsbHostdevs);
return 0;
error:
@@ -637,6 +651,7 @@ error:
tmp = virUSBDeviceListGet(list, i);
virUSBDeviceListSteal(driver->activeUsbHostdevs, tmp);
}
+ virObjectUnlock(driver->activeUsbHostdevs);
return -1;
}
@@ -802,7 +817,8 @@ int qemuPrepareHostDevices(virQEMUDriverPtr driver,
if (!def->nhostdevs)
return 0;
- if (qemuPrepareHostPCIDevices(driver, def) < 0)
+ if (qemuPrepareHostdevPCIDevices(driver, def->name, def->uuid,
+ def->hostdevs, def->nhostdevs) < 0)
return -1;
if (qemuPrepareHostUSBDevices(driver, def, coldBoot) < 0)
@@ -812,6 +828,10 @@ int qemuPrepareHostDevices(virQEMUDriverPtr driver,
}
+/*
+ * Pre-condition: driver->inactivePciHostdevs & driver->activePciHostdevs
+ * are locked
+ */
void qemuReattachPciDevice(virPCIDevicePtr dev, virQEMUDriverPtr driver)
{
int retries = 100;
@@ -851,6 +871,9 @@ void qemuDomainReAttachHostdevDevices(virQEMUDriverPtr driver,
int i;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+ virObjectLock(driver->activePciHostdevs);
+ virObjectLock(driver->inactivePciHostdevs);
+
if (!(pcidevs = qemuGetActivePciHostDeviceList(driver,
hostdevs,
nhostdevs))) {
@@ -917,6 +940,8 @@ void qemuDomainReAttachHostdevDevices(virQEMUDriverPtr driver,
virObjectUnref(pcidevs);
cleanup:
+ virObjectUnlock(driver->activePciHostdevs);
+ virObjectUnlock(driver->inactivePciHostdevs);
virObjectUnref(cfg);
}
@@ -928,6 +953,7 @@ qemuDomainReAttachHostUsbDevices(virQEMUDriverPtr driver,
{
int i;
+ virObjectLock(driver->activeUsbHostdevs);
for (i = 0; i < nhostdevs; i++) {
virDomainHostdevDefPtr hostdev = hostdevs[i];
virUSBDevicePtr usb, tmp;
@@ -979,6 +1005,7 @@ qemuDomainReAttachHostUsbDevices(virQEMUDriverPtr driver,
virUSBDeviceListDel(driver->activeUsbHostdevs, tmp);
}
}
+ virObjectUnlock(driver->activeUsbHostdevs);
}
void qemuDomainReAttachHostDevices(virQEMUDriverPtr driver,
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index bf71eba..ad91e0d 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2344,6 +2344,8 @@ qemuDomainDetachHostPciDevice(virQEMUDriverPtr driver,
if (detach->parent.data.net)
qemuDomainHostdevNetConfigRestore(detach, cfg->stateDir);
+ virObjectLock(driver->activePciHostdevs);
+ virObjectLock(driver->inactivePciHostdevs);
pci = virPCIDeviceNew(subsys->u.pci.domain, subsys->u.pci.bus,
subsys->u.pci.slot, subsys->u.pci.function);
if (pci) {
@@ -2359,6 +2361,8 @@ qemuDomainDetachHostPciDevice(virQEMUDriverPtr driver,
}
virPCIDeviceFree(pci);
}
+ virObjectUnlock(driver->activePciHostdevs);
+ virObjectUnlock(driver->inactivePciHostdevs);
if (qemuCapsGet(priv->caps, QEMU_CAPS_DEVICE) &&
qemuDomainPCIAddressReleaseSlot(priv->pciaddrs,
@@ -2401,7 +2405,9 @@ qemuDomainDetachHostUsbDevice(virQEMUDriverPtr driver,
usb = virUSBDeviceNew(subsys->u.usb.bus, subsys->u.usb.device, NULL);
if (usb) {
+ virObjectLock(driver->activeUsbHostdevs);
virUSBDeviceListDel(driver->activeUsbHostdevs, usb);
+ virObjectUnlock(driver->activeUsbHostdevs);
virUSBDeviceFree(usb);
} else {
VIR_WARN("Unable to find device %03d.%03d in list of used USB devices",
--
1.8.0.2
More information about the libvir-list
mailing list