Re: [Xen-devel] [PATCH for 4.7] pvusb: add missing definition to usbif.h

[Juergen Gross <jgross@xxxxxxxx>]

On 05/05/16 11:02, Wei Liu wrote:
> On Thu, May 05, 2016 at 08:36:45AM +0200, Juergen Gross wrote:
>> The pvusb request structure contains the transfer_flags member which
>> is missing definitions of it's semantics.
>>
>> Add the definition of the USBIF_SHORT_NOT_OK flag.
>>
>> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
>> ---
>> Please consider taking this patch for 4.7 release. I believe this is the
>> last bit missing for support of qemu based pvusb backend. The risk of the
>> patch should be zero, as no Xen component is using this header.
>> ---
>>  xen/include/public/io/usbif.h | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/xen/include/public/io/usbif.h b/xen/include/public/io/usbif.h
>> index 9ef0cdc..4053c24 100644
>> --- a/xen/include/public/io/usbif.h
>> +++ b/xen/include/public/io/usbif.h
>> @@ -187,6 +187,7 @@ struct usbif_urb_request {
>>      /* basic urb parameter */
>>      uint32_t pipe;
>>      uint16_t transfer_flags;
>> +#define USBIF_SHORT_NOT_OK  0x0001
> 
> Where does this come from? Should it be surrounded by define guard?

I just wasn't defined up to now (to be precise: transfer_flags was just
copied from the related URB struct member in the frontend, so the
interface was based on some Linux kernel internals, and the qemu backend
used a literal "1" for testing the flag).

> #ifndef USBIF_SHORT_NOT_OK
> #define USBIF_SHORT_NOT_OK 0x0001
> #endif
> 
> Why does it need to be in our public header? If we end up taking this
> I think it should at least start with XEN_ prefix.

This is just a part of the pvusb interface. So it should be defined in
the appropriate header file.

Regarding prefix: I can do this, but in this case I'd prefer to add the
prefix to all definitions in the header. As there are currently no
in-tree users of this header, the risk would still be zero. :-)

Thoughts?


Juergen


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel