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

[Juergen Gross <jgross@xxxxxxxx>]

On 14.11.23 21:57, Julien Grall wrote:
> Hi Juergen,
>
> On 14/11/2023 09:20, Juergen Gross wrote:
> > 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.
> Ah yes. Sorry I got confused.
>
> > 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.
> Do you mean in the following comment:
>
> ## Type: string
> ## Default: Not defined, tracing off
> #
> # Log xenstored messages
> # Only evaluated if XENSTORETYPE is "daemon".
> #XENSTORED_TRACE=[yes|on|1]
>
> I think here we need to remove the "Only...".
Oh, I missed that one. It needs to be considered in the stubdom case, too.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature