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.
+{
+ int i, ret = -1;
usbDeviceList *list;
usbDevice *tmp;
+ virDomainHostdevDefPtr *hostdevs = def->hostdevs;
+ int nhostdevs = def->nhostdevs;
/* To prevent situation where USB device is assigned to two domains
* we need to keep a list of currently assigned USB devices.
@@ -586,64 +624,65 @@ qemuPrepareHostdevUSBDevices(struct qemud_driver *driver,
*/
for (i = 0 ; i< nhostdevs ; i++) {
virDomainHostdevDefPtr hostdev = hostdevs[i];
+ usbDevice *usb = NULL;
if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
continue;
if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB)
continue;
- /* Resolve a vendor/product to bus/device */
- if (hostdev->source.subsys.u.usb.vendor) {
- usbDevice *usb
- = usbFindDevice(hostdev->source.subsys.u.usb.vendor,
- hostdev->source.subsys.u.usb.product);
+ 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;
Given there is a check of 'usb' after the "if...else if..." clauses.
This should be removed. see [1]
- 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));
- usbFreeDevice(usb);
+ } else if (vendor&& !bus) {
+ usbDeviceList *devs = usbFindDevByVendor(vendor, product);
+ if (!devs)
+ goto cleanup;
likewise.
+
+ if (usbDeviceListCount(devs)> 1) {
+ qemuReportError(VIR_ERR_XML_ERROR,
+ _("multiple USB deivces %x:%x, "
+ "use<address> to specify one"), vendor, product);
The place to fix the error type, and have a more clear message.
+ usbDeviceListFree(devs);
goto cleanup;
}
+ usb = usbDeviceListGet(devs, 0);
+ usbDeviceListSteal(devs, usb);
+ usbDeviceListFree(devs);
hostdev->source.subsys.u.usb.bus = usbDeviceGetBus(usb);
hostdev->source.subsys.u.usb.device = usbDeviceGetDevno(usb);
- if (usbDeviceListAdd(list, usb)< 0) {
- usbFreeDevice(usb);
+ } else if (!vendor&& bus) {
+ usb = usbFindDevByBus(bus, device);
+ if (!usb)
goto cleanup;
Should be removed.
- }
+ }
+ if (!usb)
+ goto cleanup;
[1] The check is here.
+
+ if (usbDeviceListAdd(list, usb)< 0) {
+ usbFreeDevice(usb);
+ goto cleanup;
}
}
- /* Loop 2: Mark devices in temporary list as used by @name
+ /* Mark devices in temporary list as used by @name
* and add them do driver list. However, if something goes
* wrong, perform rollback.
*/
- for (i = 0; i< usbDeviceListCount(list); i++) {
- tmp = usbDeviceListGet(list, i);
- usbDeviceSetUsedBy(tmp, name);
+ if (qemuPrepareHostdevUSBDevices(driver, def->name, list)< 0)
+ goto inactivedevs;
- VIR_DEBUG("Adding %03d.%03d dom=%s to activeUsbHostdevs",
- usbDeviceGetBus(tmp), usbDeviceGetDevno(tmp), name);
- if (usbDeviceListAdd(driver->activeUsbHostdevs, tmp)< 0) {
- usbFreeDevice(tmp);
- goto inactivedevs;
- }
- }
-
- /* Loop 3: Temporary list was successfully merged with
+ /* Loop 2: Temporary list was successfully merged with
* driver list, so steal all items to avoid freeing them
* in cleanup label.
*/
@@ -669,13 +708,6 @@ cleanup:
return ret;
}
-static int
-qemuPrepareHostUSBDevices(struct qemud_driver *driver,
- virDomainDefPtr def)
-{
- return qemuPrepareHostdevUSBDevices(driver, def->name, def->hostdevs, def->nhostdevs);
-}
-
int qemuPrepareHostDevices(struct qemud_driver *driver,
virDomainDefPtr def)
{
diff --git a/src/qemu/qemu_hostdev.h b/src/qemu/qemu_hostdev.h
index 371630a..a8acccf 100644
--- a/src/qemu/qemu_hostdev.h
+++ b/src/qemu/qemu_hostdev.h
@@ -38,8 +38,7 @@ int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver,
int nhostdevs);
int qemuPrepareHostdevUSBDevices(struct qemud_driver *driver,
const char *name,
- virDomainHostdevDefPtr *hostdevs,
- int nhostdevs);
+ usbDeviceList *list);
int qemuPrepareHostDevices(struct qemud_driver *driver,
virDomainDefPtr def);
void qemuReattachPciDevice(pciDevice *dev, struct qemud_driver *driver);
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list