[libvirt] [PATCH 7/8] Simplify PCI hostdev prepare/re-attach using a pciDeviceList type
Mark McLoughlin
markmc at redhat.com
Mon Aug 17 14:10:20 UTC 2009
The qemuPrepareHostDevices() and qemuDomainReAttachHostDevices()
functions are clutter with a bunch of calls to pciGetDevice() and
pciFreeDevice() obscuring the basic logic.
Add a pciDeviceList type and add a qemuGetPciHostDeviceList() function
to build a list from a domain definition. Use this in prepare/re-attach
fto simplify things and eliminate the multiple pciGetDevice calls.
This is especially useful because in the next patch we need to iterate
the hostdevs list a third time and we also need a list type for keeping
track of active devices.
* src/pci.[ch]: add pciDeviceList type and also a per-device 'managed'
property
* src/libvirt_private.syms: export the new functions
* src/qemu_driver.c: add qemuGetPciHostDeviceList() and re-write
qemuPrepareHostDevices() and qemuDomainReAttachHostDevices() to use it
---
src/libvirt_private.syms | 7 ++-
src/pci.c | 109 ++++++++++++++++++++++++++++++
src/pci.h | 20 ++++++
src/qemu_driver.c | 167 +++++++++++++++++++--------------------------
4 files changed, 206 insertions(+), 97 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 642c2bc..2bf4e15 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -285,7 +285,12 @@ pciFreeDevice;
pciDettachDevice;
pciReAttachDevice;
pciResetDevice;
-
+pciDeviceSetManaged;
+pciDeviceGetManaged;
+pciDeviceListNew;
+pciDeviceListFree;
+pciDeviceListAdd;
+pciDeviceListDel;
# qparams.h
qparam_get_query;
diff --git a/src/pci.c b/src/pci.c
index 74f7ef0..a37eaf7 100644
--- a/src/pci.c
+++ b/src/pci.c
@@ -63,6 +63,7 @@ struct _pciDevice {
unsigned pci_pm_cap_pos;
unsigned has_flr : 1;
unsigned has_pm_reset : 1;
+ unsigned managed : 1;
};
/* For virReportOOMError() and virReportSystemError() */
@@ -890,8 +891,116 @@ pciGetDevice(virConnectPtr conn,
void
pciFreeDevice(virConnectPtr conn ATTRIBUTE_UNUSED, pciDevice *dev)
{
+ if (!dev)
+ return;
VIR_DEBUG("%s %s: freeing", dev->id, dev->name);
if (dev->fd >= 0)
close(dev->fd);
VIR_FREE(dev);
}
+
+void pciDeviceSetManaged(pciDevice *dev, unsigned managed)
+{
+ dev->managed = !!managed;
+}
+
+unsigned pciDeviceGetManaged(pciDevice *dev)
+{
+ return dev->managed;
+}
+
+pciDeviceList *
+pciDeviceListNew(virConnectPtr conn)
+{
+ pciDeviceList *list;
+
+ if (VIR_ALLOC(list) < 0) {
+ virReportOOMError(conn);
+ return NULL;
+ }
+
+ return list;
+}
+
+void
+pciDeviceListFree(virConnectPtr conn,
+ pciDeviceList *list)
+{
+ int i;
+
+ if (!list)
+ return;
+
+ for (i = 0; i < list->count; i++) {
+ pciFreeDevice(conn, list->devs[i]);
+ list->devs[i] = NULL;
+ }
+
+ list->count = 0;
+ VIR_FREE(list->devs);
+ VIR_FREE(list);
+}
+
+int
+pciDeviceListAdd(virConnectPtr conn,
+ pciDeviceList *list,
+ pciDevice *dev)
+{
+ if (pciDeviceListFind(list, dev)) {
+ pciReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _("Device %s is already in use"), dev->name);
+ return -1;
+ }
+
+ if (VIR_REALLOC_N(list->devs, list->count+1) < 0) {
+ virReportOOMError(conn);
+ return -1;
+ }
+
+ list->devs[list->count++] = dev;
+
+ return 0;
+}
+
+void
+pciDeviceListDel(virConnectPtr conn ATTRIBUTE_UNUSED,
+ pciDeviceList *list,
+ pciDevice *dev)
+{
+ int i;
+
+ for (i = 0; i < list->count; i++) {
+ if (list->devs[i]->domain != dev->domain ||
+ list->devs[i]->bus != dev->bus ||
+ list->devs[i]->slot != dev->slot ||
+ list->devs[i]->function != dev->function)
+ continue;
+
+ pciFreeDevice(conn, list->devs[i]);
+
+ if (i != --list->count)
+ memmove(&list->devs[i],
+ &list->devs[i+1],
+ sizeof(*list->devs) * (list->count-i));
+
+ if (VIR_REALLOC_N(list->devs, list->count) < 0) {
+ ; /* not fatal */
+ }
+
+ break;
+ }
+}
+
+pciDevice *
+pciDeviceListFind(pciDeviceList *list, pciDevice *dev)
+{
+ int i;
+
+ for (i = 0; i < list->count; i++)
+ if (list->devs[i]->domain == dev->domain &&
+ list->devs[i]->bus == dev->bus &&
+ list->devs[i]->slot == dev->slot &&
+ list->devs[i]->function == dev->function)
+ return list->devs[i];
+ return NULL;
+}
diff --git a/src/pci.h b/src/pci.h
index 47882ef..75fbd51 100644
--- a/src/pci.h
+++ b/src/pci.h
@@ -27,6 +27,11 @@
typedef struct _pciDevice pciDevice;
+typedef struct {
+ unsigned count;
+ pciDevice **devs;
+} pciDeviceList;
+
pciDevice *pciGetDevice (virConnectPtr conn,
unsigned domain,
unsigned bus,
@@ -40,5 +45,20 @@ int pciReAttachDevice (virConnectPtr conn,
pciDevice *dev);
int pciResetDevice (virConnectPtr conn,
pciDevice *dev);
+void pciDeviceSetManaged(pciDevice *dev,
+ unsigned managed);
+unsigned pciDeviceGetManaged(pciDevice *dev);
+
+pciDeviceList *pciDeviceListNew (virConnectPtr conn);
+void pciDeviceListFree (virConnectPtr conn,
+ pciDeviceList *list);
+int pciDeviceListAdd (virConnectPtr conn,
+ pciDeviceList *list,
+ pciDevice *dev);
+void pciDeviceListDel (virConnectPtr conn,
+ pciDeviceList *list,
+ pciDevice *dev);
+pciDevice * pciDeviceListFind (pciDeviceList *list,
+ pciDevice *dev);
#endif /* __VIR_PCI_H__ */
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index a9da387..f2b807b 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -1329,48 +1329,16 @@ static int qemudNextFreeVNCPort(struct qemud_driver *driver ATTRIBUTE_UNUSED) {
return -1;
}
-static int qemuPrepareHostDevices(virConnectPtr conn,
- virDomainDefPtr def) {
+static pciDeviceList *
+qemuGetPciHostDeviceList(virConnectPtr conn,
+ virDomainDefPtr def)
+{
+ pciDeviceList *list;
int i;
- /* We have to use 2 loops here. *All* devices must
- * be detached before we reset any of them, because
- * in some cases you have to reset the whole PCI,
- * which impacts all devices on it
- */
-
- for (i = 0 ; i < def->nhostdevs ; i++) {
- virDomainHostdevDefPtr hostdev = def->hostdevs[i];
-
- if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
- continue;
- if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
- continue;
-
- if (hostdev->managed) {
- pciDevice *dev = pciGetDevice(conn,
- hostdev->source.subsys.u.pci.domain,
- hostdev->source.subsys.u.pci.bus,
- hostdev->source.subsys.u.pci.slot,
- hostdev->source.subsys.u.pci.function);
- if (!dev)
- goto error;
-
- if (pciDettachDevice(conn, dev) < 0) {
- pciFreeDevice(conn, dev);
- goto error;
- }
-
- pciFreeDevice(conn, dev);
- } /* else {
- XXX validate that non-managed device isn't in use, eg
- by checking that device is either un-bound, or bound
- to pci-stub.ko
- } */
- }
+ if (!(list = pciDeviceListNew(conn)))
+ return NULL;
- /* Now that all the PCI hostdevs have be dettached, we can safely
- * reset them */
for (i = 0 ; i < def->nhostdevs ; i++) {
virDomainHostdevDefPtr hostdev = def->hostdevs[i];
pciDevice *dev;
@@ -1385,95 +1353,102 @@ static int qemuPrepareHostDevices(virConnectPtr conn,
hostdev->source.subsys.u.pci.bus,
hostdev->source.subsys.u.pci.slot,
hostdev->source.subsys.u.pci.function);
- if (!dev)
- goto error;
+ if (!dev) {
+ pciDeviceListFree(conn, list);
+ return NULL;
+ }
- if (pciResetDevice(conn, dev) < 0) {
+ if (pciDeviceListAdd(conn, list, dev) < 0) {
pciFreeDevice(conn, dev);
- goto error;
+ pciDeviceListFree(conn, list);
+ return NULL;
}
- pciFreeDevice(conn, dev);
+ pciDeviceSetManaged(dev, hostdev->managed);
}
+ return list;
+}
+
+static int
+qemuPrepareHostDevices(virConnectPtr conn, virDomainDefPtr def)
+{
+ pciDeviceList *pcidevs;
+ int i;
+
+ if (!def->nhostdevs)
+ return 0;
+
+ if (!(pcidevs = qemuGetPciHostDeviceList(conn, def)))
+ return -1;
+
+ /* We have to use 2 loops here. *All* devices must
+ * be detached before we reset any of them, because
+ * in some cases you have to reset the whole PCI,
+ * which impacts all devices on it
+ */
+
+ /* XXX validate that non-managed device isn't in use, eg
+ * by checking that device is either un-bound, or bound
+ * to pci-stub.ko
+ */
+
+ for (i = 0; i < pcidevs->count; i++)
+ if (pciDeviceGetManaged(pcidevs->devs[i]) &&
+ pciDettachDevice(conn, pcidevs->devs[i]) < 0)
+ goto error;
+
+ /* Now that all the PCI hostdevs have be dettached, we can safely
+ * reset them */
+ for (i = 0; i < pcidevs->count; i++)
+ if (pciResetDevice(conn, pcidevs->devs[i]) < 0)
+ goto error;
+
+ pciDeviceListFree(conn, pcidevs);
return 0;
error:
+ pciDeviceListFree(conn, pcidevs);
return -1;
}
static void
qemuDomainReAttachHostDevices(virConnectPtr conn, virDomainDefPtr def)
{
+ pciDeviceList *pcidevs;
int i;
- /* Again 2 loops; reset all the devices before re-attach */
-
- for (i = 0 ; i < def->nhostdevs ; i++) {
- virDomainHostdevDefPtr hostdev = def->hostdevs[i];
- pciDevice *dev;
-
- if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
- continue;
- if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
- continue;
-
- dev = pciGetDevice(conn,
- hostdev->source.subsys.u.pci.domain,
- hostdev->source.subsys.u.pci.bus,
- hostdev->source.subsys.u.pci.slot,
- hostdev->source.subsys.u.pci.function);
- if (!dev) {
- virErrorPtr err = virGetLastError();
- VIR_ERROR(_("Failed to allocate pciDevice: %s\n"),
- err ? err->message : "");
- virResetError(err);
- continue;
- }
-
- if (pciResetDevice(conn, dev) < 0) {
- virErrorPtr err = virGetLastError();
- VIR_ERROR(_("Failed to reset PCI device: %s\n"),
- err ? err->message : "");
- virResetError(err);
- }
+ if (!def->nhostdevs)
+ return;
- pciFreeDevice(conn, dev);
+ if (!(pcidevs = qemuGetPciHostDeviceList(conn, def))) {
+ virErrorPtr err = virGetLastError();
+ VIR_ERROR(_("Failed to allocate pciDeviceList: %s\n"),
+ err ? err->message : "");
+ virResetError(err);
+ return;
}
- for (i = 0 ; i < def->nhostdevs ; i++) {
- virDomainHostdevDefPtr hostdev = def->hostdevs[i];
- pciDevice *dev;
-
- if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
- continue;
- if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
- continue;
- if (!hostdev->managed)
- continue;
+ /* Again 2 loops; reset all the devices before re-attach */
- dev = pciGetDevice(conn,
- hostdev->source.subsys.u.pci.domain,
- hostdev->source.subsys.u.pci.bus,
- hostdev->source.subsys.u.pci.slot,
- hostdev->source.subsys.u.pci.function);
- if (!dev) {
+ for (i = 0; i < pcidevs->count; i++)
+ if (pciResetDevice(conn, pcidevs->devs[i]) < 0) {
virErrorPtr err = virGetLastError();
- VIR_ERROR(_("Failed to allocate pciDevice: %s\n"),
+ VIR_ERROR(_("Failed to reset PCI device: %s\n"),
err ? err->message : "");
virResetError(err);
- continue;
}
- if (pciReAttachDevice(conn, dev) < 0) {
+ for (i = 0; i < pcidevs->count; i++)
+ if (pciDeviceGetManaged(pcidevs->devs[i]) &&
+ pciReAttachDevice(conn, pcidevs->devs[i]) < 0) {
virErrorPtr err = virGetLastError();
VIR_ERROR(_("Failed to re-attach PCI device: %s\n"),
err ? err->message : "");
virResetError(err);
}
- pciFreeDevice(conn, dev);
- }
+ pciDeviceListFree(conn, pcidevs);
}
static const char *const defaultDeviceACL[] = {
--
1.6.2.5
More information about the libvir-list
mailing list