Re: [Virtio-fs] [PATCH] virtiofsd: add support to change log level on fly

["Daniel P. Berrangé" <berrange@redhat.com>]

On Thu, Aug 22, 2019 at 06:33:05PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Thu, Aug 22, 2019 at 6:16 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >
> > On Thu, Aug 22, 2019 at 01:07:18PM +0800, piaojun wrote:
> > > On 2019/8/21 23:04, Dr. David Alan Gilbert wrote:
> > > > * Eryu Guan (eguan@linux.alibaba.com) wrote:
> > > >> Introduce a file which contains the target log level, and
> > > >> current_log_level will be reloaded to the target log level on SIGHUP.
> > > >>
> > > >> The file is set to "/run/virtiofsd/log_level.<pid>". Due to
> > > >> sandboxing in virtiofsd, this file can only be read/write via
> > > >> /proc/<pid>/fd/<fd> for now.
> > > >>
> > > >> Example usage:
> > > >>
> > > >>   # view current log level
> > > >>   cat /proc/<pid>/fd/<fd>
> > > >>   # change log level to debug
> > > >>   echo debug > /proc/<pid>/fd/<fd>
> > > >>   kill -HUP <pid>
> > > >>
> > > >> Signed-off-by: Eryu Guan <eguan@linux.alibaba.com>
> > > >
> > > > This looks really complex for something that can only be used for
> > > > changing log levels.
> > >
> > > I have the same opinion with Dave, and I wonder if we have some easier
> > > channel to tell virtiofsd to change log level. And making the channel
> > > more generic is good for the expansibility of virtiofsd.
> >
> > Adding Dan and Marc-André, who have dealt with DBus and QMP.  They can
> > share their experience.
> >
> > The long-term concern we should think about is whether vhost-user
> > backends should have standardized management commands.  They would be
> > defined in docs/interop/vhost-user.rst and all backends implementing a
> > device type would implement a subset of them (plus additional
> > "vendor-specific commands" if they really don't fit into the vhost-user
> > specification).
> >
> > In order to have standard commands we need to choose a standard RPC
> > mechanism.  I'm pretty sure that DBus is a good choice based on my
> > previous discussions with Dan and Marc-André.  It is easy to use from
> > most programming languages and therefore easier to integrate than a
> > custom management channel.
> >
> > Dan and Marc-André: The first commands we would like to offer in
> > virtiofsd are:
> >
> >   get-log-level -> NUM
> >   Return the current logging threshold level.
> >
> >   set-log-level NUM
> >   Set the logging threshold level.
> >
> > If you have time to give a quick overview of how this looks in DBus that
> > would be very helpful.  The virtiofsd code is written in C and currently
> 
> Well that depends how generic we want the interface to be :)
> 
> https://dbus.freedesktop.org/doc/dbus-api-design.html
> 
> service-name: none, or org.qemu.VhostUser1 or even org.qemu.VirtioFs1 ?
> iface: org.qemu.VhostUser1, org.qemu.VirtioFs1 ?
> path: /org/qemu/VhostUser1
> property: "LogLevel", readwrite, string. If you follow systemd/syslogd:
> "this accepts a numerical log level or the well-known syslog(3)
> symbolic names (lowercase): emerg, alert, crit, err, warning, notice,
> info, debug."
> 
> Systemd also has "LogTarget" and a bunch of other options/properties
> that are settable from command line and at runtime with dbus (see man
> page).
> 
> 
> > does not use glib/gobject/gio heavily, so I think we'd prefer a
> > lightweight approach without a lot of gobject boilerplate, if possible.
> 
> If you can afford GIO, I would highly recommend it. Even if you have
> to run it in a seperate thread, that may be easier to maintain.
> 
> Otherwise, sdbus or libdbus.

sdbus is fairly nice if you absolutely must avoid GIO, but per my
other msg I'd strongly recommend just using GIO for C apps.

I would strongly recommend staying well away from libdbus. It is
a very low level API and has some very unfortunate design choices
which especially impact threaded usage. Most widely used DBus impls
have dropped / avoided libdbus these  days and just impl the wire
protocol themselves.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|