[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] qemu: Keep list of USB devices attached to domains



On 2011年12月23日 01:20, Michal Privoznik wrote:
In order to avoid situation where a USB device is
in use by two domains, we must keep a list of already
attached devices like we do for PCI.
---
  src/qemu/qemu_conf.h    |    2 +
  src/qemu/qemu_driver.c  |    4 +
  src/qemu/qemu_hostdev.c |   92 ++++++++++++++++++++++++++++--
  src/qemu/qemu_hostdev.h |    4 +
  src/qemu/qemu_hotplug.c |   17 +++++-
  src/util/hostusb.c      |  140 +++++++++++++++++++++++++++++++++++++++++++++++
  src/util/hostusb.h      |   18 ++++++
  7 files changed, 269 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index f5a0f60..d8d7915 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -36,6 +36,7 @@
  # include "security/security_manager.h"
  # include "cgroup.h"
  # include "pci.h"
+# include "hostusb.h"
  # include "cpu_conf.h"
  # include "driver.h"
  # include "bitmap.h"
@@ -125,6 +126,7 @@ struct qemud_driver {
      bool autoStartBypassCache;

      pciDeviceList *activePciHostdevs;
+    usbDeviceList *activeUsbHostdevs;

      virBitmapPtr reservedVNCPorts;

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c908135..eeeb935 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -583,6 +583,9 @@ qemudStartup(int privileged) {
      if ((qemu_driver->activePciHostdevs = pciDeviceListNew()) == NULL)
          goto error;

+    if ((qemu_driver->activeUsbHostdevs = usbDeviceListNew()) == NULL)
+        goto error;
+
      if (privileged) {
          if (chown(qemu_driver->libDir, qemu_driver->user, qemu_driver->group)<  0) {
              virReportSystemError(errno,
@@ -773,6 +776,7 @@ qemudShutdown(void) {

      qemuDriverLock(qemu_driver);
      pciDeviceListFree(qemu_driver->activePciHostdevs);
+    usbDeviceListFree(qemu_driver->activeUsbHostdevs);
      virCapabilitiesFree(qemu_driver->caps);

      virDomainObjListDeinit(&qemu_driver->domains);
diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index 60401f0..c7adb1d 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -314,13 +314,30 @@ qemuPrepareHostPCIDevices(struct qemud_driver *driver,
  }


-static int
-qemuPrepareHostUSBDevices(struct qemud_driver *driver ATTRIBUTE_UNUSED,
-                          virDomainDefPtr def)
+int
+qemuPrepareHostdevUSBDevices(struct qemud_driver *driver,
+                             const char *name,
+                             virDomainHostdevDefPtr *hostdevs,
+                             int nhostdevs)
  {
+    int ret = -1;
      int i;
-    for (i = 0 ; i<  def->nhostdevs ; i++) {
-        virDomainHostdevDefPtr hostdev = def->hostdevs[i];
+    usbDeviceList *list;
+    usbDevice *tmp;
+
+    /* To prevent situation where USB device is assigned to two domains
+     * we need to keep a list of currently assigned USB devices.
+     * This is done in several loops which cannot be joined into one big
+     * loop. See qemuPrepareHostdevPCIDevices()
+     */
+    if (!(list = usbDeviceListNew()))
+        goto cleanup;
+
+    /* Loop 1: build temporary list and validate no usb device
+     * is already taken
+     */
+    for (i = 0 ; i<  nhostdevs ; i++) {
+        virDomainHostdevDefPtr hostdev = hostdevs[i];

          if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
              continue;
@@ -339,13 +356,74 @@ qemuPrepareHostUSBDevices(struct qemud_driver *driver ATTRIBUTE_UNUSED,
              hostdev->source.subsys.u.usb.bus = usbDeviceGetBus(usb);
              hostdev->source.subsys.u.usb.device = usbDeviceGetDevno(usb);

-            usbFreeDevice(usb);
+            if ((tmp = usbDeviceListFind(driver->activeUsbHostdevs, usb))) {
+                const char *other_name = usbDeviceGetUsedBy(tmp);
+
+                if (other_name)
+                    qemuReportError(VIR_ERR_OPERATION_INVALID,
+                                    _("USB device %s is in use by domain %s"),
+                                    usbDeviceGetName(tmp), other_name);
+                else
+                    qemuReportError(VIR_ERR_OPERATION_INVALID,
+                                    _("USB device %s is already in use"),
+                                    usbDeviceGetName(tmp));
+                usbFreeDevice(usb);
+                goto cleanup;
+            }
+
+            if (usbDeviceListAdd(list, usb)<  0) {
+                usbFreeDevice(usb);
+                goto cleanup;
+            }
+
          }
      }

-    return 0;
+    /* Loop 2: Mark devices in temporary list as used by @name
+     * and add them do driver list. However, if something goes
+     * wrong, perform rollback.
+     */
+    for (i = 0; i<  usbDeviceListCount(list); i++) {
+        tmp = usbDeviceListGet(list, i);
+        usbDeviceSetUsedBy(tmp, name);
+        if (usbDeviceListAdd(driver->activeUsbHostdevs, tmp)<  0) {
+            usbFreeDevice(tmp);
+            goto inactivedevs;
+        }
+    }
+
+    /* Loop 3: Temporary list was successfully merged with
+     * driver list, so steal all items to avoid freeing them
+     * in cleanup label.
+     */
+    while (usbDeviceListCount(list)>  0) {
+        tmp = usbDeviceListGet(list, 0);
+        usbDeviceListSteal(list, tmp);
+    }
+
+    ret = 0;
+    goto cleanup;
+
+inactivedevs:
+    /* Steal devices from driver->activeUsbHostdevs.
+     * We will free them later.
+     */
+    for (i = 0; i<  usbDeviceListCount(list); i++) {
+        tmp = usbDeviceListGet(list, i);
+        usbDeviceListSteal(driver->activeUsbHostdevs, tmp);
+    }
+
+cleanup:
+    usbDeviceListFree(list);
+    return ret;
  }

+static int
+qemuPrepareHostUSBDevices(struct qemud_driver *driver,
+                          virDomainDefPtr def)
+{
+    return qemuPrepareHostdevUSBDevices(driver, def->name, def->hostdevs, def->nhostdevs);
+}

  int qemuPrepareHostDevices(struct qemud_driver *driver,
                             virDomainDefPtr def)
diff --git a/src/qemu/qemu_hostdev.h b/src/qemu/qemu_hostdev.h
index 07d7de2..d852f5b 100644
--- a/src/qemu/qemu_hostdev.h
+++ b/src/qemu/qemu_hostdev.h
@@ -33,6 +33,10 @@ int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver,
                                   const char *name,
                                   virDomainHostdevDefPtr *hostdevs,
                                   int nhostdevs);
+int qemuPrepareHostdevUSBDevices(struct qemud_driver *driver,
+                                 const char *name,
+                                 virDomainHostdevDefPtr *hostdevs,
+                                 int nhostdevs);
  int qemuPrepareHostDevices(struct qemud_driver *driver,
                             virDomainDefPtr def);
  void qemuReattachPciDevice(pciDevice *dev, struct qemud_driver *driver);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 4067bb0..f3597a1 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1097,6 +1097,9 @@ int qemuDomainAttachHostDevice(struct qemud_driver *driver,
      /* Resolve USB product/vendor to bus/device */
      if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB&&
          hostdev->source.subsys.u.usb.vendor) {
+        if (qemuPrepareHostdevUSBDevices(driver, vm->def->name,&hostdev, 1)<  0)
+            goto error;
+
          usbDevice *usb
              = usbFindDevice(hostdev->source.subsys.u.usb.vendor,
                              hostdev->source.subsys.u.usb.product);
@@ -2068,6 +2071,7 @@ qemuDomainDetachHostUsbDevice(struct qemud_driver *driver,
  {
      virDomainHostdevDefPtr detach = NULL;
      qemuDomainObjPrivatePtr priv = vm->privateData;
+    usbDevice *usb;
      int i, ret;

      for (i = 0 ; i<  vm->def->nhostdevs ; i++) {
@@ -2123,6 +2127,17 @@ qemuDomainDetachHostUsbDevice(struct qemud_driver *driver,
      if (ret<  0)
          return -1;

+    usb = usbGetDevice(detach->source.subsys.u.usb.bus,
+                       detach->source.subsys.u.usb.device);
+    if (usb) {
+        usbDeviceListDel(driver->activeUsbHostdevs, usb);
+        usbFreeDevice(usb);
+    } else {
+        VIR_WARN("Unable to find device %03d.%03d in list of used USB devices",
+                 detach->source.subsys.u.usb.bus,
+                 detach->source.subsys.u.usb.device);
+    }
+
      if (vm->def->nhostdevs>  1) {
          memmove(vm->def->hostdevs + i,
                  vm->def->hostdevs + i + 1,
@@ -2162,7 +2177,7 @@ int qemuDomainDetachHostDevice(struct qemud_driver *driver,
      switch (hostdev->source.subsys.type) {
      case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
          ret = qemuDomainDetachHostPciDevice(driver, vm, dev,&detach);
-        break;
+       break;
      case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
          ret = qemuDomainDetachHostUsbDevice(driver, vm, dev,&detach);
          break;
diff --git a/src/util/hostusb.c b/src/util/hostusb.c
index 1669e2f..92f52a2 100644
--- a/src/util/hostusb.c
+++ b/src/util/hostusb.c
@@ -49,6 +49,12 @@ struct _usbDevice {
      char          name[USB_ADDR_LEN]; /* domain:bus:slot.function */
      char          id[USB_ID_LEN];     /* product vendor */
      char          *path;
+    const char    *used_by;           /* name of the domain using this dev */
+};
+
+struct _usbDeviceList {
+    unsigned int count;
+    usbDevice **devs;
  };

  /* For virReportOOMError()  and virReportSystemError() */
@@ -225,6 +231,22 @@ usbFreeDevice(usbDevice *dev)
  }


+void usbDeviceSetUsedBy(usbDevice *dev,
+                        const char *name)
+{
+    dev->used_by = name;
+}
+
+const char * usbDeviceGetUsedBy(usbDevice *dev)
+{
+    return dev->used_by;
+}
+
+const char *usbDeviceGetName(usbDevice *dev)
+{
+    return dev->name;
+}
+
  unsigned usbDeviceGetBus(usbDevice *dev)
  {
      return dev->bus;
@@ -243,3 +265,121 @@ int usbDeviceFileIterate(usbDevice *dev,
  {
      return (actor)(dev, dev->path, opaque);
  }
+
+usbDeviceList *
+usbDeviceListNew(void)
+{
+    usbDeviceList *list;
+
+    if (VIR_ALLOC(list)<  0) {
+        virReportOOMError();
+        return NULL;
+    }
+
+    return list;
+}
+
+void
+usbDeviceListFree(usbDeviceList *list)
+{
+    int i;
+
+    if (!list)
+        return;
+
+    for (i = 0; i<  list->count; i++)
+        usbFreeDevice(list->devs[i]);
+
+    VIR_FREE(list->devs);
+    VIR_FREE(list);
+}
+
+int
+usbDeviceListAdd(usbDeviceList *list,
+                 usbDevice *dev)
+{
+    if (usbDeviceListFind(list, dev)) {
+        usbReportError(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();
+        return -1;
+    }
+
+    list->devs[list->count++] = dev;
+
+    return 0;
+}
+
+usbDevice *
+usbDeviceListGet(usbDeviceList *list,
+                 int idx)
+{
+    if (idx>= list->count ||
+        idx<  0)
+        return NULL;
+
+    return list->devs[idx];
+}
+
+int
+usbDeviceListCount(usbDeviceList *list)
+{
+    return list->count;
+}
+
+usbDevice *
+usbDeviceListSteal(usbDeviceList *list,
+                   usbDevice *dev)
+{
+    usbDevice *ret = NULL;
+    int i;
+
+    for (i = 0; i<  list->count; i++) {
+        if (list->devs[i]->bus != dev->bus ||
+            list->devs[i]->dev != dev->dev)
+            continue;
+
+        ret = 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;
+    }
+    return ret;
+}
+
+void
+usbDeviceListDel(usbDeviceList *list,
+                 usbDevice *dev)
+{
+    usbDevice *ret = usbDeviceListSteal(list, dev);
+    if (ret)
+        usbFreeDevice(ret);
+}
+
+usbDevice *
+usbDeviceListFind(usbDeviceList *list,
+                  usbDevice *dev)
+{
+    int i;
+
+    for (i = 0; i<  list->count; i++) {
+        if (list->devs[i]->bus == dev->bus&&
+            list->devs[i]->dev == dev->dev)
+            return list->devs[i];
+    }
+
+    return NULL;
+}
diff --git a/src/util/hostusb.h b/src/util/hostusb.h
index f4a13ca..afaa32f 100644
--- a/src/util/hostusb.h
+++ b/src/util/hostusb.h
@@ -17,6 +17,7 @@
   *
   * Authors:
   *     Daniel P. Berrange<berrange redhat com>
+ *     Michal Privoznik<mprivozn redhat com>
   */

  #ifndef __VIR_USB_H__
@@ -25,12 +26,16 @@
  # include "internal.h"

  typedef struct _usbDevice usbDevice;
+typedef struct _usbDeviceList usbDeviceList;

  usbDevice *usbGetDevice(unsigned bus,
                          unsigned devno);
  usbDevice *usbFindDevice(unsigned vendor,
                           unsigned product);
  void       usbFreeDevice (usbDevice *dev);
+void       usbDeviceSetUsedBy(usbDevice *dev, const char *name);
+const char *usbDeviceGetUsedBy(usbDevice *dev);
+const char *usbDeviceGetName(usbDevice *dev);

  unsigned usbDeviceGetBus(usbDevice *dev);
  unsigned usbDeviceGetDevno(usbDevice *dev);
@@ -49,5 +54,18 @@ int usbDeviceFileIterate(usbDevice *dev,
                           usbDeviceFileActor actor,
                           void *opaque);

+usbDeviceList *usbDeviceListNew(void);
+void           usbDeviceListFree(usbDeviceList *list);
+int            usbDeviceListAdd(usbDeviceList *list,
+                                usbDevice *dev);
+usbDevice *    usbDeviceListGet(usbDeviceList *list,
+                                int idx);
+int            usbDeviceListCount(usbDeviceList *list);
+usbDevice *    usbDeviceListSteal(usbDeviceList *list,
+                                  usbDevice *dev);
+void           usbDeviceListDel(usbDeviceList *list,
+                                usbDevice *dev);
+usbDevice *    usbDeviceListFind(usbDeviceList *list,
+                                 usbDevice *dev);

  #endif /* __VIR_USB_H__ */

Everything looks fine, except missing to add the new internal functions
into libvirt_private.syms. ACK with the nit fixed.

Regards,
Osier


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]