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

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(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 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 :(