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

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(a)redhat.com&gt; > --- > 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(a)redhat.com&gt;

Michal