Xen devel - Oct 2012 - [PATCH] build/xenstore: Correct static link failure for xenstore

There is support for building xenstore clients statically.  However,
recent changes to the makefiles have rendered the static build broken.

tools/xenstore/Makefile sets LIBXENSTORE depending on whether
XENSTORE_STATIC_CLIENTS is specified, but will unconditionally try to
link against libxenstore.so by use of the LDLIBS_libxenstore variable.

This patch doubles the logic already present to select the appropriate
library target.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

--
This is a bit of a hack, but seems to be the only reliable way,
espcially when linking with the LDLIBS_libxenstore variable in
toos/misc.

diff -r 099589002239 -r 952b1ef29246 tools/Rules.mk
--- a/tools/Rules.mk
+++ b/tools/Rules.mk
@@ -28,7 +28,11 @@ LDLIBS_libxenguest = $(XEN_LIBXC)/libxen
 SHLIB_libxenguest  = -Wl,-rpath-link=L$(XEN_LIBXC)
 
 CFLAGS_libxenstore = -I$(XEN_XENSTORE) $(CFLAGS_xeninclude)
+ifneq ($(XENSTORE_STATIC_CLIENTS),y)
 LDLIBS_libxenstore = $(XEN_XENSTORE)/libxenstore.so
+else
+LDLIBS_libxenstore = $(XEN_XENSTORE)/libxenstore.a
+endif
 SHLIB_libxenstore  = -Wl,-rpath-link=$(XEN_XENSTORE)
 
 CFLAGS_libxenstat  = -I$(XEN_LIBXENSTAT)

On Mon, 2012-10-15 at 16:24 +0100, Andrew Cooper wrote:
> There is support for building xenstore clients statically.  However,
> recent changes to the makefiles have rendered the static build broken.
> 
> tools/xenstore/Makefile sets LIBXENSTORE depending on whether
> XENSTORE_STATIC_CLIENTS is specified, but will unconditionally try to
> link against libxenstore.so by use of the LDLIBS_libxenstore variable.
> 
> This patch doubles the logic already present to select the appropriate
> library target.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> --
> This is a bit of a hack, but seems to be the only reliable way,
> espcially when linking with the LDLIBS_libxenstore variable in
> toos/misc.
I think it would be cleaner to define LDLIBS_libxenstore_static in
Rules.mk alongside the existing thing and make the appropriate selection
in the xenstore Makefile. That keeps the fugliness next to where it is
used.
> 
> diff -r 099589002239 -r 952b1ef29246 tools/Rules.mk
> --- a/tools/Rules.mk
> +++ b/tools/Rules.mk
> @@ -28,7 +28,11 @@ LDLIBS_libxenguest = $(XEN_LIBXC)/libxen
>  SHLIB_libxenguest  = -Wl,-rpath-link=L$(XEN_LIBXC)
>  
>  CFLAGS_libxenstore = -I$(XEN_XENSTORE) $(CFLAGS_xeninclude)
> +ifneq ($(XENSTORE_STATIC_CLIENTS),y)
>  LDLIBS_libxenstore = $(XEN_XENSTORE)/libxenstore.so
> +else
> +LDLIBS_libxenstore = $(XEN_XENSTORE)/libxenstore.a
> +endif
>  SHLIB_libxenstore  = -Wl,-rpath-link=$(XEN_XENSTORE)
>  
>  CFLAGS_libxenstat  = -I$(XEN_LIBXENSTAT)

On 15/10/12 16:31, Ian Campbell wrote:
> On Mon, 2012-10-15 at 16:24 +0100, Andrew Cooper wrote:
>> There is support for building xenstore clients statically.  However,
>> recent changes to the makefiles have rendered the static build broken.
>>
>> tools/xenstore/Makefile sets LIBXENSTORE depending on whether
>> XENSTORE_STATIC_CLIENTS is specified, but will unconditionally try to
>> link against libxenstore.so by use of the LDLIBS_libxenstore variable.
>>
>> This patch doubles the logic already present to select the appropriate
>> library target.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>
>> --
>> This is a bit of a hack, but seems to be the only reliable way,
>> espcially when linking with the LDLIBS_libxenstore variable in
>> toos/misc.
> I think it would be cleaner to define LDLIBS_libxenstore_static in
> Rules.mk alongside the existing thing and make the appropriate selection
> in the xenstore Makefile. That keeps the fugliness next to where it is
> used.
But that then requires patching for *every* consumer of
LDLIBS_libxenstore, which is a substantially larger and more invasive.

~Andrew
>
>> diff -r 099589002239 -r 952b1ef29246 tools/Rules.mk
>> --- a/tools/Rules.mk
>> +++ b/tools/Rules.mk
>> @@ -28,7 +28,11 @@ LDLIBS_libxenguest = $(XEN_LIBXC)/libxen
>>  SHLIB_libxenguest  = -Wl,-rpath-link=L$(XEN_LIBXC)
>>  
>>  CFLAGS_libxenstore = -I$(XEN_XENSTORE) $(CFLAGS_xeninclude)
>> +ifneq ($(XENSTORE_STATIC_CLIENTS),y)
>>  LDLIBS_libxenstore = $(XEN_XENSTORE)/libxenstore.so
>> +else
>> +LDLIBS_libxenstore = $(XEN_XENSTORE)/libxenstore.a
>> +endif
>>  SHLIB_libxenstore  = -Wl,-rpath-link=$(XEN_XENSTORE)
>>  
>>  CFLAGS_libxenstat  = -I$(XEN_LIBXENSTAT)
>
-- 
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com

On Mon, 2012-10-15 at 16:42 +0100, Andrew Cooper wrote:
> On 15/10/12 16:31, Ian Campbell wrote:
> > On Mon, 2012-10-15 at 16:24 +0100, Andrew Cooper wrote:
> >> There is support for building xenstore clients statically. 
However,
> >> recent changes to the makefiles have rendered the static build
broken.
> >>
> >> tools/xenstore/Makefile sets LIBXENSTORE depending on whether
> >> XENSTORE_STATIC_CLIENTS is specified, but will unconditionally try
to
> >> link against libxenstore.so by use of the LDLIBS_libxenstore
variable.
> >>
> >> This patch doubles the logic already present to select the
appropriate
> >> library target.
> >>
> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >>
> >> --
> >> This is a bit of a hack, but seems to be the only reliable way,
> >> espcially when linking with the LDLIBS_libxenstore variable in
> >> toos/misc.
> > I think it would be cleaner to define LDLIBS_libxenstore_static in
> > Rules.mk alongside the existing thing and make the appropriate
selection
> > in the xenstore Makefile. That keeps the fugliness next to where it is
> > used.
> 
> But that then requires patching for *every* consumer of
> LDLIBS_libxenstore, which is a substantially larger and more invasive.
99% of them will continue to use LDLIBS_libxenstore, as they should.

The only place it needs to use LDLIBS_libxenstore_static is when
building the xenstore command line tools in tools/xenstore/Makefile.
Perhaps init-xenstore-domain and xenstore-control need it to, but that
shiould be all

However taking  a step back I think the underlying bug here is actually
the use of $(LDLIBS_libxenstore) where we should be using $(LIBXENSTORE)
when linking and not just as a dependency.

Ian.

> 
> ~Andrew
> 
> >
> >> diff -r 099589002239 -r 952b1ef29246 tools/Rules.mk
> >> --- a/tools/Rules.mk
> >> +++ b/tools/Rules.mk
> >> @@ -28,7 +28,11 @@ LDLIBS_libxenguest = $(XEN_LIBXC)/libxen
> >>  SHLIB_libxenguest  = -Wl,-rpath-link=L$(XEN_LIBXC)
> >>  
> >>  CFLAGS_libxenstore = -I$(XEN_XENSTORE) $(CFLAGS_xeninclude)
> >> +ifneq ($(XENSTORE_STATIC_CLIENTS),y)
> >>  LDLIBS_libxenstore = $(XEN_XENSTORE)/libxenstore.so
> >> +else
> >> +LDLIBS_libxenstore = $(XEN_XENSTORE)/libxenstore.a
> >> +endif
> >>  SHLIB_libxenstore  = -Wl,-rpath-link=$(XEN_XENSTORE)
> >>  
> >>  CFLAGS_libxenstat  = -I$(XEN_LIBXENSTAT)
> >
>