From mboxrd@z Thu Jan 1 00:00:00 1970 From: annie li Subject: Re: [PATCH v2] tools/xl: correctly shows split eventchannel for netfront Date: Fri, 24 Jan 2014 22:26:47 +0800 Message-ID: <52E27827.6090405@oracle.com> References: <1390560694-2130-1-git-send-email-Annie.li@oracle.com> <1390561120.2124.50.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1390561120.2124.50.camel@kazak.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: wei.liu2@citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org 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