On 05/03/2012 01:45 AM, Osier Yang wrote:
On 2012年05月01日 16:16, 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 | 118 ++++++++++++++++++++++++++++++-----------------
  src/qemu/qemu_hostdev.h |    3 +-
  2 files changed, 76 insertions(+), 45 deletions(-)

diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index 8594fb2..a3d4ad4 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -565,13 +565,51 @@ 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++) {
+        usbDevice *usb = usbDeviceListGet(list, i);
+ 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));
+            return -1;
+        }
+
+        usbDeviceSetUsedBy(usb, name);
+        VIR_DEBUG("Adding %03d.%03d dom=%s to activeUsbHostdevs",
+                  usbDeviceGetBus(usb), usbDeviceGetDevno(usb), name);
+        /*
+         * The caller is responsible to steal the list of usb
+         * from the usbDeviceList that passed in on success,

s/steal the list of usb/usb devices which are used by domain/.


we add usb devices into driver->activeUsbHostdevs from the list that passed in. we need to steal these usb devices from arguments list in order not to be freed
       s/steal the list of usb/steal the usb devices/






+         * perform rollback on failure.
+         */
+        if (usbDeviceListAdd(driver->activeUsbHostdevs, usb)<  0)
+            return -1;
+
+    }
+    return 0;
+}
+
+static int
+qemuPrepareHostUSBDevices(struct qemud_driver *driver,
+                          virDomainDefPtr def)

It's confused with qemuPrepareHostdevUSBDevices, the idea to
have a general function (new qemuPrepareHostdevUSBDevices) for
reusing is good. But we should have another name for it instead,
and keep "qemuPrepareHostdevUSBDevices" here.


        The purpose of "qemuPrepareHostdevUSBDevices" is add usb devices
      into qemu driver which is not changed.
I just move the action of usb device search up, original function did two job
      search and add,  currently, it only has "add".


       The code has been fixed according to the rest of comments.
       Thanks for the review.

       Guannan Ren

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

Reply via email to