Convert virPCIDeviceList and virUSBDeviceList into virObjectLockable

To allow modifications to the lists to be synchronized, convert
virPCIDeviceList and virUSBDeviceList into virObjectLockable
classes. The locking, however, will not be self-contained. The
users of these classes will have to call virObjectLock/Unlock
in the critical regions.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

src/libvirt_private.syms

@@ -1008,7 +1008,6 @@ virPCIDeviceListCount;
 virPCIDeviceListDel;
 virPCIDeviceListFind;
 virPCIDeviceListFindIndex;
-virPCIDeviceListFree;
 virPCIDeviceListGet;
 virPCIDeviceListNew;
 virPCIDeviceListSteal;
@@ -1234,7 +1233,6 @@ virUSBDeviceListAdd;
 virUSBDeviceListCount;
 virUSBDeviceListDel;
 virUSBDeviceListFind;
-virUSBDeviceListFree;
 virUSBDeviceListGet;
 virUSBDeviceListNew;
 virUSBDeviceListSteal;

src/lxc/lxc_hostdev.c

@@ -167,7 +167,7 @@ virLXCFindHostdevUSBDevice(virDomainHostdevDefPtr hostdev,
             *usb = virUSBDeviceListGet(devs, 0);
             virUSBDeviceListSteal(devs, *usb);
         }
-        virUSBDeviceListFree(devs);
+        virObjectUnref(devs);
 
         if (rc == 0) {
             goto out;
@@ -273,7 +273,7 @@ virLXCPrepareHostUSBDevices(virLXCDriverPtr driver,
     ret = 0;
 
 cleanup:
-    virUSBDeviceListFree(list);
+    virObjectUnref(list);
     return ret;
 }
 

src/qemu/qemu_driver.c

@@ -1047,9 +1047,9 @@ qemuShutdown(void) {
 
     qemuDriverLock(qemu_driver);
     virNWFilterUnRegisterCallbackDriver(&qemuCallbackDriver);
-    virPCIDeviceListFree(qemu_driver->activePciHostdevs);
+    virObjectUnref(qemu_driver->activePciHostdevs);
-    virPCIDeviceListFree(qemu_driver->inactivePciHostdevs);
+    virObjectUnref(qemu_driver->inactivePciHostdevs);
-    virUSBDeviceListFree(qemu_driver->activeUsbHostdevs);
+    virObjectUnref(qemu_driver->activeUsbHostdevs);
     virHashFree(qemu_driver->sharedDisks);
     virCapabilitiesFree(qemu_driver->caps);
     qemuCapsCacheFree(qemu_driver->capsCache);

src/qemu/qemu_hostdev.c

@@ -56,13 +56,13 @@ qemuGetPciHostDeviceList(virDomainHostdevDefPtr *hostdevs, int nhostdevs)
                               hostdev->source.subsys.u.pci.slot,
                               hostdev->source.subsys.u.pci.function);
         if (!dev) {
-            virPCIDeviceListFree(list);
+            virObjectUnref(list);
             return NULL;
         }
 
         if (virPCIDeviceListAdd(list, dev) < 0) {
             virPCIDeviceFree(dev);
-            virPCIDeviceListFree(list);
+            virObjectUnref(list);
             return NULL;
         }
 
@@ -98,14 +98,14 @@ qemuGetActivePciHostDeviceList(virQEMUDriverPtr driver,
                               hostdev->source.subsys.u.pci.slot,
                               hostdev->source.subsys.u.pci.function);
         if (!dev) {
-            virPCIDeviceListFree(list);
+            virObjectUnref(list);
             return NULL;
         }
 
         if ((activeDev = virPCIDeviceListFind(driver->activePciHostdevs, dev))) {
             if (virPCIDeviceListAdd(list, activeDev) < 0) {
                 virPCIDeviceFree(dev);
-                virPCIDeviceListFree(list);
+                virObjectUnref(list);
                 return NULL;
             }
         }
@@ -557,7 +557,7 @@ int qemuPrepareHostdevPCIDevices(virQEMUDriverPtr driver,
 
 inactivedevs:
     /* Only steal all the devices from driver->activePciHostdevs. We will
-     * free them in virPCIDeviceListFree().
+     * free them in virObjectUnref().
      */
     while (virPCIDeviceListCount(pcidevs) > 0) {
         virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, 0);
@@ -580,7 +580,7 @@ reattachdevs:
     }
 
 cleanup:
-    virPCIDeviceListFree(pcidevs);
+    virObjectUnref(pcidevs);
     virObjectUnref(cfg);
     return ret;
 }
@@ -686,7 +686,7 @@ qemuFindHostdevUSBDevice(virDomainHostdevDefPtr hostdev,
             *usb = virUSBDeviceListGet(devs, 0);
             virUSBDeviceListSteal(devs, *usb);
         }
-        virUSBDeviceListFree(devs);
+        virObjectUnref(devs);
 
         if (rc == 0) {
             goto out;
@@ -792,7 +792,7 @@ qemuPrepareHostUSBDevices(virQEMUDriverPtr driver,
     ret = 0;
 
 cleanup:
-    virUSBDeviceListFree(list);
+    virObjectUnref(list);
     return ret;
 }
 
@@ -880,7 +880,7 @@ void qemuDomainReAttachHostdevDevices(virQEMUDriverPtr driver,
             continue;
         }
 
-        /* virPCIDeviceListFree() will take care of freeing the dev. */
+        /* virObjectUnref() will take care of freeing the dev. */
         virPCIDeviceListSteal(driver->activePciHostdevs, dev);
     }
 
@@ -916,7 +916,7 @@ void qemuDomainReAttachHostdevDevices(virQEMUDriverPtr driver,
         qemuReattachPciDevice(dev, driver);
     }
 
-    virPCIDeviceListFree(pcidevs);
+    virObjectUnref(pcidevs);
 cleanup:
     virObjectUnref(cfg);
 }

src/qemu/qemu_hotplug.c

@@ -1221,7 +1221,7 @@ int qemuDomainAttachHostDevice(virQEMUDriverPtr driver,
         goto error;
     }
 
-    virUSBDeviceListFree(list);
+    virObjectUnref(list);
     return 0;
 
 error:
@@ -1230,7 +1230,7 @@ error:
         VIR_WARN("Unable to restore host device labelling on hotplug fail");
 
 cleanup:
-    virUSBDeviceListFree(list);
+    virObjectUnref(list);
     if (usb)
         virUSBDeviceListSteal(driver->activeUsbHostdevs, usb);
     return -1;

src/util/virpci.c

@@ -74,6 +74,8 @@ struct _virPCIDevice {
 };
 
 struct _virPCIDeviceList {
+    virObjectLockable parent;
+
     unsigned count;
     virPCIDevicePtr *devs;
 };
@@ -165,6 +167,23 @@ struct _virPCIDeviceList {
                                  PCI_EXT_CAP_ACS_CR |   \
                                  PCI_EXT_CAP_ACS_UF)
 
+static virClassPtr virPCIDeviceListClass;
+
+static void virPCIDeviceListDispose(void *obj);
+
+static int virPCIOnceInit(void)
+{
+    if (!(virPCIDeviceListClass = virClassNew(virClassForObjectLockable(),
+                                              "virPCIDeviceList",
+                                              sizeof(virPCIDeviceList),
+                                              virPCIDeviceListDispose)))
+        return -1;
+
+    return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(virPCI)
+
 static int
 virPCIDeviceConfigOpen(virPCIDevicePtr dev, bool fatal)
 {
@@ -1505,22 +1524,21 @@ virPCIDeviceListNew(void)
 {
     virPCIDeviceListPtr list;
 
-    if (VIR_ALLOC(list) < 0) {
+    if (virPCIInitialize() < 0)
-        virReportOOMError();
+        return NULL;
+
+    if (!(list = virObjectLockableNew(virPCIDeviceListClass)))
         return NULL;
-    }
 
     return list;
 }
 
-void
+static void
-virPCIDeviceListFree(virPCIDeviceListPtr list)
+virPCIDeviceListDispose(void *obj)
 {
+    virPCIDeviceListPtr list = obj;
     int i;
 
-    if (!list)
-        return;
-
     for (i = 0; i < list->count; i++) {
         virPCIDeviceFree(list->devs[i]);
         list->devs[i] = NULL;
@@ -1528,7 +1546,6 @@ virPCIDeviceListFree(virPCIDeviceListPtr list)
 
     list->count = 0;
     VIR_FREE(list->devs);
-    VIR_FREE(list);
 }
 
 int

src/util/virpci.h

@@ -25,6 +25,7 @@
 # define __VIR_PCI_H__
 
 # include "internal.h"
+# include "virobject.h"
 
 typedef struct _virPCIDevice virPCIDevice;
 typedef virPCIDevice *virPCIDevicePtr;
@@ -78,7 +79,6 @@ void virPCIDeviceReattachInit(virPCIDevice *dev);
 
 
 virPCIDeviceListPtr virPCIDeviceListNew(void);
-void virPCIDeviceListFree(virPCIDeviceListPtr list);
 int  virPCIDeviceListAdd(virPCIDeviceListPtr list,
                          virPCIDevicePtr dev);
 virPCIDevicePtr virPCIDeviceListGet(virPCIDeviceListPtr list,

src/util/virusb.c

@@ -57,6 +57,7 @@ struct _virUSBDevice {
 };
 
 struct _virUSBDeviceList {
+    virObjectLockable parent;
     unsigned int count;
     virUSBDevicePtr *devs;
 };
@@ -67,6 +68,23 @@ typedef enum {
     USB_DEVICE_FIND_BY_BUS = 1 << 1,
 } virUSBDeviceFindFlags;
 
+static virClassPtr virUSBDeviceListClass;
+
+static void virUSBDeviceListDispose(void *obj);
+
+static int virUSBOnceInit(void)
+{
+    if (!(virUSBDeviceListClass = virClassNew(virClassForObjectLockable(),
+                                              "virUSBDeviceList",
+                                              sizeof(virUSBDeviceList),
+                                              virUSBDeviceListDispose)))
+        return -1;
+
+    return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(virUSB)
+
 static int virUSBSysReadFile(const char *f_name, const char *d_name,
                              int base, unsigned int *value)
 {
@@ -184,7 +202,7 @@ cleanup:
     }
 
     if (!ret)
-        virUSBDeviceListFree(list);
+        virObjectUnref(list);
     return ret;
 }
 
@@ -204,7 +222,7 @@ virUSBDeviceFindByVendor(unsigned int vendor,
         return -1;
 
     if (list->count == 0) {
-        virUSBDeviceListFree(list);
+        virObjectUnref(list);
         if (!mandatory) {
             VIR_DEBUG("Did not find USB device %x:%x",
                       vendor, product);
@@ -222,7 +240,7 @@ virUSBDeviceFindByVendor(unsigned int vendor,
     if (devices)
         *devices = list;
     else
-        virUSBDeviceListFree(list);
+        virObjectUnref(list);
 
     return count;
 }
@@ -242,7 +260,7 @@ virUSBDeviceFindByBus(unsigned int bus,
         return -1;
 
     if (list->count == 0) {
-        virUSBDeviceListFree(list);
+        virObjectUnref(list);
         if (!mandatory) {
             VIR_DEBUG("Did not find USB device bus:%u device:%u",
                       bus, devno);
@@ -261,7 +279,7 @@ virUSBDeviceFindByBus(unsigned int bus,
         *usb = virUSBDeviceListGet(list, 0);
         virUSBDeviceListSteal(list, *usb);
     }
-    virUSBDeviceListFree(list);
+    virObjectUnref(list);
 
     return 0;
 }
@@ -283,7 +301,7 @@ virUSBDeviceFind(unsigned int vendor,
         return -1;
 
     if (list->count == 0) {
-        virUSBDeviceListFree(list);
+        virObjectUnref(list);
         if (!mandatory) {
             VIR_DEBUG("Did not find USB device %x:%x bus:%u device:%u",
                       vendor, product, bus, devno);
@@ -302,7 +320,7 @@ virUSBDeviceFind(unsigned int vendor,
         *usb = virUSBDeviceListGet(list, 0);
         virUSBDeviceListSteal(list, *usb);
     }
-    virUSBDeviceListFree(list);
+    virObjectUnref(list);
 
     return 0;
 }
@@ -404,27 +422,25 @@ virUSBDeviceListNew(void)
 {
     virUSBDeviceListPtr list;
 
-    if (VIR_ALLOC(list) < 0) {
+    if (virUSBInitialize() < 0)
-        virReportOOMError();
+        return NULL;
+
+    if (!(list = virObjectLockableNew(virUSBDeviceListClass)))
         return NULL;
-    }
 
     return list;
 }
 
-void
+static void
-virUSBDeviceListFree(virUSBDeviceListPtr list)
+virUSBDeviceListDispose(void *obj)
 {
+    virUSBDeviceListPtr list = obj;
     int i;
 
-    if (!list)
-        return;
-
     for (i = 0; i < list->count; i++)
         virUSBDeviceFree(list->devs[i]);
 
     VIR_FREE(list->devs);
-    VIR_FREE(list);
 }
 
 int

src/util/virusb.h

@@ -26,6 +26,7 @@
 # define __VIR_USB_H__
 
 # include "internal.h"
+# include "virobject.h"
 
 # define USB_DEVFS "/dev/bus/usb/"
 
@@ -81,7 +82,6 @@ int virUSBDeviceFileIterate(virUSBDevicePtr dev,
                             void *opaque);
 
 virUSBDeviceListPtr virUSBDeviceListNew(void);
-void virUSBDeviceListFree(virUSBDeviceListPtr list);
 int virUSBDeviceListAdd(virUSBDeviceListPtr list,
                         virUSBDevicePtr dev);
 virUSBDevicePtr virUSBDeviceListGet(virUSBDeviceListPtr list,