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

[Martin Kletzander]

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

Thank you, I'll give Jim (original author) and others some time to chime
in too.

>Michal
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20230608/68778418/attachment.sig>