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

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/.

> + * 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.