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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2013-01-21 at 18:16 +0000, Thanos Makatos wrote:
> > 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:

Hrm yes so I did. At the time I was imagining you wanted to control
these separately from $(LIBEXEC) for some reason but looking at what
you'd done here I don't see why that should be the case.

> 
> > > 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