From 3bfc76a953c24793c2193768cca68d428490c01a Mon Sep 17 00:00:00 2001 From: Andrea Bolognani Date: Wed, 5 Jul 2023 18:07:34 +0200 Subject: [PATCH] rpm: Introduce new macros for handling of systemd units systemd provides a number of standard RPM macros but they don't quite satisfy our requirements, as evidenced by the fact that we have already built some custom tooling around them. Scenarios that the standard macros don't cover and that we're already addressing with our custom ones: * for some services (libvirtd, virtnetworkd, virtnwfilterd) there are multiple conditions that might lead to a restart, and we want to make sure that they're not needlessly restarted several times per transaction; * some services (virtlogd, virtlockd) must not be restarted during upgrade, so we have to reload them instead. Issues that neither the standard macros nor our custom ones address: * presets for units should be applied when the unit is first installed, not when the package that contains it is. The package split that happened in 9.1.0 highlighted why this last point is so important: when virtproxyd and its sockets were moved from libvirt-daemon to the new libvirt-daemon-proxy package, upgrades from 9.0.0 caused presets for them to be applied. On a platform such as Fedora, where modular daemons are the default, this has resulted in breaking existing deployments in at least two scenarios. The first one is machines that were configured to use the monolithic daemon, either because the local admin had manually changed the configuration or because the installation dated back to before modular daemons had become the default. In this case, virtproxyd.socket being enabled resulted in a silent conflict with libvirtd.socket, which by design shares the same path, and thus a completely broken setup. The second one is machines where virtproxy-tls.socket, which is disabled by default, had manually been enabled: in this case, applying the presets resulted in it being disabled and thus a loss of remote availability. Note that these are just two concrete scenarios, but the problem is more generic. For example, if we were to add more units to an existing package, per the current approach they wouldn't have their presets applied. The new macros are designed to avoid all of the pitfalls mentioned above. As a bonus, they're also simpler to use: where the current approach requires restarts and other operations to be handled separately, the new one integrates the two so that, for each scriptlet, a single macro call is needed. https://bugzilla.redhat.com/show_bug.cgi?id=2210058 Signed-off-by: Andrea Bolognani Reviewed-by: Martin Kletzander --- libvirt.spec.in | 140 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 140 insertions(+) diff --git a/libvirt.spec.in b/libvirt.spec.in index d09c3b3340..a41800c273 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1471,18 +1471,158 @@ then \ fi \ %libvirt_daemon_finish_restart %1 +%define libvirt_rpmstatedir %{_localstatedir}/lib/rpm-state/libvirt + +# Mark units such that presets will later be applied to them. Meant +# to be called during %pre. Units that already exist on the system +# will not be marked, with the assumption that presets have already +# been applied at some point in the past. This makes it safe to call +# this macro for all units each time %pre runs. +%define libvirt_systemd_schedule_preset() \ + mkdir -p %{libvirt_rpmstatedir} || : \ + for unit in %{?*}; do \ + if ! test -e %{_unitdir}/$unit; then \ + touch %{libvirt_rpmstatedir}/preset-$unit || : \ + fi \ + done \ + %{nil} + +# Apply presets for units that have previously been marked. Meant to +# be called during %posttrans. Note that foo.service must be passed +# as the first argument, before all the various foo*.socket +# associated with it, for things to work correctly. This is necessary +# because Also=foo.socket is usually present in foo.service's +# [Install] section, and we want that configuration to take +# precedence over foo.socket's own presets. +%define libvirt_systemd_perform_preset() \ + %{?7:%{error:Too many arguments}} \ + for unit in %{?2} %{?3} %{?4} %{?5} %{?6} %1; do \ + if test -e %{libvirt_rpmstatedir}/preset-$unit; then \ + /usr/bin/systemctl --no-reload preset $unit || : \ + fi \ + rm -f %{libvirt_rpmstatedir}/preset-$unit \ + done \ + rmdir %{libvirt_rpmstatedir} 2>/dev/null || : \ + %{nil} + +# Mark a single unit for restart. Meant to be called during %pre. +%define libvirt_systemd_schedule_restart() \ + mkdir -p %{libvirt_rpmstatedir} || : \ + touch %{libvirt_rpmstatedir}/restart-%1 || : \ + %{nil} + +# Restart a unit that was previously marked. Meant to be called +# during %posttrans. If systemd is not running, no action will be +# performed. +%define libvirt_systemd_perform_restart() \ + if test -d /run/systemd/system && \ + test -e %{libvirt_rpmstatedir}/restart-%1; then \ + /usr/bin/systemctl try-restart %1 >/dev/null 2>&1 || : \ + fi \ + rm -f %{libvirt_rpmstatedir}/restart-%1 \ + rmdir %{libvirt_rpmstatedir} 2>/dev/null || : \ + %{nil} + +# Mark a single unit for reload. Meant to be called during %pre. +%define libvirt_systemd_schedule_reload() \ + mkdir -p %{libvirt_rpmstatedir} || : \ + touch %{libvirt_rpmstatedir}/reload-%1 || : \ + %{nil} + +# Reload a unit that was previously marked. Meant to be called during +# %posttrans. If systemd is not running, no action will be performed. +%define libvirt_systemd_perform_reload() \ + if test -d /run/systemd/system && \ + test -e %{libvirt_rpmstatedir}/reload-%1; then \ + /usr/bin/systemctl try-reload-or-restart %1 >/dev/null 2>&1 || : \ + fi \ + rm -f %{libvirt_rpmstatedir}/reload-%1 \ + rmdir %{libvirt_rpmstatedir} 2>/dev/null || : \ + %{nil} + +# Disable a single unit, optionally stopping it if systemd is +# running. Meant to be called during %preun. +%define libvirt_systemd_disable() \ + if test -d /run/systemd/system; then \ + /usr/bin/systemctl --no-reload disable --now %{?*} || : \ + else \ + /usr/bin/systemctl --no-reload disable %{?*} || : \ + fi \ + %{nil} + +# %pre implementation for services that should be restarted on +# upgrade. Note that foo.service must be passed as the first +# argument, before all the various foo*.socket associated with it. +%define libvirt_systemd_restart_pre() \ + %libvirt_systemd_schedule_preset %{?*} \ + %libvirt_systemd_schedule_restart %1 \ + %{nil} + +# %pre implementation for services that should be reloaded on +# upgrade. Note that foo.service must be passed as the first +# argument, before all the various foo*.socket associated with it. +%define libvirt_systemd_reload_pre() \ + %libvirt_systemd_schedule_preset %{?*} \ + %libvirt_systemd_schedule_reload %1 \ + %{nil} + +# %pre implementation for services that should be neither restarted +# nor reloaded on upgrade. +%define libvirt_systemd_noaction_pre() \ + %libvirt_systemd_schedule_preset %{?*} \ + %{nil} + +# %posttrans implementation for all services. We can use a single +# macro to cover all scenarios, because each operation will only be +# performed if it had previously been scheduled. Note that +# foo.service must be passed as the first argument, before all the +# various foo*.socket associated with it. +%define libvirt_systemd_posttrans() \ + %libvirt_systemd_perform_preset %{?*} \ + %libvirt_systemd_perform_reload %1 \ + %libvirt_systemd_perform_restart %1 \ + %{nil} + +# %preun implementation for all services. +%define libvirt_systemd_preun() \ + if [ $1 -lt 1 ]; then \ + %libvirt_systemd_disable %{?*} \ + fi \ + %{nil} + # For daemons with only UNIX sockets %define libvirt_daemon_systemd_post() %systemd_post %1.socket %1-ro.socket %1-admin.socket %1.service %define libvirt_daemon_systemd_preun() %systemd_preun %1.service %1-ro.socket %1-admin.socket %1.socket +%define libvirt_systemd_unix_pre() %libvirt_systemd_restart_pre %1.service %1.socket %1-ro.socket %1-admin.socket +%define libvirt_systemd_unix_posttrans() %libvirt_systemd_posttrans %1.service %1.socket %1-ro.socket %1-admin.socket +%define libvirt_systemd_unix_preun() %libvirt_systemd_preun %1.service %1.socket %1-ro.socket %1-admin.socket + # For daemons with UNIX and INET sockets %define libvirt_daemon_systemd_post_inet() %systemd_post %1.socket %1-ro.socket %1-admin.socket %1-tls.socket %1-tcp.socket %1.service %define libvirt_daemon_systemd_preun_inet() %systemd_preun %1.service %1-ro.socket %1-admin.socket %1-tls.socket %1-tcp.socket %1.socket +%define libvirt_systemd_inet_pre() %libvirt_systemd_restart_pre %1.service %1.socket %1-ro.socket %1-admin.socket %1-tls.socket %1-tcp.socket +%define libvirt_systemd_inet_posttrans() %libvirt_systemd_posttrans %1.service %1.socket %1-ro.socket %1-admin.socket %1-tls.socket %1-tcp.socket +%define libvirt_systemd_inet_preun() %libvirt_systemd_preun %1.service %1.socket %1-ro.socket %1-admin.socket %1-tls.socket %1-tcp.socket + # For daemons with only UNIX sockets and no unprivileged read-only access %define libvirt_daemon_systemd_post_priv() %systemd_post %1.socket %1-admin.socket %1.service %define libvirt_daemon_systemd_preun_priv() %systemd_preun %1.service %1-admin.socket %1.socket +%define libvirt_systemd_privileged_pre() %libvirt_systemd_reload_pre %1.service %1.socket %1-admin.socket +%define libvirt_systemd_privileged_posttrans() %libvirt_systemd_posttrans %1.service %1.socket %1-admin.socket +%define libvirt_systemd_privileged_preun() %libvirt_systemd_preun %1.service %1.socket %1-admin.socket + +# For one-shot daemons that have no associated sockets and should never be restarted +%define libvirt_systemd_oneshot_pre() %libvirt_systemd_noaction_pre %1.service +%define libvirt_systemd_oneshot_posttrans() %libvirt_systemd_posttrans %1.service +%define libvirt_systemd_oneshot_preun() %libvirt_systemd_preun %1.service + +# For packages that install configuration for other daemons +%define libvirt_systemd_config_pre() %libvirt_systemd_schedule_restart %1.service +%define libvirt_systemd_config_posttrans() %libvirt_systemd_perform_restart %1.service + %pre daemon %libvirt_sysconfig_pre libvirtd