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 then 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 <abologna@redhat.com> --- 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 -- 2.41.0

In most cases the replacement is straightforward, with the biggest difference being that we now schedule restarts during %pre instead of %post. This also means that we can get rid of %post for most packages, reducing the number of scriptlets that need to run during install/upgrade. Notable exceptions are libvirt-guests.service, where we stop using the standard systemd macros to adopt our custom ones, as well as the virtlogd and virtlockd services, where the reload operation is moved from %postun to %posttrans. https://bugzilla.redhat.com/show_bug.cgi?id=2210058 Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- libvirt.spec.in | 168 +++++++++++++++++------------------------------- 1 file changed, 59 insertions(+), 109 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index a41800c273..d929170b2b 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -461,7 +461,8 @@ Requires: polkit >= 0.112 Requires: dmidecode %endif # For service management -Requires(post): /usr/bin/systemctl +Requires(posttrans): /usr/bin/systemctl +Requires(preun): /usr/bin/systemctl # libvirtd depends on 'messagebus' service Requires: dbus # For uid creation during pre @@ -1625,103 +1626,78 @@ fi \ %pre daemon %libvirt_sysconfig_pre libvirtd - -%post daemon -%libvirt_daemon_systemd_post_inet libvirtd -%libvirt_daemon_schedule_restart libvirtd +%libvirt_systemd_inet_pre libvirtd %posttrans daemon %libvirt_sysconfig_posttrans libvirtd -%libvirt_daemon_perform_restart libvirtd +%libvirt_systemd_inet_posttrans libvirtd %preun daemon -%libvirt_daemon_systemd_preun_inet libvirtd +%libvirt_systemd_inet_preun libvirtd %pre daemon-common %libvirt_sysconfig_pre libvirt-guests +%libvirt_systemd_oneshot_pre libvirt-guests # 'libvirt' group is just to allow password-less polkit access to libvirt # daemons. The uid number is irrelevant, so we use dynamic allocation. getent group libvirt >/dev/null || groupadd -r libvirt exit 0 -%post daemon-common -%systemd_post libvirt-guests.service - %posttrans daemon-common %libvirt_sysconfig_posttrans libvirt-guests +%libvirt_systemd_oneshot_posttrans libvirt-guests %preun daemon-common -%systemd_preun libvirt-guests.service - -%postun daemon-common -/bin/systemctl daemon-reload >/dev/null 2>&1 || : -%systemd_postun libvirt-guests.service +%libvirt_systemd_oneshot_preun libvirt-guests %pre daemon-lock %libvirt_sysconfig_pre virtlockd - -%post daemon-lock -%libvirt_daemon_systemd_post_priv virtlockd +%libvirt_systemd_privileged_pre virtlockd %posttrans daemon-lock %libvirt_sysconfig_posttrans virtlockd +%libvirt_systemd_privileged_posttrans virtlockd %preun daemon-lock -%libvirt_daemon_systemd_preun_priv virtlockd - -%postun daemon-lock -/bin/systemctl daemon-reload >/dev/null 2>&1 || : -if [ $1 -ge 1 ] ; then - /bin/systemctl reload-or-try-restart virtlockd.service >/dev/null 2>&1 || : -fi +%libvirt_systemd_privileged_preun virtlockd %pre daemon-log %libvirt_sysconfig_pre virtlogd - -%post daemon-log -%libvirt_daemon_systemd_post_priv virtlogd +%libvirt_systemd_privileged_pre virtlogd %posttrans daemon-log %libvirt_sysconfig_posttrans virtlogd +%libvirt_systemd_privileged_posttrans virtlogd %preun daemon-log -%libvirt_daemon_systemd_preun_priv virtlogd - -%postun daemon-log -/bin/systemctl daemon-reload >/dev/null 2>&1 || : -if [ $1 -ge 1 ] ; then - /bin/systemctl reload-or-try-restart virtlogd.service >/dev/null 2>&1 || : -fi +%libvirt_systemd_privileged_preun virtlogd %pre daemon-proxy %libvirt_sysconfig_pre virtproxyd - -%post daemon-proxy -%libvirt_daemon_systemd_post_inet virtproxyd +%libvirt_systemd_inet_pre virtproxyd %posttrans daemon-proxy %libvirt_sysconfig_posttrans virtproxyd +%libvirt_systemd_inet_posttrans virtproxyd %preun daemon-proxy -%libvirt_daemon_systemd_preun_inet virtproxyd +%libvirt_systemd_inet_preun virtproxyd %pre daemon-driver-network %libvirt_sysconfig_pre virtnetworkd +%libvirt_systemd_unix_pre virtnetworkd %post daemon-driver-network %if %{with_firewalld_zone} %firewalld_reload %endif -%libvirt_daemon_systemd_post virtnetworkd -%libvirt_daemon_schedule_restart virtnetworkd - %posttrans daemon-driver-network %libvirt_sysconfig_posttrans virtnetworkd -%libvirt_daemon_perform_restart virtnetworkd +%libvirt_systemd_unix_posttrans virtnetworkd %preun daemon-driver-network -%libvirt_daemon_systemd_preun virtnetworkd +%libvirt_systemd_unix_preun virtnetworkd %postun daemon-driver-network %if %{with_firewalld_zone} @@ -1730,77 +1706,63 @@ fi %pre daemon-driver-nwfilter %libvirt_sysconfig_pre virtnwfilterd - -%post daemon-driver-nwfilter -%libvirt_daemon_systemd_post virtnwfilterd -%libvirt_daemon_schedule_restart virtnwfilterd +%libvirt_systemd_unix_pre virtnwfilterd %posttrans daemon-driver-nwfilter %libvirt_sysconfig_posttrans virtnwfilterd -%libvirt_daemon_perform_restart virtnwfilterd +%libvirt_systemd_unix_posttrans virtnwfilterd %preun daemon-driver-nwfilter -%libvirt_daemon_systemd_preun virtnwfilterd +%libvirt_systemd_unix_preun virtnwfilterd %pre daemon-driver-nodedev %libvirt_sysconfig_pre virtnodedevd - -%post daemon-driver-nodedev -%libvirt_daemon_systemd_post virtnodedevd -%libvirt_daemon_schedule_restart virtnodedevd +%libvirt_systemd_unix_pre virtnodedevd %posttrans daemon-driver-nodedev %libvirt_sysconfig_posttrans virtnodedevd -%libvirt_daemon_perform_restart virtnodedevd +%libvirt_systemd_unix_posttrans virtnodedevd %preun daemon-driver-nodedev -%libvirt_daemon_systemd_preun virtnodedevd +%libvirt_systemd_unix_preun virtnodedevd %pre daemon-driver-interface %libvirt_sysconfig_pre virtinterfaced - -%post daemon-driver-interface -%libvirt_daemon_systemd_post virtinterfaced -%libvirt_daemon_schedule_restart virtinterfaced +%libvirt_systemd_unix_pre virtinterfaced %posttrans daemon-driver-interface %libvirt_sysconfig_posttrans virtinterfaced -%libvirt_daemon_perform_restart virtinterfaced +%libvirt_systemd_unix_posttrans virtinterfaced %preun daemon-driver-interface -%libvirt_daemon_systemd_preun virtinterfaced +%libvirt_systemd_unix_preun virtinterfaced %pre daemon-driver-secret %libvirt_sysconfig_pre virtsecretd - -%post daemon-driver-secret -%libvirt_daemon_systemd_post virtsecretd -%libvirt_daemon_schedule_restart virtsecretd +%libvirt_systemd_unix_pre virsecretd %posttrans daemon-driver-secret %libvirt_sysconfig_posttrans virtsecretd -%libvirt_daemon_perform_restart virtsecretd +%libvirt_systemd_unix_posttrans virsecretd %preun daemon-driver-secret -%libvirt_daemon_systemd_preun virtsecretd +%libvirt_systemd_unix_preun virsecretd %pre daemon-driver-storage-core %libvirt_sysconfig_pre virtstoraged - -%post daemon-driver-storage-core -%libvirt_daemon_systemd_post virtstoraged -%libvirt_daemon_schedule_restart virtstoraged +%libvirt_systemd_unix_pre virtstoraged %posttrans daemon-driver-storage-core %libvirt_sysconfig_posttrans virtstoraged -%libvirt_daemon_perform_restart virtstoraged +%libvirt_systemd_unix_posttrans virtstoraged %preun daemon-driver-storage-core -%libvirt_daemon_systemd_preun virtstoraged +%libvirt_systemd_unix_preun virtstoraged %if %{with_qemu} %pre daemon-driver-qemu %libvirt_sysconfig_pre virtqemud +%libvirt_systemd_unix_pre virtqemud # We want soft static allocation of well-known ids, as disk images # are commonly shared across NFS mounts by id rather than name; see # https://fedoraproject.org/wiki/Packaging:UsersAndGroups @@ -1815,66 +1777,57 @@ if ! getent passwd qemu >/dev/null; then fi exit 0 -%post daemon-driver-qemu -%libvirt_daemon_systemd_post virtqemud -%libvirt_daemon_schedule_restart virtqemud - %posttrans daemon-driver-qemu %libvirt_sysconfig_posttrans virtqemud -%libvirt_daemon_perform_restart virtqemud +%libvirt_systemd_unix_posttrans virtqemud %preun daemon-driver-qemu -%libvirt_daemon_systemd_preun virtqemud +%libvirt_systemd_unix_preun virtqemud %endif %if %{with_lxc} %pre daemon-driver-lxc %libvirt_sysconfig_pre virtlxcd - -%post daemon-driver-lxc -%libvirt_daemon_systemd_post virtlxcd -%libvirt_daemon_schedule_restart virtlxcd +%libvirt_systemd_unix_pre virtlxcd %posttrans daemon-driver-lxc %libvirt_sysconfig_posttrans virtlxcd -%libvirt_daemon_perform_restart virtlxcd +%libvirt_systemd_unix_posttrans virtlxcd %preun daemon-driver-lxc -%libvirt_daemon_systemd_preun virtlxcd +%libvirt_systemd_unix_preun virtlxcd %endif %if %{with_vbox} %pre daemon-driver-vbox %libvirt_sysconfig_pre virtvboxd - -%post daemon-driver-vbox -%libvirt_daemon_systemd_post virtvboxd -%libvirt_daemon_schedule_restart virtvboxd +%libvirt_systemd_unix_pre virtvboxd %posttrans daemon-driver-vbox %libvirt_sysconfig_posttrans virtvboxd -%libvirt_daemon_perform_restart virtvboxd +%libvirt_systemd_unix_posttrans virtvboxd %preun daemon-driver-vbox -%libvirt_daemon_systemd_preun virtvboxd +%libvirt_systemd_unix_preun virtvboxd %endif %if %{with_libxl} %pre daemon-driver-libxl %libvirt_sysconfig_pre virtxend - -%post daemon-driver-libxl -%libvirt_daemon_systemd_post virtxend -%libvirt_daemon_schedule_restart virtxend +%libvirt_systemd_unix_pre virtxend %posttrans daemon-driver-libxl %libvirt_sysconfig_posttrans virtxend -%libvirt_daemon_perform_restart virtxend +%libvirt_systemd_unix_posttrans virtxend %preun daemon-driver-libxl -%libvirt_daemon_systemd_preun virtxend +%libvirt_systemd_unix_preun virtxend %endif +%pre daemon-config-network +%libvirt_systemd_config_pre libvirtd +%libvirt_systemd_config_pre virtnetworkd + %post daemon-config-network if test $1 -eq 1 && test ! -f %{_sysconfdir}/libvirt/qemu/networks/default.xml ; then # see if the network used by default network creates a conflict, @@ -1911,15 +1864,15 @@ if test $1 -eq 1 && test ! -f %{_sysconfdir}/libvirt/qemu/networks/default.xml ; ln -s ../default.xml %{_sysconfdir}/libvirt/qemu/networks/autostart/default.xml # libvirt saves this file with mode 0600 chmod 0600 %{_sysconfdir}/libvirt/qemu/networks/default.xml - - # Make sure libvirt picks up the new network defininiton - %libvirt_daemon_schedule_restart libvirtd - %libvirt_daemon_schedule_restart virtnetworkd fi %posttrans daemon-config-network -%libvirt_daemon_perform_restart libvirtd -%libvirt_daemon_perform_restart virtnetworkd +%libvirt_systemd_config_posttrans libvirtd +%libvirt_systemd_config_posttrans virtnetworkd + +%pre daemon-config-nwfilter +%libvirt_systemd_config_pre libvirtd +%libvirt_systemd_config_pre virtnwfilterd %post daemon-config-nwfilter for datadir_file in %{_datadir}/libvirt/nwfilter/*.xml; do @@ -1929,13 +1882,10 @@ for datadir_file in %{_datadir}/libvirt/nwfilter/*.xml; do install -m 0600 "$datadir_file" "$sysconfdir_file" fi done -# Make sure libvirt picks up the new nwfilter defininitons -%libvirt_daemon_schedule_restart libvirtd -%libvirt_daemon_schedule_restart virtnwfilterd %posttrans daemon-config-nwfilter -%libvirt_daemon_perform_restart libvirtd -%libvirt_daemon_perform_restart virtnwfilterd +%libvirt_systemd_config_posttrans libvirtd +%libvirt_systemd_config_posttrans virtnwfilterd %if %{with_lxc} %pre login-shell -- 2.41.0