[PATCH] xenstored: do not redirect stderr to /dev/null

Edwin Török posted 1 patch 6 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/87fbae122fd2d75852026d621358031c72c9a36d.1698227069.git.edwin.torok@cloud.com
tools/hotplug/Linux/init.d/sysconfig.xencommons.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] xenstored: do not redirect stderr to /dev/null
Posted by Edwin Török 6 months, 1 week ago
From: Edwin Török <edwin.torok@cloud.com>

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 <edwin.torok@cloud.com>
---
 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
 
 ## Type: string
 ## Default: Not defined, tracing off
-- 
2.41.0


Re: [PATCH] xenstored: do not redirect stderr to /dev/null
Posted by Edwin Torok 6 months ago

> On 25 Oct 2023, at 14:50, Edwin Török <edvin.torok@citrix.com> wrote:
> 
> From: Edwin Török <edwin.torok@cloud.com>
> 
> 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 <edwin.torok@cloud.com>
> ---
> 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