Re: [Xen-devel] [Patch V2 2/3] usb: Introduce Xen pvUSB frontend

[Juergen Gross <jgross@suse.com>]

On 06/12/2015 06:20 PM, Greg KH wrote:
> On Fri, Jun 12, 2015 at 04:10:00PM +0200, Juergen Gross wrote:
>> Introduces the Xen pvUSB frontend. With pvUSB it is possible for a Xen
>> domU to communicate with a USB device assigned to that domU. The
>> communication is all done via the pvUSB backend in a driver domain
>> (usually Dom0) which is owner of the physical device.
>>
>> The code is taken from the pvUSB implementation in Xen done by Fujitsu
>> based on Linux kernel 2.6.18.
>>
>> Changes from the original version are:
>> - port to upstream kernel
>> - put all code in just one source file
>> - move module to appropriate location in kernel tree
>> - adapt to Linux style guide
>> - minor code modifications to increase readability
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>>   drivers/usb/Kconfig            |    2 +
>>   drivers/usb/Makefile           |    2 +
>>   drivers/usb/xen/Kconfig        |   10 +
>>   drivers/usb/xen/Makefile       |    5 +
>>   drivers/usb/xen/xen-usbfront.c | 1647 ++++++++++++++++++++++++++++++++++++++++
>
> A subdirectory for a single file?  That seems like overkill, don't you
> think?  As this is a USB "host" driver, why not put it in that
> directory?

As already suggested by David: I'll change this.

>
> Also, last time these patches were posted, people asked why you can't
> use libusb/usbfs instead, what happened with that?  Or usbip?

Like David already said: this was the backend, which I'm currently
working on to be part of qemu using libusb.

>
>> +config XEN_USB_FRONTEND
>> +	tristate "Xen USB frontend driver"
>> +	depends on XEN
>> +	default m
>
> Remove this, default should be 'n'.

Really? Don't you think default should be to use USB devices in Xen
guests? Kernels configured not to be able to run as Xen guests with
pv drivers won't have that module due to "depends on XEN".

>
>> +/* status of attached device */
>> +struct vdevice_status {
>> +	int devnum;
>> +	enum usb_device_state status;
>> +	 enum usb_device_speed speed;
>> +};
>
> Always run your patches through checkpatch.pl so people don't tell you
> about the things that checkpatch.pl would have told you about...

I did (I'll change the indentation, of course):

./scripts/checkpatch.pl 
jg/out.pvusb.v2/0002-usb-Introduce-Xen-pvUSB-frontend.patch
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#57:
new file mode 100644

total: 0 errors, 1 warnings, 1675 lines checked

jg/out.pvusb.v2/0002-usb-Introduce-Xen-pvUSB-frontend.patch has style 
problems, please review.

If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.


Juergen