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

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



On 2012年05月03日 02:06, Osier Yang wrote:
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.

I.e. Given the name of the new helper function is confused with
"qemuPrepareHostUSBDevices", and it's actually no need to use it
here, IMO the new helper function is no need.



+ 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 redhat com
https://www.redhat.com/mailman/listinfo/libvir-list


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