On Thu, Jun 08, 2023 at 09:55:28AM +0200, Michal Prívozník wrote:
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@xxxxxxxxxx> --- 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-proxyI 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@xxxxxxxxxx>
Thank you, I'll give Jim (original author) and others some time to chime in too.
Michal
Attachment:
signature.asc
Description: PGP signature
