[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

>>> On 06.05.16 at 09:49, <JBeulich@xxxxxxxx> wrote:
>>>> On 06.05.16 at 07:01, <JGross@xxxxxxxx> wrote:
>> On 05/05/16 11:22, Wei Liu wrote:
>>> On Thu, May 05, 2016 at 11:10:33AM +0200, Juergen Gross wrote:
>>>> 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.
>>> OK. I get it now.
>>>> 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?
>>> Actually not all public #define are prefixed by XEN_ (netif.h does,
>>> blkif.h doesn't) so I won't insists on this. But I still using XEN_
>>> prefix is better.
>> Sure. But I think it should be consistent at header file level. So in
>> my opinion the question is: should I change all definitions in usbif.h
>> to use the XEN_ prefix or should I add the new definition without
>> prefix?
> Since changing them all is not even an option (breaking possible
> existing users, even if we don't know of any, is not allowed), I
> think leaving the XEN_ off of the new addition here is acceptable
> (as being more consistent inside the header, as you validly say). So
> since Wei already said he won't insist on the prefix, I think this can
> go in as is.

Of course only if Wei would release-ack it...


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.