1:51 a.m.
>> On 5/21/2016 at 05:25 AM, in message
<573F80E3.4070905(a)suse.com>, Jim Fehlig
<jfehlig(a)suse.com> wrote:
On 05/20/2016 02:25 PM, Jim Fehlig wrote:
> On 05/20/2016 04:32 AM, Joao Martins wrote:
>> On 05/19/2016 09:14 AM, Chunyan Liu wrote:
>>> Support hot attach/detach a USB host device to guest.
>>> Curretnly libxl only supports xen PV guest, and only
> s/Curretnly/Currently/
>
>>> supports specifying USB host device by 'bus number'
>>> and 'device number'.
>>>
>>> For example:
>>> usb.xml:
>>> <hostdev mode='subsystem' type='usb'
managed='no'>
>>> <source>
>>> <address bus='1' device='3'/>
>>> </source>
>>> </hostdev>
>>> #xl attach-device dom usb.xml
>>> #xl detach-device dom usb.xml
>>>
>>> Signed-off-by: Chunyan Liu <cyliu(a)suse.com>
>>> ---
>>> Changes:
>>> * add LIBXL_HAVE_PVUSB check
>>> * fix Jim's comments
>>>
>>> src/libxl/libxl_driver.c | 136
++++++++++++++++++++++++++++++++++++++++++++++-
>>> 1 file changed, 135 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>>> index 960673f..a171efe 100644
>>> --- a/src/libxl/libxl_driver.c
>>> +++ b/src/libxl/libxl_driver.c
>>> @@ -3028,6 +3028,56 @@ libxlDomainAttachHostPCIDevice(libxlDriverPrivatePtr
driver,
>>> return ret;
>>> }
>>>
>>> +#ifdef LIBXL_HAVE_PVUSB
>>> +static int
>>> +libxlDomainAttachHostUSBDevice(libxlDriverPrivatePtr driver,
>>> + virDomainObjPtr vm,
>>> + virDomainHostdevDefPtr hostdev)
>>> +{
>>> + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
>>> + libxl_device_usbdev usbdev;
>>> + virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
>>> + int ret = -1;
>>> +
>>> + libxl_device_usbdev_init(&usbdev);
>>> +
>>> + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS ||
>>> + hostdev->source.subsys.type !=
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB)
>>> + goto cleanup;
>>> +
>>> + if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs +
1) < 0)
>>> + goto cleanup;
>>> +
>>> + if (virHostdevPrepareUSBDevices(hostdev_mgr, LIBXL_DRIVER_NAME,
>>> + vm->def->name, &hostdev, 1,
0) < 0)
>>> + goto cleanup;
>>> +
>>> + if (libxlMakeUSB(hostdev, &usbdev) < 0)
>>> + goto reattach;
>>> +
>>> + if (libxl_device_usbdev_add(cfg->ctx, vm->def->id,
&usbdev, 0) < 0) {
>>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>>> + _("libxenlight failed to attach usb device
Busnum:%3x, Devnum:%3x"),
>>> + hostdev->source.subsys.u.usb.bus,
>>> + hostdev->source.subsys.u.usb.device);
>>> + goto reattach;
>>> + }
>>> +
>>> + vm->def->hostdevs[vm->def->nhostdevs++] = hostdev;
>>> + ret = 0;
>>> + goto cleanup;
>>> +
>>> + reattach:
>>> + virHostdevReAttachUSBDevices(hostdev_mgr, LIBXL_DRIVER_NAME,
>>> + vm->def->name, &hostdev, 1);
>>> +
>>> + cleanup:
>>> + virObjectUnref(cfg);
>>> + libxl_device_usbdev_dispose(&usbdev);
>>> + return ret;
>>> +}
>>> +#endif
>>> +
>>> static int
>>> libxlDomainAttachHostDevice(libxlDriverPrivatePtr driver,
>>> virDomainObjPtr vm,
>>> @@ -3046,6 +3096,13 @@ libxlDomainAttachHostDevice(libxlDriverPrivatePtr
driver,
>>> return -1;
>>> break;
>>>
>>> +#ifdef LIBXL_HAVE_PVUSB
>>> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
>>> + if (libxlDomainAttachHostUSBDevice(driver, vm, hostdev) < 0)
>>> + return -1;
>>> + break;
>>> +#endif
>>> +
>>> default:
>>> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>>> _("hostdev subsys type '%s' not
supported"),
>>> @@ -3271,7 +3328,9 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef,
virDomainDeviceDefPtr dev)
>>> case VIR_DOMAIN_DEVICE_HOSTDEV:
>>> hostdev = dev->data.hostdev;
>>>
>>> - if (hostdev->source.subsys.type !=
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
>>> + if (hostdev->source.subsys.type !=
>>> + (VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI ||
>>> + VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB))
>> Is this conditional correct when LIBXL_HAVE_PVUSB isn't there?
> I think there are two problems here. First, (VIR_DOMAIN_DEVICE_DISK ||
> VIR_DOMAIN_DEVICE_LEASE) evaluates to 1, so essentially this is equivalent
to
>
> if (hostdev->source.subsys.type != 1)
Oh, my god. Your are right. Stupid mistake.
>
> Second, I agree that we don't want to add a USB device to the config if
libxl
> cant support it.
Something like the below diff?
Yes, I think it's OK. And another place needs to be updated, since now it's
not only pci device, but also could be usb device, so the error message
should be updated.
if (virDomainHostdevFind(vmdef, hostdev, &found) >= 0) {
- pcisrc = &hostdev->source.subsys.u.pci;
virReportError(VIR_ERR_OPERATION_FAILED,
- _("target pci device %.4x:%.2x:%.2x.%.1x\
- already exists"),
- pcisrc->addr.domain, pcisrc->addr.bus,
- pcisrc->addr.slot, pcisrc->addr.function);
+ _("device already exists in domain
configuration"));
return -1;
}
Sorry for not noticing that in time.
- Chunyan
Regards,
Jim
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 9001f34..5b6f87a 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -3326,10 +3326,14 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef,
virDomainDeviceDefPtr dev)
case VIR_DOMAIN_DEVICE_HOSTDEV:
hostdev = dev->data.hostdev;
- if (hostdev->source.subsys.type !=
- (VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI ||
- VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB))
+ switch (hostdev->source.subsys.type) {
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
+#ifndef LIBXL_HAVE_PVUSB
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
+#endif
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
return -1;
+ }
if (virDomainHostdevFind(vmdef, hostdev, &found) >= 0) {
pcisrc = &hostdev->source.subsys.u.pci;
l