From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Luis R. Rodriguez" Subject: Re: [PATCH v6 12/13] autoconf: xen: enable explicit preference option for xenstored preference Date: Wed, 2 Jul 2014 22:09:11 +0200 Message-ID: <20140702200911.GY27687@wotan.suse.de> References: <1402622331-4282-1-git-send-email-mcgrof@do-not-panic.com> <1402622331-4282-13-git-send-email-mcgrof@do-not-panic.com> <1404309271.5562.96.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1X2Qq9-0005qs-Ee for xen-devel@lists.xenproject.org; Wed, 02 Jul 2014 20:09:13 +0000 Content-Disposition: inline In-Reply-To: <1404309271.5562.96.camel@kazak.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: Keir Fraser , "Luis R. Rodriguez" , Ian Jackson , Tim Deegan , Jan Beulich , xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org 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