Re: [Xen-devel] [PATCH] tools: Reorder and add includes in libvchan

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2012-03-13 at 17:09 +0000, George Dunlap wrote:
> On Tue, 2012-03-13 at 16:52 +0000, Ian Campbell wrote:
> > On Tue, 2012-03-13 at 16:36 +0000, George Dunlap wrote:
> > > On Tue, 2012-03-13 at 16:30 +0000, Ian Jackson wrote:
> > > > George Dunlap writes ("[Xen-devel] [PATCH] tools: Reorder and add 
> > > > includes in libvchan"):
> > > > > libxenvchan.h includes xen/sys/evtchn.h, which in the XenServer 
> > > > > patchqueue
> > > > > may contain domid_t values.  Just re-order the order of includes to 
> > > > > make sure
> > > > > that Xen-defined values make sense to the compiler.
> > > > 
> > > > Surely this should be fixed by having (XenServer's) evtchn.h include
> > > > xen.h itself ?
> > > 
> > > Well, that's what I thought, but it's not what ijc thought (IIRC).
> > 
> > I can't remember that, but that's probably just me being forgetful
> > and/or I've not made the link between that conversation and this patch.
> 
> Yeah, I'm having trouble reconstructing the exact conversation too.  It
> just had to do with it being OK to assume that the #include-r had
> included other things already.

IIRC I think my point then was that both styles had proponents and
arguments for why their was the right way ;-).

> 
> > What is the related patch which adds the domid's to evtchn.h? Are we
> > going to see it upstream at some point too?
> 
> Unfortunately not; it's the ioctl having to do with the qemu privilege
> restriction stuff; not suitable for upstream (IIRC), since it requires
> Linux to know details of, and be able to enforce, the hypercall
> interaction between the tools and Xen.

Right, no problem.

> > I notice that upstream tools/include/xen-sys/Linux/evtchn.h (which I
> > presume to be the header in question) uses "unsigned int" for the
> > various domain ids which it contains. ISTM that either a) XS should use
> > "unsigned int" too or b) they should be domid_t in upstream too.
> > 
> > I expect s/unsigned int/domid_t/ == an ABI change so I think only "a)"
> > is actually an option, although maybe "b)" is an option if you are
> > careful/clever with the padding.
> 
> I think I'd be happy with a.  I had actually considered it but discarded
> it for some reason I can't seem to remember now.
> 
> Let me check if 'a' will work; if not, let me see if adding xen/xen.h to
> evtchn.h will work.

OK, thanks!

Ian.



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