Re: [Xen-devel] [PATCH 1 of 5 V2] remus: add libnl3 dependency to autoconf scripts

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2013-08-29 at 15:16 -0700, Shriram Rajagopalan wrote:
> # HG changeset patch
> # User Shriram Rajagopalan <rshriram@xxxxxxxxx>
> # Date 1377641625 25200
> # Node ID cc2b88cba1c67cc0e0a00c7e63a3ba97e14609cb
> # Parent  40b079bd57dea24c3b000f233ed123763483a4d5
> remus: add libnl3 dependency to autoconf scripts
> 
> 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.
> 
> Signed-off-by: Shriram Rajagopalan <rshriram@xxxxxxxxx>
> 
> diff -r 40b079bd57de -r cc2b88cba1c6 config/Tools.mk.in
> --- a/config/Tools.mk.in      Sun Aug 25 14:39:36 2013 -0700
> +++ b/config/Tools.mk.in      Tue Aug 27 15:13:45 2013 -0700
> @@ -35,6 +35,7 @@ PTHREAD_LDFLAGS     := @PTHREAD_LDFLAGS@
>  PTHREAD_LIBS        := @PTHREAD_LIBS@
>  
>  PTYFUNCS_LIBS       := @PTYFUNCS_LIBS@
> +LIBNL3_LIBS         := @LIBNL3_LIBS@
>  
>  # Download GIT repositories via HTTP or GIT's own protocol?
>  # GIT's protocol is faster and more robust, when it works at all (firewalls
> @@ -54,6 +55,7 @@ CONFIG_QEMU_TRAD    := @qemu_traditional
>  CONFIG_QEMU_XEN     := @qemu_xen@
>  CONFIG_XEND         := @xend@
>  CONFIG_BLKTAP1      := @blktap1@
> +CONFIG_REMUS_NETBUF := @remus_netbuf@
>  
>  #System options
>  ZLIB                := @zlib@
> diff -r 40b079bd57de -r cc2b88cba1c6 tools/configure.ac
> --- a/tools/configure.ac      Sun Aug 25 14:39:36 2013 -0700
> +++ b/tools/configure.ac      Tue Aug 27 15:13:45 2013 -0700
> @@ -208,4 +208,19 @@ AC_SUBST(libiconv)
>  # Checks for header files.
>  AC_CHECK_HEADERS([yajl/yajl_version.h sys/eventfd.h])
>  
> +# Check for libnl3 >=3.2.8. If present enable remus network buffering.
> +PKG_CHECK_EXISTS([libnl-3.0 >= 3.2.8 libnl-route-3.0 >= 3.2.8 libnl-cli-3.0 
> >= 3.2.8],
> +             [libnl3_lib="y"], [libnl3_lib="n"])
> +AC_CHECK_HEADER([netlink/route/qdisc/plug.h], [libnl3_dev="y"], 
> [libnl3_dev="n"])

Aren't these a bit redundant? i.e. doesn't PKG_CHECK_EXISTS cover the
development headers? At least on Debian libnl-3-dev contains:
[...]
-rw-r--r-- root/root       770 2013-05-21 22:52 
./usr/include/libnl3/netlink/route/qdisc/plug.h
[...]
-rw-r--r-- root/root       239 2013-05-21 22:52 
./usr/lib/x86_64-linux-gnu/pkgconfig/libnl-3.0.pc
[...]

> +AS_IF([test "x$libnl3" = "xn" || test "x$libnl3_dev" = "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])

This should be mentioned in README too.

> +         AC_SUBST(remus_netbuf, [n])
> +         ],[
> +         LIBNL3_LIBS="-lnl-3 -lnl-route-3"

Since libnl3 uses pkg-cofnig, these should come from there I think.
There's probably a macro in pkg-config.m4 to help?

> +         AC_SUBST(LIBNL3_LIBS)
> +         AC_SUBST(remus_netbuf, [y])
> +])
> +
>  AC_OUTPUT()
> diff -r 40b079bd57de -r cc2b88cba1c6 tools/libxl/Makefile
> --- a/tools/libxl/Makefile    Sun Aug 25 14:39:36 2013 -0700
> +++ b/tools/libxl/Makefile    Tue Aug 27 15:13:45 2013 -0700
> @@ -20,7 +20,7 @@ LIBUUID_LIBS += -luuid
>  endif
>  
>  LIBXL_LIBS =
> -LIBXL_LIBS = $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) 
> $(LDLIBS_libxenstore) $(LDLIBS_libblktapctl) $(PTYFUNCS_LIBS) $(LIBUUID_LIBS)
> +LIBXL_LIBS = $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) 
> $(LDLIBS_libxenstore) $(LDLIBS_libblktapctl) $(PTYFUNCS_LIBS) $(LIBUUID_LIBS) 
> $(LIBNL3_LIBS)

I think it might be time to break this long line and use
LIBXL_LIBS  = X Y Z
LIBXL_LIBS += A B C
(no need for one line per lib though)

>  
>  CFLAGS_LIBXL += $(CFLAGS_libxenctrl)
>  CFLAGS_LIBXL += $(CFLAGS_libxenguest)



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