[PATCH] spec: Do not disable some systemd units of newly split package

[Andrea Bolognani]

On Wed, Jun 07, 2023 at 04:31:36PM +0200, Martin Kletzander wrote:
> Since virtproxyd was split into libvirt-daemon-proxy package it can
> happen that, in case a distribution has such systemd preset, when
> installing this package, already pre-enabled and configured units like
> -tls.socket and -tcp.socket will get disabled.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2210058
> Fixes: 5358618b1cd0afc126aed313249bf2134731665f
> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
> ---
> This is more like an RFC as I would really like to know what to really do in
> this case.  What happens, basically, is that if you have libvirt-daemon-9.0.0
> and set up virtproxyd-tls.socket for example and then upgrade to anything newer,
> then the package libvirt-daemon-proxy will get installed.  The %post action
> calls "%libvirt_daemon_systemd_post_inet virtproxyd" which calls "%systemd_post
> with all virtproxyd units.  What %systemd_post is supposed to do is reset units
> to a preset state in the case of package installation, but not during upgrade.
> However the libvirt-daemon-proxy package did not exist on the system before, so
> this action is not an update, but an installation.
>
> If no preset is mentioned for a unit then `systemctl preset` does not change
> anything.  However some distros might have a catch-all preset "disable *" for
> some reason, I guess based on an example in the documentation, and there is no
> way to override an already configured preset, you can only enable or disable a
> unit in a preset.
>
> That all means than it can happen that you enable virtproxyd-tcp.socket, for
> example, then update your system and find that it is disabled.  There are
> various ways to deal with this, but I don't see any one that would 100% satisfy
> me with regards to all the issues and at the same time could be implemented
> "soon enough" given libvirt already had three releases with the
> libvirt-daemon-proxy split.
>
>  libvirt.spec.in | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index 1f77cd90b772..50f521b7ce88 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -1592,7 +1592,13 @@ fi
>
>  %post daemon-proxy
>  %if %{with_modular_daemons}
> -%libvirt_daemon_systemd_post_inet virtproxyd
> +# Since this was split into a different package, a transparent update for the
> +# virtproxyd units could actually disable an already configured ones
> +# (e.g. virtproxyd-tls.socket) as %systemd_post runs `systemctl preset` if this
> +# is an installation (and is skipped on update).  So skip this step for those
> +# that need an extra setup to work since they will most likely not be preset to
> +# enabled, but that is up to the point of the distribution.
> +%libvirt_daemon_systemd_post virtproxyd

It's actually worse than that: if you are using the monolithic daemon
on a distro that uses split daemons by default (e.g. Fedora), then
upgrading will result in libvirt-daemon-proxy being installed and
virtproxyd being enabled due to the preset.

However, since libvirtd and virtproxyd use (by design) the same paths
for their sockets, this will result in socket activation being broken
and libvirtd effectively not working anymore once the first 120
seconds after boot have passed.

I have been meaning to look into this but other things got in the
way, so thank you for getting the ball rolling; unfortunately, I
don't think what you have come up with will help in the scenario I've
just described.

Basically we need to detect if we're installing the
libvirt-daemon-proxy package as part of an upgrade and *not touch
anything* if that's the case. I'm not sure how that can be achieved
in the context of RPM scriptlets though, or if it's even possible :(

CC'ing Laine, who reported this issue initially, and Dan, who worked
with us to track it down.

-- 
Andrea Bolognani / Red Hat / Virtualization