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

[Martin Kletzander]

On Wed, Jun 14, 2023 at 04:45:06PM -0600, Jim Fehlig wrote:
>On 6/9/23 03:05, Andrea Bolognani wrote:
>> On Thu, Jun 08, 2023 at 12:35:45PM -0600, Jim Fehlig wrote:
>>> On 6/8/23 08:52, Andrea Bolognani wrote:
>>>> On Wed, Jun 07, 2023 at 04:31:36PM +0200, Martin Kletzander wrote:
>>>>> +# 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),
>>>
>>> Why use the monolithic and split daemons together? Shouldn't we discourage
>>> such configuration? :-)
>>
>> Whether to use monlithic or split daemons is ultimately a choice of
>> the local admin. Fedora defaults to split daemons, but switching back
>> to a "classic" monolithic deployment is still a fully supported
>> scenario.
>>
>> Additionally, the current default has been adopted relatively
>> recently, and we have made the explicit decision *not* to migrate
>> existing installations over. So if your OS was originally installed
>> before split daemons had become the default and you've been dutifully
>> upgrading to subsequent Fedora releases without ever reinstalling,
>> then you're also going to be using the monolithic daemon.
>>
>>>> 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 :(
>>>
>>> It's possible to determine a package install vs upgrade, but IIUC the
>>> problem can occur when installing or upgrading libvirt-daemon-proxy. The
>>> usual 'if [ $1 -ge 2 ]' for upgrade-only actions wont work in that case.
>>
>> Yeah, exactly: it's easy to detect whether a single package is being
>> upgraded or installed from scratch, but in this case we would need to
>> know whether libvirt-daemon is being upgraded in the scriptlet for
>> libvirt-daemon-proxy, which I don't think is possible.
>
>I think we need to know more than whether libvirt-daemon is being upgraded. We
>need to also know whether it's running, right? Even if libvirt-daemon is being
>upgraded, it's fine to call %systemd_post on virtproxyd sockets if the libvirtd
>ones are disabled, right? If this is the case, my idea to use trigger scriptlets
>doesn't help.
>
>> Can we somehow enforce that libvirt-daemon is fully configured before
>> libvirt-daemon-proxy? If so, we could create a witness file based on
>> the information we need in libvirt-daemon's %post, look at it in
>> libvirt-daemon-proxy's %post and base our decision on that.
>
>What would the witness file indicate? As I understand, it would essentially have
>to indicate whether libvirtd sockets/service are enabled. If so, couldn't that
>be done directly in virtproxy post script? Something like the below hack?
>

We already have some things for this.  There is
libvirt_daemon_schedule_restart, but that is done in the post phase.

What we could do is check the current state (whether libvirtd is running
and what is being installed) in %pretrans of the daemon and virtproxyd,
save it somewhere similar to libvirt_daemon_schedule_restart and then in
%posttrans or %post actually figure out what the new state should be.

Does that make sense?

>Regards,
>Jim
>
>diff --git a/libvirt.spec.in b/libvirt.spec.in
>index 1f77cd90b7..c848c70c0c 100644
>--- a/libvirt.spec.in
>+++ b/libvirt.spec.in
>@@ -1592,7 +1592,19 @@ fi
>
>  %post daemon-proxy
>  %if %{with_modular_daemons}
>-%libvirt_daemon_systemd_post_inet virtproxyd
>+libvirtd_enabled=0
>+for unit in -ro.socket .service .socket
>+do
>+    if systemctl is-enabled libvirtd$unit
>+    then
>+        libvirtd_enabled=1
>+        break
>+    fi
>+done
>+if [ $libvirtd_enabled -eq 0 ]
>+then
>+    %libvirt_daemon_systemd_post_inet virtproxyd
>+fi
>  %endif
>
>  %preun daemon-proxy
>
-------------- 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/20230616/720d8991/attachment-0001.sig>