[PATCH] libvirt-guests: Fix dependency ordering in service file
After some debugging and discussion with systemd team it turns out we are misusing the ordering in libvirt-guests.service. That happened because we want to support both monolithic and modular daemon setups and on top of that we also want to support socket activation and services without socket activation. Unfortunately this is impossible to express in the unit file because of how transactions are handled in systemd when dependencies are resolved and multiple actions (jobs) are queued. For explanation from Michal Sekletar see comment #7 in the BZ this patch is fixing: https://bugzilla.redhat.com/show_bug.cgi?id=1964855#c7 In order to support all the scenarios this patch also amends the manpages so that users that are changing the default can also read how to correct the dependency ordering in libvirt-guests unit file. Ideally we would also keep the existing configuration during upgrade, but due to our huge support matrix this seems hardly feasible as it could introduce even more problems. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/manpages/libvirtd.rst | 14 ++++++++++++++ docs/manpages/virtlxcd.rst | 14 ++++++++++++++ docs/manpages/virtqemud.rst | 14 ++++++++++++++ docs/manpages/virtvboxd.rst | 14 ++++++++++++++ docs/manpages/virtvzd.rst | 14 ++++++++++++++ docs/manpages/virtxend.rst | 14 ++++++++++++++ tools/libvirt-guests.service.in | 6 ------ 7 files changed, 84 insertions(+), 6 deletions(-) diff --git a/docs/manpages/libvirtd.rst b/docs/manpages/libvirtd.rst index ee72f0838221..1347b9b21042 100644 --- a/docs/manpages/libvirtd.rst +++ b/docs/manpages/libvirtd.rst @@ -79,6 +79,20 @@ unit files must be masked: $ systemctl mask libvirtd.socket libvirtd-ro.socket \ libvirtd-admin.socket libvirtd-tls.socket libvirtd-tcp.socket +If using libvirt-guests service then the ordering for that service needs to be +adapted so that it is ordered after the service unit instead of the socket unit. +Since dependencies and ordering cannot be changed with drop-in overrides, the +whole libvirt-guests unit file needs to be changed. In order to preserve such +change copy the installed ``/usr/lib/systemd/system/libvirt-guests.service`` to +``/etc/systemd/system/libvirt-guests.service`` and make the change there, +specifically make sure the ``After=`` ordering mentions ``libvirtd.service`` and +not ``libvirtd.socket``: + +:: + + [Unit] + After=libvirtd.service + OPTIONS ======= diff --git a/docs/manpages/virtlxcd.rst b/docs/manpages/virtlxcd.rst index 2e9d8fd14bbb..aebc8adb5822 100644 --- a/docs/manpages/virtlxcd.rst +++ b/docs/manpages/virtlxcd.rst @@ -60,6 +60,20 @@ unit files must be masked: $ systemctl mask virtlxcd.socket virtlxcd-ro.socket \ virtlxcd-admin.socket +If using libvirt-guests service then the ordering for that service needs to be +adapted so that it is ordered after the service unit instead of the socket unit. +Since dependencies and ordering cannot be changed with drop-in overrides, the +whole libvirt-guests unit file needs to be changed. In order to preserve such +change copy the installed ``/usr/lib/systemd/system/libvirt-guests.service`` to +``/etc/systemd/system/libvirt-guests.service`` and make the change there, +specifically make sure the ``After=`` ordering mentions ``virtlxcd.service`` and +not ``virtlxcd.socket``: + +:: + + [Unit] + After=virtlxcd.service + OPTIONS ======= diff --git a/docs/manpages/virtqemud.rst b/docs/manpages/virtqemud.rst index ea8d6e3105db..fa9a6ce3755c 100644 --- a/docs/manpages/virtqemud.rst +++ b/docs/manpages/virtqemud.rst @@ -60,6 +60,20 @@ unit files must be masked: $ systemctl mask virtqemud.socket virtqemud-ro.socket \ virtqemud-admin.socket +If using libvirt-guests service then the ordering for that service needs to be +adapted so that it is ordered after the service unit instead of the socket unit. +Since dependencies and ordering cannot be changed with drop-in overrides, the +whole libvirt-guests unit file needs to be changed. In order to preserve such +change copy the installed ``/usr/lib/systemd/system/libvirt-guests.service`` to +``/etc/systemd/system/libvirt-guests.service`` and make the change there, +specifically make sure the ``After=`` ordering mentions ``virtqemud.service`` and +not ``virtqemud.socket``: + +:: + + [Unit] + After=virtqemud.service + OPTIONS ======= diff --git a/docs/manpages/virtvboxd.rst b/docs/manpages/virtvboxd.rst index d7339d99f22b..f90de3451d8d 100644 --- a/docs/manpages/virtvboxd.rst +++ b/docs/manpages/virtvboxd.rst @@ -58,6 +58,20 @@ unit files must be masked: $ systemctl mask virtvboxd.socket virtvboxd-ro.socket \ virtvboxd-admin.socket +If using libvirt-guests service then the ordering for that service needs to be +adapted so that it is ordered after the service unit instead of the socket unit. +Since dependencies and ordering cannot be changed with drop-in overrides, the +whole libvirt-guests unit file needs to be changed. In order to preserve such +change copy the installed ``/usr/lib/systemd/system/libvirt-guests.service`` to +``/etc/systemd/system/libvirt-guests.service`` and make the change there, +specifically make sure the ``After=`` ordering mentions ``virtvboxd.service`` and +not ``virtvboxd.socket``: + +:: + + [Unit] + After=virtvboxd.service + OPTIONS ======= diff --git a/docs/manpages/virtvzd.rst b/docs/manpages/virtvzd.rst index 42dfa263e450..970719aac1d5 100644 --- a/docs/manpages/virtvzd.rst +++ b/docs/manpages/virtvzd.rst @@ -60,6 +60,20 @@ unit files must be masked: $ systemctl mask virtvzd.socket virtvzd-ro.socket \ virtvzd-admin.socket +If using libvirt-guests service then the ordering for that service needs to be +adapted so that it is ordered after the service unit instead of the socket unit. +Since dependencies and ordering cannot be changed with drop-in overrides, the +whole libvirt-guests unit file needs to be changed. In order to preserve such +change copy the installed ``/usr/lib/systemd/system/libvirt-guests.service`` to +``/etc/systemd/system/libvirt-guests.service`` and make the change there, +specifically make sure the ``After=`` ordering mentions ``virtvzd.service`` and +not ``virtvzd.socket``: + +:: + + [Unit] + After=virtvzd.service + OPTIONS ======= diff --git a/docs/manpages/virtxend.rst b/docs/manpages/virtxend.rst index b08346b489d2..cf7685ecc0e6 100644 --- a/docs/manpages/virtxend.rst +++ b/docs/manpages/virtxend.rst @@ -60,6 +60,20 @@ unit files must be masked: $ systemctl mask virtxend.socket virtxend-ro.socket \ virtxend-admin.socket +If using libvirt-guests service then the ordering for that service needs to be +adapted so that it is ordered after the service unit instead of the socket unit. +Since dependencies and ordering cannot be changed with drop-in overrides, the +whole libvirt-guests unit file needs to be changed. In order to preserve such +change copy the installed ``/usr/lib/systemd/system/libvirt-guests.service`` to +``/etc/systemd/system/libvirt-guests.service`` and make the change there, +specifically make sure the ``After=`` ordering mentions ``virtxend.service`` and +not ``virtxend.socket``: + +:: + + [Unit] + After=virtxend.service + OPTIONS ======= diff --git a/tools/libvirt-guests.service.in b/tools/libvirt-guests.service.in index 3cf647619612..1c569c320dfd 100644 --- a/tools/libvirt-guests.service.in +++ b/tools/libvirt-guests.service.in @@ -9,12 +9,6 @@ After=virtlxcd.socket After=virtvboxd.socket After=virtvzd.socket After=virtxend.socket -After=libvirtd.service -After=virtqemud.service -After=virtlxcd.service -After=virtvboxd.service -After=virtvzd.service -After=virtxend.service After=virt-guest-shutdown.target Documentation=man:libvirt-guests(8) Documentation=https://libvirt.org -- 2.37.2
On Tue, Aug 30, 2022 at 08:43:47AM +0200, Martin Kletzander wrote:
After some debugging and discussion with systemd team it turns out we are misusing the ordering in libvirt-guests.service. That happened because we want to support both monolithic and modular daemon setups and on top of that we also want to support socket activation and services without socket activation. Unfortunately this is impossible to express in the unit file because of how transactions are handled in systemd when dependencies are resolved and multiple actions (jobs) are queued. For explanation from Michal Sekletar see comment #7 in the BZ this patch is fixing:
https://bugzilla.redhat.com/show_bug.cgi?id=1964855#c7
In order to support all the scenarios this patch also amends the manpages so that users that are changing the default can also read how to correct the dependency ordering in libvirt-guests unit file.
Ideally we would also keep the existing configuration during upgrade, but due to our huge support matrix this seems hardly feasible as it could introduce even more problems.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> ---
echo request^W^Wping
docs/manpages/libvirtd.rst | 14 ++++++++++++++ docs/manpages/virtlxcd.rst | 14 ++++++++++++++ docs/manpages/virtqemud.rst | 14 ++++++++++++++ docs/manpages/virtvboxd.rst | 14 ++++++++++++++ docs/manpages/virtvzd.rst | 14 ++++++++++++++ docs/manpages/virtxend.rst | 14 ++++++++++++++ tools/libvirt-guests.service.in | 6 ------ 7 files changed, 84 insertions(+), 6 deletions(-)
diff --git a/docs/manpages/libvirtd.rst b/docs/manpages/libvirtd.rst index ee72f0838221..1347b9b21042 100644 --- a/docs/manpages/libvirtd.rst +++ b/docs/manpages/libvirtd.rst @@ -79,6 +79,20 @@ unit files must be masked: $ systemctl mask libvirtd.socket libvirtd-ro.socket \ libvirtd-admin.socket libvirtd-tls.socket libvirtd-tcp.socket
+If using libvirt-guests service then the ordering for that service needs to be +adapted so that it is ordered after the service unit instead of the socket unit. +Since dependencies and ordering cannot be changed with drop-in overrides, the +whole libvirt-guests unit file needs to be changed. In order to preserve such +change copy the installed ``/usr/lib/systemd/system/libvirt-guests.service`` to +``/etc/systemd/system/libvirt-guests.service`` and make the change there, +specifically make sure the ``After=`` ordering mentions ``libvirtd.service`` and +not ``libvirtd.socket``: + +:: + + [Unit] + After=libvirtd.service +
OPTIONS ======= diff --git a/docs/manpages/virtlxcd.rst b/docs/manpages/virtlxcd.rst index 2e9d8fd14bbb..aebc8adb5822 100644 --- a/docs/manpages/virtlxcd.rst +++ b/docs/manpages/virtlxcd.rst @@ -60,6 +60,20 @@ unit files must be masked: $ systemctl mask virtlxcd.socket virtlxcd-ro.socket \ virtlxcd-admin.socket
+If using libvirt-guests service then the ordering for that service needs to be +adapted so that it is ordered after the service unit instead of the socket unit. +Since dependencies and ordering cannot be changed with drop-in overrides, the +whole libvirt-guests unit file needs to be changed. In order to preserve such +change copy the installed ``/usr/lib/systemd/system/libvirt-guests.service`` to +``/etc/systemd/system/libvirt-guests.service`` and make the change there, +specifically make sure the ``After=`` ordering mentions ``virtlxcd.service`` and +not ``virtlxcd.socket``: + +:: + + [Unit] + After=virtlxcd.service +
OPTIONS ======= diff --git a/docs/manpages/virtqemud.rst b/docs/manpages/virtqemud.rst index ea8d6e3105db..fa9a6ce3755c 100644 --- a/docs/manpages/virtqemud.rst +++ b/docs/manpages/virtqemud.rst @@ -60,6 +60,20 @@ unit files must be masked: $ systemctl mask virtqemud.socket virtqemud-ro.socket \ virtqemud-admin.socket
+If using libvirt-guests service then the ordering for that service needs to be +adapted so that it is ordered after the service unit instead of the socket unit. +Since dependencies and ordering cannot be changed with drop-in overrides, the +whole libvirt-guests unit file needs to be changed. In order to preserve such +change copy the installed ``/usr/lib/systemd/system/libvirt-guests.service`` to +``/etc/systemd/system/libvirt-guests.service`` and make the change there, +specifically make sure the ``After=`` ordering mentions ``virtqemud.service`` and +not ``virtqemud.socket``: + +:: + + [Unit] + After=virtqemud.service +
OPTIONS ======= diff --git a/docs/manpages/virtvboxd.rst b/docs/manpages/virtvboxd.rst index d7339d99f22b..f90de3451d8d 100644 --- a/docs/manpages/virtvboxd.rst +++ b/docs/manpages/virtvboxd.rst @@ -58,6 +58,20 @@ unit files must be masked: $ systemctl mask virtvboxd.socket virtvboxd-ro.socket \ virtvboxd-admin.socket
+If using libvirt-guests service then the ordering for that service needs to be +adapted so that it is ordered after the service unit instead of the socket unit. +Since dependencies and ordering cannot be changed with drop-in overrides, the +whole libvirt-guests unit file needs to be changed. In order to preserve such +change copy the installed ``/usr/lib/systemd/system/libvirt-guests.service`` to +``/etc/systemd/system/libvirt-guests.service`` and make the change there, +specifically make sure the ``After=`` ordering mentions ``virtvboxd.service`` and +not ``virtvboxd.socket``: + +:: + + [Unit] + After=virtvboxd.service +
OPTIONS ======= diff --git a/docs/manpages/virtvzd.rst b/docs/manpages/virtvzd.rst index 42dfa263e450..970719aac1d5 100644 --- a/docs/manpages/virtvzd.rst +++ b/docs/manpages/virtvzd.rst @@ -60,6 +60,20 @@ unit files must be masked: $ systemctl mask virtvzd.socket virtvzd-ro.socket \ virtvzd-admin.socket
+If using libvirt-guests service then the ordering for that service needs to be +adapted so that it is ordered after the service unit instead of the socket unit. +Since dependencies and ordering cannot be changed with drop-in overrides, the +whole libvirt-guests unit file needs to be changed. In order to preserve such +change copy the installed ``/usr/lib/systemd/system/libvirt-guests.service`` to +``/etc/systemd/system/libvirt-guests.service`` and make the change there, +specifically make sure the ``After=`` ordering mentions ``virtvzd.service`` and +not ``virtvzd.socket``: + +:: + + [Unit] + After=virtvzd.service +
OPTIONS ======= diff --git a/docs/manpages/virtxend.rst b/docs/manpages/virtxend.rst index b08346b489d2..cf7685ecc0e6 100644 --- a/docs/manpages/virtxend.rst +++ b/docs/manpages/virtxend.rst @@ -60,6 +60,20 @@ unit files must be masked: $ systemctl mask virtxend.socket virtxend-ro.socket \ virtxend-admin.socket
+If using libvirt-guests service then the ordering for that service needs to be +adapted so that it is ordered after the service unit instead of the socket unit. +Since dependencies and ordering cannot be changed with drop-in overrides, the +whole libvirt-guests unit file needs to be changed. In order to preserve such +change copy the installed ``/usr/lib/systemd/system/libvirt-guests.service`` to +``/etc/systemd/system/libvirt-guests.service`` and make the change there, +specifically make sure the ``After=`` ordering mentions ``virtxend.service`` and +not ``virtxend.socket``: + +:: + + [Unit] + After=virtxend.service +
OPTIONS ======= diff --git a/tools/libvirt-guests.service.in b/tools/libvirt-guests.service.in index 3cf647619612..1c569c320dfd 100644 --- a/tools/libvirt-guests.service.in +++ b/tools/libvirt-guests.service.in @@ -9,12 +9,6 @@ After=virtlxcd.socket After=virtvboxd.socket After=virtvzd.socket After=virtxend.socket -After=libvirtd.service -After=virtqemud.service -After=virtlxcd.service -After=virtvboxd.service -After=virtvzd.service -After=virtxend.service After=virt-guest-shutdown.target Documentation=man:libvirt-guests(8) Documentation=https://libvirt.org -- 2.37.2
On 8/30/22 08:43, Martin Kletzander wrote:
After some debugging and discussion with systemd team it turns out we are misusing the ordering in libvirt-guests.service. That happened because we want to support both monolithic and modular daemon setups and on top of that we also want to support socket activation and services without socket activation. Unfortunately this is impossible to express in the unit file because of how transactions are handled in systemd when dependencies are resolved and multiple actions (jobs) are queued. For explanation from Michal Sekletar see comment #7 in the BZ this patch is fixing:
https://bugzilla.redhat.com/show_bug.cgi?id=1964855#c7
In order to support all the scenarios this patch also amends the manpages so that users that are changing the default can also read how to correct the dependency ordering in libvirt-guests unit file.
Ideally we would also keep the existing configuration during upgrade, but due to our huge support matrix this seems hardly feasible as it could introduce even more problems.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/manpages/libvirtd.rst | 14 ++++++++++++++ docs/manpages/virtlxcd.rst | 14 ++++++++++++++ docs/manpages/virtqemud.rst | 14 ++++++++++++++ docs/manpages/virtvboxd.rst | 14 ++++++++++++++ docs/manpages/virtvzd.rst | 14 ++++++++++++++ docs/manpages/virtxend.rst | 14 ++++++++++++++ tools/libvirt-guests.service.in | 6 ------ 7 files changed, 84 insertions(+), 6 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Martin Kletzander -
Michal Prívozník