Re: [PATCH 5/5] tools/hotplug: support XENSTORED_TRACE in systemd

[Olaf Hering <olaf@aepfle.de>]

On Tue, Dec 09, Ian Jackson wrote:

> Olaf Hering writes ("Re: [PATCH 5/5] tools/hotplug: support XENSTORED_TRACE in systemd"):
> > On Fri, Dec 05, Ian Jackson wrote:
> > > I think the only way to make this work properly is to factor the
> > > necessary parts out of init.d/xencommons into a new script which can
> > > be used by both xencommons and systemd.  I'm not sure such a patch
> > > would be appropriate for 4.5 at this stage.
> > 
> > I came up with this, it appears to work in my testing. Will do more
> > testing later today.
> 
> Thanks.  I think this is going in roughly the right direction.
> 
> But: I think the script is rather over-engineered, and that it ought
> to be in /etc.

Why should the wrapper be in /etc?!
xendomains isnt in /etc either.

> > +	$(INSTALL_PROG) $(XENSTORED_LIBEXEC) $(DESTDIR)$(LIBEXEC_BIN)
> 
> Sysadmins might want to edit the script to do something we haven't
> thought of.  So it should be in /etc where they can do so.

So they should mail here if they find substantial functionality missing.
Until then they continue to not enable xencommons and run their own
startup script.

> I don't think this script wants to contain an option parser!

How should it handle exec vs. no-exec? Just a single yes/no knob, so
essentially sysv vs systemd?

> The systemd unit doesn't currently contain anything messing about with
> xenstore-read to detect when xenstored is working.  Is that a bug that
> was previously in the systemd unit, or is it a mistake in your patch
> that this is added here ?

No idea how long it takes to have a functional xenstored after running
it. Perhaps the forking has some overhead and it returns earlier than it
can process requests. If thats the reason why the loop exists in the
sysv runlevel script then that loop should be used only without --no-fork.


Olaf