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

[Juergen Gross <jgross@xxxxxxxx>]

On 08.12.23 17:16, Anthony PERARD wrote:
> 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.
Okay, fair enough.

Lets drop the patch then.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature