From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH 7/7] tools/hotplug: add wrapper to start xenstored Date: Tue, 6 Jan 2015 11:41:55 +0000 Message-ID: <1420544515.28863.148.camel@citrix.com> References: <1418988333-5404-1-git-send-email-olaf@aepfle.de> <1418988333-5404-8-git-send-email-olaf@aepfle.de> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1418988333-5404-8-git-send-email-olaf@aepfle.de> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Olaf Hering Cc: Wei Liu , Stefano Stabellini , Ian Jackson , xen-devel@lists.xen.org, m.a.young@durham.ac.uk List-Id: xen-devel@lists.xenproject.org On Fri, 2014-12-19 at 12:25 +0100, Olaf Hering wrote: > The shell wrapper in xenstored.service does not handle XENSTORE_TRACE. > > Create a separate wrapper script which is used in the sysv runlevel > script and in systemd xenstored.service. It preserves existing > behaviour by handling the XENSTORE_TRACE boolean. It also implements > the handling of XENSTORED_ARGS=. This variable has to be added to > sysconfig/xencommons. Why don't we just drop XENSTORED_* in favour of XENSTORED_ARGS (with an example in the sysconfig file of enabling tracing if you like)? Going to a wrapper script just to make some fairly uncommon debugging option marginally more convenient seems like overkill to me, plus XENSTORED_ARGS would allow for passing other useful options to xenstored. > The wrapper uses exec unconditionally. This works because the > systemd service file passes --no-fork, which has the desired effect > that the binary launched by systemd becomes the final daemon > process. The sysv script does not pass --no-fork, which causes > xenstored to fork internally to return to the caller of the wrapper > script. > > The place of the wrapper is currently LIBEXEC_BIN, it has to be > decided what the final location is supposed to be. IanJ wants it in > "/etc". If we go this route then I agree with Ian J. (/etc/xen/scripts, I suppose). Ian.