Re: [PATCH v6 12/13] autoconf: xen: enable explicit preference option for xenstored preference

["Luis R. Rodriguez" <mcgrof@suse.com>]

On Wed, Jul 02, 2014 at 02:54:31PM +0100, Ian Campbell wrote:
> On Thu, 2014-06-12 at 18:18 -0700, Luis R. Rodriguez wrote:
> > Since the xenstored preference
> > is explicit now and since we require configure substitutions for it we
> > make use of the AX_XEN_EXPAND_CONFIG() helpers as otherwise substitution
> > for SBINDIR is not propagated from the top level configuration.
> 
> I'm afraid I don't understand what is going on here. Why is SBINDIR not
> propagated?

Its saying that we need AX_XEN_EXPAND_CONFIG() in order to do substitutions
for variables that include SBINDIR in this case for the xenstore full path.

> > Since we are now parsing an entry within Paths.mk.in on tools we let
> > the move the parsing of the file to be the tool's configure.
>
> I can't parse this.

I meant to clarify that since we now have to do a substitution on tools
we move the calls to AX_XEN_EXPAND_CONFIG() there. This patch should have
also removed the AX_XEN_EXPAND_CONFIG() from the top level configure.ac.

After testing this though and removing AX_XEN_EXPAND_CONFIG() from configure.ac
I do see though that substitution doesn't work, it only works if we leave
AX_XEN_EXPAND_CONFIG() on both places. This is the issue I mentioned that
I ran into and you had proposed we could try to figure out a fix if
this was happening. Well it is. Any ideas ? Or are you OK to live with
calling AX_XEN_EXPAND_CONFIG() twice, once on configure.ac and another
on tools/configure.ac ?

> > diff --git a/m4/xenstored.m4 b/m4/xenstored.m4
> > new file mode 100644
> > index 0000000..5a4d891

<-- snip -->

> > +AC_DEFUN([AX_XENSTORE_SET], [
> > +	XENSTORE=$xenstore
> > +	AC_SUBST(XENSTORE)
> 
> This isn't used any more.

Removed this last AC_SUBST() line.

> > +
> > +	AS_IF([test "x$XENSTORED" = "x"], [
> > +		XENSTORED=$xenstored
> > +	])
> > +	AC_SUBST(XENSTORED)
> > +
> > +	AS_IF([test "x$XENSTORE" != "xxenstored" && test "x$XENSTORE" != "xoxenstored"],
> > +		[AC_MSG_ERROR([Invalid xenstore: $XENSTORE])])
> 
> You'll have already bailed from the previous incantation before you ever
> hit this, no?

Yes true, nuked this superfluous check.

> > diff --git a/tools/hotplug/Linux/init.d/sysconfig.xencommons b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
> > similarity index 64%
> > rename from tools/hotplug/Linux/init.d/sysconfig.xencommons
> > rename to tools/hotplug/Linux/init.d/sysconfig.xencommons.in
> > index 25f7f00..d7d09ff 100644
> > --- a/tools/hotplug/Linux/init.d/sysconfig.xencommons
> > +++ b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
> > @@ -8,8 +8,17 @@
> >  ## Type: string
> >  ## Default: xenstored
> >  #
> > -# Select xenstored implementation
> > -#XENSTORED=[oxenstored|xenstored]
> > +# Select xenstore implementation, this can be either
> > +# of these below. If using systemd its preferred you
> 
> "it's", and I think you are missing a "that" before you.

Amended, thanks.

  Luis