Re: [Xen-devel] [PATCH 9 of 9 RFC v2] blktap3/libblktapctl: Introduce makefile that builds tapback-required libblktapctl functionality

[Thanos Makatos <thanos.makatos@xxxxxxxxxx>]

> On Tue, 2012-12-04 at 18:19 +0000, Thanos Makatos wrote:
> > diff --git a/config/StdGNU.mk b/config/StdGNU.mk
> > --- a/config/StdGNU.mk
> > +++ b/config/StdGNU.mk
> > @@ -77,3 +77,6 @@ ifeq ($(lto),y)
> >  CFLAGS += -flto
> >  LDFLAGS-$(clang) += -plugin LLVMgold.so  endif
> > +
> > +TAPDISK_EXEC = "tapdisk"
> > +TAPDISK_EXECDIR = $(LIBEXEC)
> 
> Do these need to be configurable?
> 
> I'd be tempted to suggest that you just do
>         #define TAPDISK_EXEC "tapdisk"
>         #define TAPDISK_EXECDIR LIBEXEC
> in one of you headers.
> 
> Ian.

They were originally specified in the Makefile and you suggested they were put 
elsewhere:

> > diff --git a/tools/blktap2/control/Makefile
> > b/tools/blktap3/control/Makefile copy from
> > tools/blktap2/control/Makefile copy to tools/blktap3/control/Makefile
> > --- a/tools/blktap2/control/Makefile
> > +++ b/tools/blktap3/control/Makefile
> > @@ -6,40 +6,36 @@ MINOR              = 0
> >  LIBNAME            = libblktapctl
> >  LIBSONAME          = $(LIBNAME).so.$(MAJOR)
> >
> > -IBIN               = tap-ctl
> > +override CFLAGS += \
> > +   -I../include \
> > +   -DTAPDISK_EXEC='"tapdisk"' \
> > +   -DTAPDISK_EXECDIR='"/usr/local/libexec"' \
> 
> This should come from tools/configure and config/Tools.mk etc rather
> than being hard coded here.
> 
> By default it should likely be under /usr/lib/xen (aka LIBEXEC_DIR).
> Check out buildmakevars2file in tools/Rules.mk and the use of it in
> tools/libxl/Makefile -- you probably want to do something similar.

Is the above still valid? I don't see any reason why they should be 
configurable, so I'd agree they should be placed in a header file.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel