From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hongyang Yang Subject: Re: [PATCH v10 2/5] remus: add libnl3 dependency for network buffering support Date: Fri, 6 Jun 2014 09:48:50 +0800 Message-ID: <53911E02.6040706@cn.fujitsu.com> References: <1401932069-16460-1-git-send-email-yanghy@cn.fujitsu.com> <1401932069-16460-3-git-send-email-yanghy@cn.fujitsu.com> <21392.39022.100253.330973@mariner.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: <21392.39022.100253.330973@mariner.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 Jackson Cc: ian.campbell@citrix.com, wency@cn.fujitsu.com, stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com, yunhong.jiang@intel.com, eddie.dong@intel.com, xen-devel@lists.xen.org, rshriram@cs.ubc.ca, roger.pau@citrix.com, laijs@cn.fujitsu.com List-Id: xen-devel@lists.xenproject.org On 06/06/2014 12:18 AM, Ian Jackson wrote: > Yang Hongyang writes ("[PATCH v10 2/5] remus: add libnl3 dependency for network buffering support"): >> Libnl3 is required for controlling Remus network buffering. >> This patch adds dependency on libnl3 (>= 3.2.8) to autoconf scripts. >> Also provide ability to configure tools without libnl3 support, that >> is without network buffering support. > > This patch looks broadly good to me. I have some very minor comments > about the details. > >> when there's no network buffering support,libxl__netbuffer_enabled() >> returns 0, otherwise returns 1. > > The commit message should explicitly state that callers will be > introduced in the rest of the series. > >> Signed-off-by: Shriram Rajagopalan > > For a patch which changes configure.ac, it would be helpful to add a > reminder (for the commiter) to rerun autogen.sh. This should ideally > appear just before the first Signed-off-by. The committer should > delete the note, and rerun autogen.sh, as they apply the patch. Thanks, will add the comment. > >> Signed-off-by: Lai Jiangshan >> Signed-off-by: Yang Hongyang >> Reviewed-by: Wen Congyang > ... >> +# Check for libnl3 >=3.2.8. If present enable remus network buffering. >> +PKG_CHECK_MODULES(LIBNL3, [libnl-3.0 >= 3.2.8 libnl-route-3.0 >= 3.2.8], >> + [libnl3_lib="y"], [libnl3_lib="n"]) >> + >> +AS_IF([test "x$libnl3_lib" = "xn" ], [ >> + AC_MSG_WARN([Disabling support for Remus network buffering. >> + Please install libnl3 libraries, command line tools and devel >> + headers - version 3.2.8 or higher]) >> + AC_SUBST(remus_netbuf, [n]) >> + ],[ >> + AC_SUBST(LIBNL3_LIBS) >> + AC_SUBST(LIBNL3_CFLAGS) > > It might be better to put these AC_SUBSTs into the main body of > configure.ac ? Like this: > > diff --git a/tools/configure.ac b/tools/configure.ac > index 38d2d05..ee36707 100644 > --- a/tools/configure.ac > +++ b/tools/configure.ac > @@ -257,10 +257,11 @@ AS_IF([test "x$libnl3_lib" = "xn" ], [ > headers - version 3.2.8 or higher]) > AC_SUBST(remus_netbuf, [n]) > ],[ > - AC_SUBST(LIBNL3_LIBS) > - AC_SUBST(LIBNL3_CFLAGS) > AC_SUBST(remus_netbuf, [y]) > ]) > > +AC_SUBST(LIBNL3_LIBS) > +AC_SUBST(LIBNL3_CFLAGS) > + > AC_OUTPUT() yes, i'll try that, if we do these, then the following check of CONFIG_REMUS_NETBUF in libxl Makefile will no longer need: LIBXL_LIBS = LIBXL_LIBS = $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(LDLIBS_libblktapctl) $(PTYFUNCS_LIBS) $(LIBUUID_LIBS) +ifeq ($(CONFIG_REMUS_NETBUF),y) +LIBXL_LIBS += $(LIBNL3_LIBS) +endif CFLAGS_LIBXL += $(CFLAGS_libxenctrl) CFLAGS_LIBXL += $(CFLAGS_libxenguest) CFLAGS_LIBXL += $(CFLAGS_libxenstore) CFLAGS_LIBXL += $(CFLAGS_libblktapctl) +ifeq ($(CONFIG_REMUS_NETBUF),y) +CFLAGS_LIBXL += $(LIBNL3_CFLAGS) +endif CFLAGS_LIBXL += -Wshadow > > Thanks, > Ian. > . > -- Thanks, Yang.