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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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".

> + *
> + */
> +#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.

> +#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).

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.

Ian.


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