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

Re: [libvirt] [PATCH 2/3] qemu: make use of usb search function to initialize usb devices



On 05/05/2012 03:36 AM, Eric Blake wrote:
On 05/04/2012 02:25 AM, Guannan Ren wrote:
refactor qemuPrepareHostdevUSBDevices function, make it focus on
adding usb device to activeUsbHostdevs after check. After that,
the usb hotplug function qemuDomainAttachHostDevice also could use
it.

expand qemuPrepareHostUSBDevices to perform the usb search,
rollback on failure.
---
  src/qemu/qemu_hostdev.c |  117 +++++++++++++++++++++++++++-------------------
  src/qemu/qemu_hostdev.h |    3 +-
  2 files changed, 70 insertions(+), 50 deletions(-)
diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index b98fd8f..0cb89a6 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -565,13 +565,50 @@ qemuPrepareHostPCIDevices(struct qemud_driver *driver,
  int
  qemuPrepareHostdevUSBDevices(struct qemud_driver *driver,
                               const char *name,
-                             virDomainHostdevDefPtr *hostdevs,
-                             int nhostdevs)
+                             usbDeviceList *list)
  {
-    int ret = -1;
      int i;
+    usbDevice *tmp;
+
+    for (i = 0; i<  usbDeviceListCount(list); i++) {
Thankfully, usbDeviceListCount() is O(1); if it were O(n) then you would
have a quadratic loop.  Still, it is slightly faster to factor the
function call out of the for loop.  On the other hand, right now we
aren't using gcc's attribute((pure/const)) because the warnings in gcc
4.6 were buggy; if we were to use that, then usbDeviceListCount() would
already be marked and there would be nothing to factor out.  So, this is
fine as is, and we can save the optimization for a future day when we
rely on gcc 4.7 to help us mark the attributes where needed.

      I combine the two patches 2/3, 3/3, and made a small modification
   on invoking usbDeviceListCount() on V5.

...

+    unsigned int count;
+    usbDevice *tmp;
+
+    count = usbDeviceListCount(list);
+
+    for (i = 0; i<  count; i++) {
...




Unfortunately, this failed to compile on its own:

qemu/qemu_hotplug.c: In function 'qemuDomainAttachHostDevice':
qemu/qemu_hotplug.c:1137:9: error: passing argument 3 of
'qemuPrepareHostdevUSBDevices' from incompatible pointer type [-Werror]
qemu/qemu_hostdev.h:39:5: note: expected 'struct usbDeviceList *' but
argument is of type 'struct virDomainHostdevDef **'
qemu/qemu_hotplug.c:1137:9: error: too many arguments to function
'qemuPrepareHostdevUSBDevices'
qemu/qemu_hostdev.h:39:5: note: declared here

But since applying patch 3/3 fixes the failure:

ACK if you squash 2/3 and 3/3 into a single patch, so that the total
series is only 2 patches instead of 3.


    Thanks for these reviews and comments.

    Guannan Ren




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