Re: [Xen-devel] [PATCH v2] tools/xl: correctly shows split eventchannel for netfront

[annie li <annie.li@xxxxxxxxxx>]

On 2014/1/24 18:58, Ian Campbell wrote:
> On Fri, 2014-01-24 at 18:51 +0800, Annie Li wrote:
>
> > +/*
> > + * LIBXL_HAVE_NETWORK_SPLIT_EVENTCHANNEL
> > + * If this is defined, libxl_nicinfo will contain an integer type field: 
> > evtch_rx,
> > + * containing the value of eventchannel for rx path of netback&netfront which 
> > support
> > + * split event channel. The original evtch field contains the value of 
> > eventchannel
> > + * for tx path in such case.
> I think it can be either "event channel" (two words) or
> "evtchn" (abbreviation) but not "eventchannel".
Ok, thanks.

> > + *
> > + */
> > +#if LIBXL_API_VERSION > 0x040400
> > +#define LIBXL_HAVE_NETWORK_SPLIT_EVENTCHANNEL 1
> This should be unconditional. There are several existing examples in
> libxl.h which you could have copied.
OK

> > +#endif
> > +
> >   /* Functions annotated with LIBXL_EXTERNAL_CALLERS_ONLY may not be
> >    * called from within libxl itself. Callers outside libxl, who
> >    * do not #include libxl_internal.h, are fine. */
> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > index 649ce50..b1b4946 100644
> > --- a/tools/libxl/libxl_types.idl
> > +++ b/tools/libxl/libxl_types.idl
> > @@ -489,6 +489,9 @@ libxl_nicinfo = Struct("nicinfo", [
> >       ("devid", libxl_devid),
> >       ("state", integer),
> >       ("evtch", integer),
> > +#ifdef LIBXL_HAVE_NETWORK_SPLIT_EVENTCHANNEL
> > +    ("evtch_rx", integer),
> > +#endif
> Did you even build this? Because it can't have worked (Libxl_types.idl
> is a Python source file which is not preprocessed).
My fault, I only built out xl from libxl and replace current one for 
test, not familiar with Python.:-(
> In any case, this ifdef is unneccessary and wrong. the LIBXL_HAVE
> indicates to the consumer that the field is present, but it should not
> actually gate the presence of the field.
>
> Think about it -- if an application is building against libxl version
> 4.4 (which was therefore built with evtchn_rx present) but requests
> LIBXL_API_VERSION == 0x040300 then your patch has now created an ABI
> mismatch.
I misunderstood LIBXL_HAVE_* in libxl.h, and thought it is used by libxl 
itself. Much clear now, thanks.
Thanks
Annie

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