Re: [PATCH v2 28/29] tools/xenstored: support complete log capabilities in stubdom

[Juergen Gross <jgross@xxxxxxxx>]

On 14.11.23 10:05, Julien Grall wrote:
> Hi,
>
> On 14/11/2023 06:45, Juergen Gross wrote:
> > On 13.11.23 23:40, Julien Grall wrote:
> > > Hi Juergen,
> > >
> > > On 10/11/2023 16:08, Juergen Gross wrote:
> > > > With 9pfs being fully available in Xenstore-stubdom now, there is no
> > > > reason to not fully support all logging capabilities in stubdom.
> > > >
> > > > Open the logfile on stubdom only after the 9pfs file system has been
> > > > mounted.
> > > >
> > > > Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> > > > Reviewed-by: Jason Andryuk <jandryuk@xxxxxxxxx>
> > > > ---
> > > >   tools/hotplug/Linux/launch-xenstore.in |  1 +
> > > >   tools/xenstored/control.c              | 30 +++++++++++++-------------
> > > >   tools/xenstored/minios.c               |  3 +++
> > > >   3 files changed, 19 insertions(+), 15 deletions(-)
> > > >
> > > > diff --git a/tools/hotplug/Linux/launch-xenstore.in 
> > > > b/tools/hotplug/Linux/launch-xenstore.in
> > > > index e854ca1eb8..da4eeca7c5 100644
> > > > --- a/tools/hotplug/Linux/launch-xenstore.in
> > > > +++ b/tools/hotplug/Linux/launch-xenstore.in
> > > > @@ -98,6 +98,7 @@ test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && . 
> > > > @CONFIG_DIR@/@CONFIG_LEAF
> > > >       [ -z "$XENSTORE_DOMAIN_SIZE" ] && XENSTORE_DOMAIN_SIZE=8
> > > >       XENSTORE_DOMAIN_ARGS="$XENSTORE_DOMAIN_ARGS --memory 
> > > > $XENSTORE_DOMAIN_SIZE"
> > > >       [ -z "$XENSTORE_MAX_DOMAIN_SIZE" ] || 
> > > > XENSTORE_DOMAIN_ARGS="$XENSTORE_DOMAIN_ARGS --maxmem $XENSTORE_MAX_DOMAIN_SIZE"
> > > > +    [ -z "$XENSTORED_TRACE" ] || 
> > > > XENSTORE_DOMAIN_ARGS="$XENSTORE_DOMAIN_ARGS -T xenstored-trace.log"
> > > I am probably missing something, but any reason to not use 
> > > @XEN_LOG_DIR@/xenstored-trace.log as we do for xenstored?
> > Yes. Stubdom has no access to XEN_LOG_DIR.
> Ok. This restriction is not that obvious... The documentation in 
> sysconfig.xencommons.in implies that it will be written in XEN_LOG_DIR:
> ## Type: string
> ## Default: ""
> #
> # Additional commandline arguments to start xenstored,
> # like "--trace-file @XEN_LOG_DIR@/xenstored-trace.log"
> # See "@sbindir@/xenstored --help" for possible options.
> # Only evaluated if XENSTORETYPE is "daemon".
> XENSTORED_ARGS=
>
> Also, we are saying this is only supported when Xenstored is daemonized. So I 
> think there are some documentation update necessary in this patch.
This is for the daemon. And the documentation example here is using an
absolute filename, so this is very clear where the trace file will be written.

For stubdom a related parameter "XENSTORE_DOMAIN_ARGS" is available. I can add
a sentence to the explanation of that parameter describing setting of a
possible trace file specification.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature