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

[Michal Prívozník]

On 6/7/23 16:31, 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
>  %endif
>  
>  %preun daemon-proxy

I think this is sensible approach. Although at this point it may be a
bit late (at least for bleeding edge distros). But hey, slower distros
may still benefit from this change. You can count on my:

Reviewed-by: Michal Privoznik <mprivozn at redhat.com>

Michal