Re: [PATCH v2 5/5] tools/xenstored: remove "-N" command line option

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Fri, Dec 08, 2023 at 04:58:52PM +0100, Juergen Gross wrote:
> On 08.12.23 16:46, Andrew Cooper wrote:
> > On 21/11/2023 11:40 am, Juergen Gross wrote:
> > > The "-N" (do not daemonize) command line option is of questionable use:
> > > its sole purpose seems to be to aid debugging of xenstored by making it
> > > easier to start xenstored under gdb, or to see any debug messages
> > > easily.
> > > 
> > > Debug messages can as well be sent to syslog(), while gdb can be
> > > attached to the daemon easily. The only not covered case is an error
> > > while initializing xenstored, but this could be handled e.g. by saving
> > > a core dump, which can be analyzed later.
> > > 
> > > The call of talloc_enable_leak_report_full() done only with "-N"
> > > specified is no longer needed, as the same can be achieved via
> > > "xenstore-control memreport".
> > > 
> > > Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> > 
> > Systemd wants daemons to not fork, because systemd can start them up in
> > a better-prescribed environment than deamonise() can make.
> > 
> > It was a lazy port to system which has caused us not to be using -N in
> > the first place.
> > 
> > So no - I think this option specifically wants to stay, and the systemd
> > integration improved.
> 
> The problem with this approach is that we have some functionality in the
> launch-xenstore script relying on [o]xenstored coming back after having
> forked off the daemon: we are setting the oom-score, which can be done

It's perfectly reasonable to bane the use of "-N" when using
`./launch-xenstore` to start xenstored. It doesn't mean that the option
needs to be removed from xenstored.

> only when knowing the process id.
> 
> So we need a solution for this problem before we can really encourage
> users to use the -N option.
> 
> Please note that setting the oom-score from within xenstored was rejected
> back when I posted a patch in this regard, especially as it is specific to
> Linux. Additionally this would mean we need to add this functionality to
> xenstored AND oxenstored.

There's still a world where -N can be used, and oom-score can be set,
I'm pretty sure that can be done with an hand-crafted "systemd.service"
file. One probably need to have something like "OOMPolicy=" or
"OOMScoreAdjust=" in their service file, and start the `xenstored`
daemon of their choice directly, even with -N as systemd can detect when
the daemon is ready because we use sd_notify() (at least in cxenstored).


So, I think we should keep -N.

Cheers,

-- 
Anthony PERARD