Re: [PATCH V2 2/4] libxl: support hotplug USB host device

[Jim Fehlig <jfehlig@xxxxxxxx>]

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@xxxxxxxx>
>>> ---
>>> 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)
>
> 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?

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

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