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

[Jim Fehlig]

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 at 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)

Second, I agree that we don't want to add a USB device to the config if libxl
cant support it.

Regards,
Jim