[libvirt] [PATCH v2 2/3] qemu: make use of usb search function to initialize usb devices

Guannan Ren gren at redhat.com
Thu May 3 08:51:39 UTC 2012


On 05/03/2012 01:45 AM, Osier Yang wrote:
> 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/.


        we add usb devices into driver->activeUsbHostdevs from the list 
that passed in.
        we need to steal these usb devices from arguments list in order 
not to be freed
        s/steal the list of usb/steal the usb devices/





>
>> +         * 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.


         The purpose of "qemuPrepareHostdevUSBDevices" is add usb devices
       into qemu driver which is not changed.
         I just move the action of usb device search up, original 
function did two job
       search and add,  currently, it only has "add".


        The code has been fixed according to the rest of comments.
        Thanks for the review.

        Guannan Ren




More information about the libvir-list mailing list