Re: [PATCH v2 3/3] qemu: search usb device accurately to improve usb device hotplug

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

 



On 2012年05月01日 16:16, Guannan Ren wrote:
One usb device could be allowed to hotplug in at a time. If user
give a xml as follows.

s/give/gives/

Probably there are two usb devices avaiable

s/avaiable/available/

but with different value of "bus, device"

we give a error to let user use<address>  to specify the desired one.

<hostdev mode='subsystem' type='usb' managed='yes'>
   <source>
     <vendor id='0x15e1'/>
     <product id='0x2007'/>
   </source>
</hostdev>
---
  src/qemu/qemu_hotplug.c |   68 +++++++++++++++++++++++++++++++++++++---------
  1 files changed, 54 insertions(+), 14 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 7cf7b90..15693a0 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1121,6 +1121,9 @@ int qemuDomainAttachHostDevice(struct qemud_driver *driver,
                                 virDomainObjPtr vm,
                                 virDomainHostdevDefPtr hostdev)
  {
+    usbDeviceList *list;
+    usbDevice * usb = NULL;

s/usbDevice * usb/usbDevice *usb/

+
      if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
          qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                          _("hostdev mode '%s' not supported"),
@@ -1128,29 +1131,62 @@ int qemuDomainAttachHostDevice(struct qemud_driver *driver,
          return -1;
      }

-    /* 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;
+    if (!(list = usbDeviceListNew()))

virReportOOMError()

+        goto cleanup;
+
+    if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) {
+        unsigned vendor = hostdev->source.subsys.u.usb.vendor;
+        unsigned product = hostdev->source.subsys.u.usb.product;
+        unsigned bus = hostdev->source.subsys.u.usb.bus;
+        unsigned device = hostdev->source.subsys.u.usb.device;
+
+        if (vendor&&  bus) {
+            usb = usbFindDevice(vendor, product, bus, device);
+            if (!usb)
+                goto cleanup;

Should be removed. See [1]

+
+        } else if (vendor&&  !bus) {
+            usbDeviceList *devs = usbFindDevByVendor(vendor, product);
+            if (!devs)
+                goto cleanup;
+
+            if (usbDeviceListCount(devs)>  1) {
+                qemuReportError(VIR_ERR_XML_ERROR,
+                                _("multiple USB deivces %x:%x, "
+                                  "use<address>  to specify one"), vendor, product);
+                usbDeviceListFree(devs);
+                goto cleanup;
+            }
+            usb = usbDeviceListGet(devs, 0);
+            usbDeviceListSteal(devs, usb);
+            usbDeviceListFree(devs);

-        usbDevice *usb
-            = usbFindDevice(hostdev->source.subsys.u.usb.vendor,
-                            hostdev->source.subsys.u.usb.product);
+            hostdev->source.subsys.u.usb.bus = usbDeviceGetBus(usb);
+            hostdev->source.subsys.u.usb.device = usbDeviceGetDevno(usb);
+
+        } else if (!vendor&&  bus) {
+            usb = usbFindDevByBus(bus, device);
+            if (!usb)
+                goto cleanup;

Should be removed. see [1]

+        }

          if (!usb)
-            return -1;
+            goto cleanup;

[1] There is a duplicate check here.


-        hostdev->source.subsys.u.usb.bus = usbDeviceGetBus(usb);
-        hostdev->source.subsys.u.usb.device = usbDeviceGetDevno(usb);
+        if (usbDeviceListAdd(list, usb)<  0) {
+            usbFreeDevice(usb);
+            goto cleanup;
+        }

-        usbFreeDevice(usb);
-    }
+        if (qemuPrepareHostdevUSBDevices(driver, vm->def->name, list)<  0)
+            goto cleanup;

Do we really need a list here? there is only found usb device is , I guess
the only purpose is to take use of the new helper function
qemuPrepareHostdevUSBDevices, I.e. to use the new helper function,
it create a new list, and the only one usb device is inserted to
and removed from the list. IMO it's a waste.


+        usbDeviceListSteal(list, usb);
+    }

      if (virSecurityManagerSetHostdevLabel(driver->securityManager,
                                            vm->def, hostdev)<  0)
-        return -1;
+        goto cleanup;

      switch (hostdev->source.subsys.type) {
      case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
@@ -1172,6 +1208,7 @@ int qemuDomainAttachHostDevice(struct qemud_driver *driver,
          goto error;
      }

+    usbDeviceListFree(list);
      return 0;

  error:
@@ -1179,6 +1216,9 @@ error:
                                                vm->def, hostdev)<  0)
          VIR_WARN("Unable to restore host device labelling on hotplug fail");

+cleanup:
+    usbDeviceListFree(list);
+    usbDeviceListSteal(driver->activeUsbHostdevs, usb);
      return -1;
  }


--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]