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

[Wei Liu <wei.liu2@citrix.com>]

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@suse.com>
> >> ---
> >> 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.

Wei.

> 
> Juergen
> 

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