From: Marc-André Lureau <marcandre.lureau@redhat.com>

Hi,

This is a small series that allows basic QEMU VM CGroup support with
the help of machined --user:
https://github.com/systemd/systemd/pull/15312

The first few patches are fixes to register dbus and slirp-helper
correctly with the VM cgroup.

A few changes are done to the machined support, adding session
support, and registering the VM to get a systemd scope cgroup under
user machine.slice.

Marc-André Lureau (15):
  slirp: leave the dbus daemon running on error
  slirp: add helper to VM cgroup
  qemu-dbus: prevent double start of the bus daemon
  qemu-dbus: remove unused variable
  qemu-dbus: register DBus bus to the VM cgroup
  qemu-cgroup: remove unnecessary include
  dbus: rename virDBusIs* -> virDBusSystemIs*
  dbus: add virDBusIsService{Enabled,Registered} with @conn argument
  systemd: check org.freedesktop.machine1 registration
  systemd: only check for activitable machine1 service
  systemd: register machines against session service
  cgroup: return directly if there is nothing to remove
  qemu-cgroup: register ext devices when cpu/cpuset controller are
    missing
  qemu: create cgroup regardless of controller support
  qemu-cgroup: drop the need for privileges to use cgroup

 src/qemu/qemu_cgroup.c        | 13 -------
 src/qemu/qemu_dbus.c          | 29 +++++++++++++-
 src/qemu/qemu_dbus.h          |  3 ++
 src/qemu/qemu_extdevice.c     | 12 ++++++
 src/qemu/qemu_process.c       |  6 +--
 src/qemu/qemu_slirp.c         | 16 +++++++-
 src/qemu/qemu_slirp.h         |  3 ++
 src/util/vircgroup.c          |  5 +++
 src/util/vircgroupv2devices.c |  7 ++--
 src/util/virdbus.c            | 71 ++++++++++++++++++++++++++---------
 src/util/virdbus.h            |  7 +++-
 src/util/virfirewalld.c       |  2 +-
 src/util/virsystemd.c         | 27 +++++--------
 tests/virsystemdtest.c        | 35 -----------------
 14 files changed, 140 insertions(+), 96 deletions(-)

-- 
2.26.0.rc2.42.g98cedd0233

On Mon, Apr 06, 2020 at 11:26:57PM +0200, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Hi,
> 
> This is a small series that allows basic QEMU VM CGroup support with
> the help of machined --user:
> https://github.com/systemd/systemd/pull/15312
> 
> The first few patches are fixes to register dbus and slirp-helper
> correctly with the VM cgroup.
> 
> A few changes are done to the machined support, adding session
> support, and registering the VM to get a systemd scope cgroup under
> user machine.slice.

Hi,

Before we start with anything I would like to know what is the
motivation behind having CGroup support for session VMs?

From the systemd pull request it looks like you would like to have
session VMs under the /sys/fs/cgroup/machine.slice which is completely
wrong as we should not mix system and session VMs under the same slice.

In addition it would not work because because you would use session
D-Bus which would start machined under user running session VM and that
user will not have permissions to do anything with the system
machine.slice.  If a regular user wants to do anything with cgroups
delegation has to be used and obviously we cannot delegate the system
machine.slice, it would have to live in a different location and since
the QEMU process is running under the specific user it would have to
live within /sys/fs/cgroup/user.slice/user-1000.slice/user@1000.service/
where by default only memory and pids controllers are available.
Delegation would have to be set in order to get other controllers as
well and all of this would work only if cgroups v2 are used.

Pavel

Hi

On Tue, Apr 7, 2020 at 10:55 AM Pavel Hrdina <phrdina@redhat.com> wrote:
>
> On Mon, Apr 06, 2020 at 11:26:57PM +0200, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Hi,
> >
> > This is a small series that allows basic QEMU VM CGroup support with
> > the help of machined --user:
> > https://github.com/systemd/systemd/pull/15312
> >
> > The first few patches are fixes to register dbus and slirp-helper
> > correctly with the VM cgroup.
> >
> > A few changes are done to the machined support, adding session
> > support, and registering the VM to get a systemd scope cgroup under
> > user machine.slice.
>
> Hi,
>
> Before we start with anything I would like to know what is the
> motivation behind having CGroup support for session VMs?

My initial motivation was to have a way to group VM processes and kill
them altogether, because I tend to have a lot of them around after a
while.

Given that systemd --user is very capable and based on
https://www.freedesktop.org/wiki/Software/systemd/writing-vm-managers/,
I thought that was probably the way to go.

>
> From the systemd pull request it looks like you would like to have
> session VMs under the /sys/fs/cgroup/machine.slice which is completely
> wrong as we should not mix system and session VMs under the same slice.

No, it is under user.slice, ex with this series:

   CGroup: /user.slice/user-1000.slice/user@1000.service
           ├─machine.slice
           │ └─machine-qemu\x2delmarco\x2d1\x2dfedora.scope
           │   ├─24714 /usr/bin/swtpm socket --daemon --ctrl
type=unixio,path=/run/user/1000/libvirt/qemu/run/swtpm/1-fedora-swtpm.sock,mode=0600
--tpmstate dir=/home/elmarco/.config/libvirt/qemu/swtpm/053f84e7>
           │   ├─24716 /usr/bin/dbus-daemon
--config-file=/run/user/1000/libvirt/qemu/run/dbus/1-fedora-dbus.conf
           │   ├─24719
/home/elmarco/src/libslirp-rs/target/debug/libslirp-helper --fd=27
--dbus-id=slirp-52:54:00:9c:bb:6c
--dbus-address=unix:path=/run/user/1000/libvirt/qemu/run/dbus/1-fedora-dbus.sock
--exi>
           │   ├─24722 /usr/bin/qemu-system-x86_64 -name
guest=fedora,debug-threads=on -S -object
secret,id=masterKey0,format=raw,file=/home/elmarco/.config/libvirt/qemu/lib/domain-1-fedora/master-key.aes
-obje>
           │   └─emulator

>
> In addition it would not work because because you would use session
> D-Bus which would start machined under user running session VM and that
> user will not have permissions to do anything with the system
> machine.slice.  If a regular user wants to do anything with cgroups
> delegation has to be used and obviously we cannot delegate the system
> machine.slice, it would have to live in a different location and since
> the QEMU process is running under the specific user it would have to
> live within /sys/fs/cgroup/user.slice/user-1000.slice/user@1000.service/
> where by default only memory and pids controllers are available.
> Delegation would have to be set in order to get other controllers as
> well and all of this would work only if cgroups v2 are used.

I thought delegation was required too, but I can't see any "Delegate="
in my user machine cgroup tree. (using systemctl --user show - note
that /machine.slice doesn't have Delegate set either)

But you can see that basic process management works fine with the
systemd series proposed.

Yes, this is certainly cgroups v2 only.

thanks

On Tue, Apr 07, 2020 at 12:48:26PM +0200, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Apr 7, 2020 at 10:55 AM Pavel Hrdina <phrdina@redhat.com> wrote:
> >
> > On Mon, Apr 06, 2020 at 11:26:57PM +0200, marcandre.lureau@redhat.com wrote:
> > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > >
> > > Hi,
> > >
> > > This is a small series that allows basic QEMU VM CGroup support with
> > > the help of machined --user:
> > > https://github.com/systemd/systemd/pull/15312
> > >
> > > The first few patches are fixes to register dbus and slirp-helper
> > > correctly with the VM cgroup.
> > >
> > > A few changes are done to the machined support, adding session
> > > support, and registering the VM to get a systemd scope cgroup under
> > > user machine.slice.
> >
> > Hi,
> >
> > Before we start with anything I would like to know what is the
> > motivation behind having CGroup support for session VMs?
> 
> My initial motivation was to have a way to group VM processes and kill
> them altogether, because I tend to have a lot of them around after a
> while.
> 
> Given that systemd --user is very capable and based on
> https://www.freedesktop.org/wiki/Software/systemd/writing-vm-managers/,
> I thought that was probably the way to go.

Yes, it makes sense. It also should help us close a feature gap
by enabling many of our resource tuning APIs to now work in session
mode, since we can now use cgroups tuning.

> > From the systemd pull request it looks like you would like to have
> > session VMs under the /sys/fs/cgroup/machine.slice which is completely
> > wrong as we should not mix system and session VMs under the same slice.
> 
> No, it is under user.slice, ex with this series:
> 
>    CGroup: /user.slice/user-1000.slice/user@1000.service
>            ├─machine.slice
>            │ └─machine-qemu\x2delmarco\x2d1\x2dfedora.scope
>            │   ├─24714 /usr/bin/swtpm socket --daemon --ctrl
> type=unixio,path=/run/user/1000/libvirt/qemu/run/swtpm/1-fedora-swtpm.sock,mode=0600
> --tpmstate dir=/home/elmarco/.config/libvirt/qemu/swtpm/053f84e7>
>            │   ├─24716 /usr/bin/dbus-daemon
> --config-file=/run/user/1000/libvirt/qemu/run/dbus/1-fedora-dbus.conf
>            │   ├─24719
> /home/elmarco/src/libslirp-rs/target/debug/libslirp-helper --fd=27
> --dbus-id=slirp-52:54:00:9c:bb:6c
> --dbus-address=unix:path=/run/user/1000/libvirt/qemu/run/dbus/1-fedora-dbus.sock
> --exi>
>            │   ├─24722 /usr/bin/qemu-system-x86_64 -name
> guest=fedora,debug-threads=on -S -object
> secret,id=masterKey0,format=raw,file=/home/elmarco/.config/libvirt/qemu/lib/domain-1-fedora/master-key.aes
> -obje>
>            │   └─emulator

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

On Tue, Apr 07, 2020 at 12:48:26PM +0200, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Apr 7, 2020 at 10:55 AM Pavel Hrdina <phrdina@redhat.com> wrote:
> >
> > On Mon, Apr 06, 2020 at 11:26:57PM +0200, marcandre.lureau@redhat.com wrote:
> > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > >
> > > Hi,
> > >
> > > This is a small series that allows basic QEMU VM CGroup support with
> > > the help of machined --user:
> > > https://github.com/systemd/systemd/pull/15312
> > >
> > > The first few patches are fixes to register dbus and slirp-helper
> > > correctly with the VM cgroup.
> > >
> > > A few changes are done to the machined support, adding session
> > > support, and registering the VM to get a systemd scope cgroup under
> > > user machine.slice.
> >
> > Hi,
> >
> > Before we start with anything I would like to know what is the
> > motivation behind having CGroup support for session VMs?
> 
> My initial motivation was to have a way to group VM processes and kill
> them altogether, because I tend to have a lot of them around after a
> while.
> 
> Given that systemd --user is very capable and based on
> https://www.freedesktop.org/wiki/Software/systemd/writing-vm-managers/,
> I thought that was probably the way to go.
> 
> >
> > From the systemd pull request it looks like you would like to have
> > session VMs under the /sys/fs/cgroup/machine.slice which is completely
> > wrong as we should not mix system and session VMs under the same slice.
> 
> No, it is under user.slice, ex with this series:
> 
>    CGroup: /user.slice/user-1000.slice/user@1000.service
>            ├─machine.slice
>            │ └─machine-qemu\x2delmarco\x2d1\x2dfedora.scope
>            │   ├─24714 /usr/bin/swtpm socket --daemon --ctrl
> type=unixio,path=/run/user/1000/libvirt/qemu/run/swtpm/1-fedora-swtpm.sock,mode=0600
> --tpmstate dir=/home/elmarco/.config/libvirt/qemu/swtpm/053f84e7>
>            │   ├─24716 /usr/bin/dbus-daemon
> --config-file=/run/user/1000/libvirt/qemu/run/dbus/1-fedora-dbus.conf
>            │   ├─24719
> /home/elmarco/src/libslirp-rs/target/debug/libslirp-helper --fd=27
> --dbus-id=slirp-52:54:00:9c:bb:6c
> --dbus-address=unix:path=/run/user/1000/libvirt/qemu/run/dbus/1-fedora-dbus.sock
> --exi>
>            │   ├─24722 /usr/bin/qemu-system-x86_64 -name
> guest=fedora,debug-threads=on -S -object
> secret,id=masterKey0,format=raw,file=/home/elmarco/.config/libvirt/qemu/lib/domain-1-fedora/master-key.aes
> -obje>
>            │   └─emulator

OK, that sounds good, I did no realize it works like that if the
machine.slice file is placed under the user directory.

> > In addition it would not work because because you would use session
> > D-Bus which would start machined under user running session VM and that
> > user will not have permissions to do anything with the system
> > machine.slice.  If a regular user wants to do anything with cgroups
> > delegation has to be used and obviously we cannot delegate the system
> > machine.slice, it would have to live in a different location and since
> > the QEMU process is running under the specific user it would have to
> > live within /sys/fs/cgroup/user.slice/user-1000.slice/user@1000.service/
> > where by default only memory and pids controllers are available.
> > Delegation would have to be set in order to get other controllers as
> > well and all of this would work only if cgroups v2 are used.
> 
> I thought delegation was required too, but I can't see any "Delegate="
> in my user machine cgroup tree. (using systemctl --user show - note
> that /machine.slice doesn't have Delegate set either)
> 
> But you can see that basic process management works fine with the
> systemd series proposed.

systemd does partial delegation for the user cgroup so the user is able
to use memory and pids controllers by default.  If user needs to use
other controllers as well the administrator has to set Delegate=yes
using for example 'systemctl edit user@1000.service'.

We would have to document this as a prerequisite to be able to use other
controllers as well.

> Yes, this is certainly cgroups v2 only.

OK, I wanted to be sure that this targets cgroups v2 only.

In general the idea sounds good and it would allow users to restrict VMs
to not consume all the resources assigned to themselves and also to get
some VM statistics that are gathered from cgroups.

Most of the patches in this series looks like unrelated fixes of our
current code so I would suggest posting them in separately from the
session cgroup support.

Pavel