xenstored: do not redirect stderr to /dev/null

Commit Message

Edwin Török Oct. 25, 2023, 10:05 a.m. UTC

From: Edwin Török <[e-mail]>

By default stderr gets redirected to /dev/null because oxenstored daemonizes itself.
This must be a left-over from pre-systemd days.

In ee7815f49f ("tools/oxenstored: Set uncaught exception handler") a workaround was added to log exceptions
directly to syslog to cope with standard error being lost.

However it is better to not lose standard error (what if the connection to syslog itself fails, how'd we log that?),
and use the '--no-fork' flag to do that.
This flag is supported by both C and O versions of xenstored.

Both versions also call sd_notify so there is no need for forking.

Leave the default daemonize as is so that xenstored keeps working on non-Linux systems as before.

Signed-off-by: Edwin Török <[e-mail]>
---
 tools/hotplug/Linux/init.d/sysconfig.xencommons.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Edwin Török Oct. 30, 2023, 6:11 p.m. UTC | #1

> On 25 Oct 2023, at 14:50, Edwin Török <[e-mail]> wrote:
> 
> From: Edwin Török <[e-mail]>
> 
> By default stderr gets redirected to /dev/null because oxenstored daemonizes itself.
> This must be a left-over from pre-systemd days.
> 
> In ee7815f49f ("tools/oxenstored: Set uncaught exception handler") a workaround was added to log exceptions
> directly to syslog to cope with standard error being lost.
> 
> However it is better to not lose standard error (what if the connection to syslog itself fails, how'd we log that?),
> and use the '--no-fork' flag to do that.
> This flag is supported by both C and O versions of xenstored.
> 
> Both versions also call sd_notify so there is no need for forking.
> 
> Leave the default daemonize as is so that xenstored keeps working on non-Linux systems as before.
> 
> Signed-off-by: Edwin Török <[e-mail]>
> ---
> tools/hotplug/Linux/init.d/sysconfig.xencommons.in | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
> index 433e4849af..09a1230cee 100644
> --- a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
> +++ b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
> @@ -52,7 +52,7 @@
> # like "--trace-file @XEN_LOG_DIR@/xenstored-trace.log"
> # See "@sbindir@/xenstored --help" for possible options.
> # Only evaluated if XENSTORETYPE is "daemon".
> -XENSTORED_ARGS=
> +XENSTORED_ARGS=--no-fork


I think the CI failure is due to this patch, and it only happens on Linux systems that do not use systemd.
In that case we do need to fork, because that is the only way not to tie up the boot sequence.

I'll try to make '--no-fork' depend on having systemd present, because otherwise I tested both C and O xenstored and they do start up with --no-fork.

Best regards,
--Edwin

Patch

diff --git a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
index 433e4849af..09a1230cee 100644
--- a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
+++ b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in
@@ -52,7 +52,7 @@ 
 # like "--trace-file @XEN_LOG_DIR@/xenstored-trace.log"
 # See "@sbindir@/xenstored --help" for possible options.
 # Only evaluated if XENSTORETYPE is "daemon".
-XENSTORED_ARGS=
+XENSTORED_ARGS=--no-fork
 
 ## Type: string
 ## Default: Not defined, tracing off