Re: [Qemu-devel] [RFC 0/3] virtiofsd: get/set log level via DBus

[Daniel P . Berrangé]

On Thu, Sep 05, 2019 at 04:21:33PM +0100, Stefan Hajnoczi wrote:
> It is likely that virtiofsd will need to support "management commands" for
> reconfiguring it at runtime.  The first use case was proposed by Eryu Guan for
> getting/setting the current log level.
> 
> I promised to try out DBus as the management interface because it has a rich
> feature set and is accessible from most programming languages.  It should be
> able to support all the use cases we come up with.
> 
> This patch series is a prototype that implements the get-log-level and
> set-log-level management commands via DBus.  Use the new virtiofsctl tool to
> talk to a running virtiofsd process:
> 
>   # dbus-run-session ./virtiofsd ...
>   ...
>   Using dbus address 
> unix:abstract=/tmp/dbus-H9WBbpjk3O,guid=0be16acefb868e6025a8737f5d7124d2
>   # export 
> DBUS_SESSION_BUS_ADDRESS=unix:abstract=/tmp/dbus-H9WBbpjk3O,guid=0be16acefb868e6025a8737f5d7124d2
>   # ./virtiofsctl set-log-level err
> 
> Most of the work is done by gdbus-codegen(1).  It generates code for the
> org.qemu.Virtiofsd.xml interface definition.  Our code can use the simple
> virtiofsd_get/set_log_level() APIs and it will make corresponding DBus calls.
> 
> I'm pretty happy with this approach because the code is straightforward.  It
> hasn't even triggered seccomp failures yet :).
> 
> Error handling is a little problematic.  I noticed that virtiofsctl silently
> returns success even if it cannot talk to virtiofsd.  This is due to the code
> generated by gdbus-codegen(1) which has no error reporting :(.  This can be
> solved by writing more low-level GDBus code instead of using the high-level
> generated bindings.

You declared a simple property for the log level, which gets mapped into
GObject properties, and hence hit the error reporting limitations they
have.  DBus at the protocol level can report errors for properties.

For log level settings I'm not worried, but more generally we may well
hit cases where error reporting is functionally critical. So we have a
choice I think

 - Enhance gdbus-codegen so that it can map DBus properties to a different
   impl, with explicit getters/setters, ignoring GObject's properties. This
   would allow for a GError ** parameter to handle/report errors

 - Don't use properties at the DBus protocol level. Instead simply define
   explicit setter & getter methods in DBus. These woudl again bypass
   GObject's properties

Option 2 is probably easier to be honest, especially since in the code
you end up calling what are setters & getters on the client side anyway.

> What do you think about this approach?

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 :|