Comment 4 for bug 1818711

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Currently (quite a while actually [3]) the ordering on shutdown is like this:
  libvirtd -> domain -> virt-guest-shutdown.target -> save-restore.service
And as Steve mentioned "save-restore" is to allow to shutdown or save guest state - and that not only via the default libvirt-guests.service but if wanted also via other services, therefore the .target in [3].

As you assumed the KillMode [1] of libvirtd is explicitly set to NOT kill [2] processes as you'd want to start/restart domains (e.g. qemu processes) and networks (e.g. dnsmasq processes) independently and don't want to loose them only since you re-started (or even crashed) the libvirt daemon itself.

Therefore you'd really not want a cgroup cleanup or anything like a signal handler in libvirtd that cleans up all childs - but nobody here asked for it anyway so we are fine. Lets discuss how we could clean it up better on shutdown.

To only take action at the right time we'd need a way to differ between:
- "libvirtd.service is stopping or restarting"
- "the system is shutting down".

That is already done to shutdown/save guests to preserve them in libvirt-guests.service which - before libvirtd is gone. If it does nothing or fails it is considered fine that later on that the systemd killing spree reaps what is left.

To properly remove dnsmasq processes you'd want the equivalent of a "for each net do virsh net-destroy $net". That would need to be:
- on system shutdown only
- could use similar simple mechanisms like libvirt-guests.service
- would need to be ordered to be on shutdown after libvirt-guests.service (so guests still have network when they shutdown/safe)

#1
We could now make this part of libvirt-guests.service actions to do this - after cleaning up guests.
Pro:
- no new service in the jungle of dependencies
Con:
- other components might not assume it will reap networks
- the name isn't indicating it will reap networks
- purpose overloading of the already somewhat awkward libvirt-guests.service script

#2
Or we could add another service that is very much like libvirt-guests.service maybe libvirt-network-shutdown.service - ordered to run on shutdown (would have no real start action) after libvirt-guests.service and to iterate cleaning up all currently active libvirtd networks.
It would link to [3] and actually every other ordering the same way as libvirt-guests.service does.
Pro:
- cleaner separation of guest/network services
- explicit ordering
- ability to enable/disable service individually
Con:
- one more (albeit shutdown only oneshot) service

Once we have had a pre-discussion on this here I could write something patch worthy for an initial RFC. I'd lean towards #2, but want to hear other opinions first.

[1]: https://www.freedesktop.org/software/systemd/man/systemd.kill.html#
[2]: https://libvirt.org/git/?p=libvirt.git;a=commit;h=cb640543c86428a9f472e0c425b65e462d118afa
[3]: https://libvirt.org/git/?p=libvirt.git;a=commit;h=01079727fe29dfeafac751a07e564e0d6bb53389