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

Stefan Hajnoczi posted 3 patches 4 years, 6 months ago
Failed in applying to current master (apply log)
configure                                |   7 +
Makefile                                 |  16 +++
Makefile.objs                            |   1 +
contrib/virtiofsd/Makefile.objs          |  10 +-
contrib/virtiofsd/dbus.h                 |   9 ++
contrib/virtiofsd/dbus.c                 | 162 +++++++++++++++++++++++
contrib/virtiofsd/passthrough_ll.c       |   8 +-
contrib/virtiofsd/virtiofsctl.c          |  55 ++++++++
.gitignore                               |   1 +
contrib/virtiofsd/org.qemu.Virtiofsd.xml |   7 +
10 files changed, 274 insertions(+), 2 deletions(-)
create mode 100644 contrib/virtiofsd/dbus.h
create mode 100644 contrib/virtiofsd/dbus.c
create mode 100644 contrib/virtiofsd/virtiofsctl.c
create mode 100644 contrib/virtiofsd/org.qemu.Virtiofsd.xml
[Qemu-devel] [RFC 0/3] virtiofsd: get/set log level via DBus
Posted by Stefan Hajnoczi 4 years, 6 months ago
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.

What do you think about this approach?

Stefan Hajnoczi (3):
  virtiofsd: add org.qemu.Virtiofsd interface
  virtiofsd: add DBus server to handle log level changes
  virtiofsd: add virtiofsctl command-line management tool

 configure                                |   7 +
 Makefile                                 |  16 +++
 Makefile.objs                            |   1 +
 contrib/virtiofsd/Makefile.objs          |  10 +-
 contrib/virtiofsd/dbus.h                 |   9 ++
 contrib/virtiofsd/dbus.c                 | 162 +++++++++++++++++++++++
 contrib/virtiofsd/passthrough_ll.c       |   8 +-
 contrib/virtiofsd/virtiofsctl.c          |  55 ++++++++
 .gitignore                               |   1 +
 contrib/virtiofsd/org.qemu.Virtiofsd.xml |   7 +
 10 files changed, 274 insertions(+), 2 deletions(-)
 create mode 100644 contrib/virtiofsd/dbus.h
 create mode 100644 contrib/virtiofsd/dbus.c
 create mode 100644 contrib/virtiofsd/virtiofsctl.c
 create mode 100644 contrib/virtiofsd/org.qemu.Virtiofsd.xml

-- 
2.21.0


Re: [Qemu-devel] [RFC 0/3] virtiofsd: get/set log level via DBus
Posted by Daniel P. Berrangé 4 years, 6 months ago
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 :|

Re: [Qemu-devel] [RFC 0/3] virtiofsd: get/set log level via DBus
Posted by Stefan Hajnoczi 4 years, 6 months ago
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.
> 
> What do you think about this approach?

Hi Eryu Guan,
Have you had a chance to try these patches?  Do they meet your
requirements for being able to get/set the log level at runtime?

Stefan
Re: [Qemu-devel] [RFC 0/3] virtiofsd: get/set log level via DBus
Posted by Dr. David Alan Gilbert 4 years, 6 months ago
* Stefan Hajnoczi (stefanha@redhat.com) 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 :).

Yes it's less complex than I'd worried.
Now, I do think we've got to think about how qemu in general is going to
use dbus as people were discussing it, because then we have to think
what the security aspects are - do we need to look at some calls only
available to some clients etc.

Dave

> 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.
> 
> What do you think about this approach?
> 
> Stefan Hajnoczi (3):
>   virtiofsd: add org.qemu.Virtiofsd interface
>   virtiofsd: add DBus server to handle log level changes
>   virtiofsd: add virtiofsctl command-line management tool
> 
>  configure                                |   7 +
>  Makefile                                 |  16 +++
>  Makefile.objs                            |   1 +
>  contrib/virtiofsd/Makefile.objs          |  10 +-
>  contrib/virtiofsd/dbus.h                 |   9 ++
>  contrib/virtiofsd/dbus.c                 | 162 +++++++++++++++++++++++
>  contrib/virtiofsd/passthrough_ll.c       |   8 +-
>  contrib/virtiofsd/virtiofsctl.c          |  55 ++++++++
>  .gitignore                               |   1 +
>  contrib/virtiofsd/org.qemu.Virtiofsd.xml |   7 +
>  10 files changed, 274 insertions(+), 2 deletions(-)
>  create mode 100644 contrib/virtiofsd/dbus.h
>  create mode 100644 contrib/virtiofsd/dbus.c
>  create mode 100644 contrib/virtiofsd/virtiofsctl.c
>  create mode 100644 contrib/virtiofsd/org.qemu.Virtiofsd.xml
> 
> -- 
> 2.21.0
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [RFC 0/3] virtiofsd: get/set log level via DBus
Posted by Stefan Hajnoczi 4 years, 6 months ago
On Thu, Sep 05, 2019 at 06:40:21PM +0100, Dr. David Alan Gilbert wrote:
> * Stefan Hajnoczi (stefanha@redhat.com) 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 :).
> 
> Yes it's less complex than I'd worried.
> Now, I do think we've got to think about how qemu in general is going to
> use dbus as people were discussing it, because then we have to think
> what the security aspects are - do we need to look at some calls only
> available to some clients etc.

The approach I took in this patch series is to launch a session bus just
for this virtiofsd.  The abstract socket unix(7) namespace used by GDBus
by default does not offer any security.  I think any process on the host
can connect to it, regardless of uid/gid.

A path like unix:path=/tmp/foo would allow us to use UNIX Domain Socket
permissions as the main security mechanism.  I'm not enthusiastic about
using SELinux or some kind of DBus-specific policy language if we can
avoid it because it's complex and obscure.

Stefan
Re: [Qemu-devel] [RFC 0/3] virtiofsd: get/set log level via DBus
Posted by Dr. David Alan Gilbert 4 years, 6 months ago
* Stefan Hajnoczi (stefanha@redhat.com) wrote:
> On Thu, Sep 05, 2019 at 06:40:21PM +0100, Dr. David Alan Gilbert wrote:
> > * Stefan Hajnoczi (stefanha@redhat.com) 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 :).
> > 
> > Yes it's less complex than I'd worried.
> > Now, I do think we've got to think about how qemu in general is going to
> > use dbus as people were discussing it, because then we have to think
> > what the security aspects are - do we need to look at some calls only
> > available to some clients etc.
> 
> The approach I took in this patch series is to launch a session bus just
> for this virtiofsd.  The abstract socket unix(7) namespace used by GDBus
> by default does not offer any security.  I think any process on the host
> can connect to it, regardless of uid/gid.
> 
> A path like unix:path=/tmp/foo would allow us to use UNIX Domain Socket
> permissions as the main security mechanism.

Yes, that I'm fine with; my worry is that there was talk of much more
complex dbus setups coming out of the qemu world with many things being
connected; and then we have to think about security upfront.

>  I'm not enthusiastic about
> using SELinux or some kind of DBus-specific policy language if we can
> avoid it because it's complex and obscure.

Right.

Dave

> Stefan


--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [RFC 0/3] virtiofsd: get/set log level via DBus
Posted by Daniel P. Berrangé 4 years, 6 months ago
On Fri, Sep 06, 2019 at 11:29:26AM +0100, Stefan Hajnoczi wrote:
> On Thu, Sep 05, 2019 at 06:40:21PM +0100, Dr. David Alan Gilbert wrote:
> > * Stefan Hajnoczi (stefanha@redhat.com) 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 :).
> > 
> > Yes it's less complex than I'd worried.
> > Now, I do think we've got to think about how qemu in general is going to
> > use dbus as people were discussing it, because then we have to think
> > what the security aspects are - do we need to look at some calls only
> > available to some clients etc.
> 
> The approach I took in this patch series is to launch a session bus just
> for this virtiofsd.  The abstract socket unix(7) namespace used by GDBus
> by default does not offer any security.  I think any process on the host
> can connect to it, regardless of uid/gid.

Other users will be able to connect(), but you'll find that the dbus
policy causes their connections to be dropped immediately - even if
they are the root user in fact.

> A path like unix:path=/tmp/foo would allow us to use UNIX Domain Socket
> permissions as the main security mechanism.  I'm not enthusiastic about
> using SELinux or some kind of DBus-specific policy language if we can
> avoid it because it's complex and obscure.

Yep, that just needs you to supply a config file when launching to
specify a desired filesystem path.

I don't think it is an either/or matter - I think we'll  want all
three in general - DAC controls on the socket, and DBus policy and
SELinux policy. DAC controls on the socket alone are not sufficient
if you want to separate each QEMU from each other and they're running
the same UID which is common.

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