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

[Juergen Gross <jgross@xxxxxxxx>]

On 15.11.23 09:31, Edwin Torok wrote:
> > On 14 Nov 2023, at 22:11, Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx> wrote:
> >
> > On 13/11/2023 12:43 pm, 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>
> > CC Edvin.
> >
> > There's a patch out to specifically use this option (under systemd) to
> > fix a bug we found.
> >
> > I can't recall the details, but I seem to recall it wasn't specific to
> > oxenstored.
>
> The patch is here 
> https://lore.kernel.org/xen-devel/ECFA15A7-9DC8-4476-8D0B-44A6D12192D6@xxxxxxxxx/
>
> It is about not losing stderr when run under systemd (well you can use syslog 
> but what if your connection to syslog fails or you fail before getting to the 
> point of parsing which syslog to use).
> Alternatively we could have a "don't redirect stderr to /dev/null" flag,
> but such redirections are usually expected when daemonizing.
>
> It'd be good if the --no-fork flag could be retained for C xenstored, currently 
> there is a CI failure on a non-systemd system that I'm trying to fix (a bit 
> blindly because I don't have such a system myself), but from my testing the 
> flag does work on a systemd system with C xenstored.
>
> The patch is motivated by having some undebuggable issues in oxenstored where 
> it just exits without writing any messages and without dumping core, so by 
> retaining the stderr path we should have an output of last resort in case 
> something goes so wrong that the syslog error handler cannot function.
Using the --no-fork as in your patch would mean that setting the oom_score in
the lauch_xenstore script wouldn't be executed any longer, right?


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature