[PATCH] spec: Do not disable some systemd units of newly split package
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@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 -- 2.41.0
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@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@redhat.com> Michal
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@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@redhat.com>
Thank you, I'll give Jim (original author) and others some time to chime in too.
Michal
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@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
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 :( CC'ing Laine, who reported this issue initially, and Dan, who worked with us to track it down. -- Andrea Bolognani / Red Hat / Virtualization
On 6/8/23 08:52, Andrea Bolognani wrote:
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@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
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? :-)
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 :(
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. Can we skip the 'libvirt_daemon_systemd_post_inet' if virtproxyd is enabled? Or adjust the macro to not 'systemd_post' sockets that are already enabled? Regards, Jim
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. 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.
Can we skip the 'libvirt_daemon_systemd_post_inet' if virtproxyd is enabled? Or adjust the macro to not 'systemd_post' sockets that are already enabled?
Sockets *and* services. We don't want virtproxyd.service to be started on boot if libvirtd.service will also be. -- Andrea Bolognani / Red Hat / Virtualization
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.
Nod. Having both installed and using one or the other is fine. Not sure why I was thinking both were being used at the same time, but I think we can agree that is unwise :-).
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.
Yes, it's the same for Leap or Tumbleweed users.
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.
It may be possible using trigger scriptlets http://ftp.rpm.org/api/4.4.2.2/triggers.html I need to finish another task before looking in more detail. If anyone picks this up in the meantime please let me know.
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.
We may be able to accomplish this with trigger scriptlets and state files https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_savin...
Can we skip the 'libvirt_daemon_systemd_post_inet' if virtproxyd is enabled? Or adjust the macro to not 'systemd_post' sockets that are already enabled?
Sockets *and* services. We don't want virtproxyd.service to be started on boot if libvirtd.service will also be.
As a side note, I see virtproxyd service and sockets 'Conflicts' with libvirtd counterparts. Regards, Jim
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? 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
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
On 6/16/23 02:12, Martin Kletzander wrote:
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?
Yes, and sorry for getting caught up in the followup issue Andrea raised. Although I'm having a hard time reproducing either issue, which makes it difficult to experiment and eventually test a fix. My setup is a recent openSUSE Tumbleweed. For your issue I added the following presets to /usr/lib/systemd/system-preset/90-default-openSUSE.preset enable virtqemud.socket enable virtlogd.socket enable virtnetworkd.socket enable virtnodedevd.socket enable virtstoraged.socket enable virtsecretd.socket Next I installed libvirt-daemon and libvirt-daemon-qemu without commit b1da03b5b3. As expected, all the above sockets where enabled, while virtproxyd.socket and libvirtd.socket were disabled. I then enabled virtproxyd.socket and virtproxyd-tcp.socket. Finally I updated the setup with packages containing b1da03b5b3, the new libvirt-daemon-proxy was installed, but in the end virtproxyd.socket and virtproxyd-tcp.socket were still enabled and the system seemed fine. In an attempt to reproduce Andrea's report, I added the following presets to /usr/lib/systemd/system-preset/90-default-openSUSE.preset enable virtqemud.socket enable virtlogd.socket enable virtnetworkd.socket enable virtnodedevd.socket enable virtstoraged.socket enable virtsecretd.socket enable virtproxyd.socket As before, installed packages without commit b1da03b5b3. I then disabled all the virt* presets (with exception of virtlogd) and enabled libvirtd.socket. After updating to packages containing commit b1da03b5b3, virtproxyd.socket was still disabled and libvirt.socket was enabled. No problem connecting to libvirtd, even after the service timeout. IIUC, the Tumbleweed default is to disable presets, similar to the Fedora default you mentioned earlier. The systemd-presets-common-SUSE package contains /usr/lib/systemd/system-preset/99-default-disable.preset, which has the single entry 'disable *'. Any tips on reproducing the issue? I must be missing something. What are the libvirt-related presets in Fedora? I couldn't easily find them. FYI, the Tumbleweed ones are here https://build.opensuse.org/package/show/Base:System/systemd-presets-common-S... https://build.opensuse.org/package/show/Base:System/systemd-presets-branding... In the past I've never added libvirt services/sockets to the openSUSE presets since users could easily enable libvirtd and virtlogd and be done with it. (Note the SLES presets do enable libvirtd and virtlogd.) But it becomes more burdensome with modular daemons, so I plan to add the above presets (excluding virtproxyd) to the openSUSE defaults. Regards, Jim
On Fri, Jun 16, 2023 at 03:57:37PM -0600, Jim Fehlig wrote:
On 6/16/23 02:12, Martin Kletzander wrote:
On Wed, Jun 14, 2023 at 04:45:06PM -0600, Jim Fehlig wrote:
On 6/9/23 03:05, Andrea Bolognani wrote:
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?
The current logic in all packages' %post scriptlet basically translates to: if installing the package from scratch, apply the systemd presets; if upgrading, leave things well alone. What I would want the witness file to indicate would be: libvirt-daemon is being upgraded, so when running the %post for libvirt-daemon-proxy behave as in the upgrade scenario as opposed to the install-from-scratch scenario. Detecting whether the monolithic libvirtd and its sockets are enabled could also work, but feels more fragile. Specifically, it would not cover the scenario described by Martin, where the deployment is a split daemon one but with some units that are disabled by default based on presets have been manually enabled by the admin. The problem is that I'm not sure we can create and process such a witness file reliably. Figuring that out requires research and real-life testing :)
In an attempt to reproduce Andrea's report, I added the following presets to /usr/lib/systemd/system-preset/90-default-openSUSE.preset
enable virtqemud.socket enable virtlogd.socket enable virtnetworkd.socket enable virtnodedevd.socket enable virtstoraged.socket enable virtsecretd.socket enable virtproxyd.socket
As before, installed packages without commit b1da03b5b3. I then disabled all the virt* presets (with exception of virtlogd) and enabled libvirtd.socket.
I'm not sure this is entirely relevant, but just for completeness' sake: in a monolithic daemon deployment, you want libvirtd.service in addition to its sockets to be enabled. This is needed to make sure domain autostart works as intended. Basically, starting from a split daemon deployment, you want to follow the steps outlined here[1] but in reverse.
After updating to packages containing commit b1da03b5b3, virtproxyd.socket was still disabled and libvirt.socket was enabled. No problem connecting to libvirtd, even after the service timeout.
I'll try to reproduce this myself later this week, but what you're describing doesn't match my expectations: virtproxyd.socket should have been enabled during installation. Are the packages you're using for testing a direct rebuild of the spec file shipped upstream? Or have you integrated the changes into the openSUSE spec file somehow? Note that the scriptlets in the upstream spec file call out to some standard macros, and it's also possible that the implementation of said macros is not the same across Fedora and openSUSE.
Any tips on reproducing the issue? I must be missing something. What are the libvirt-related presets in Fedora? I couldn't easily find them. FYI, the Tumbleweed ones are here
https://build.opensuse.org/package/show/Base:System/systemd-presets-common-S... https://build.opensuse.org/package/show/Base:System/systemd-presets-branding...
They're part of the fedora-release package[2]. Specifically, we have # Virtualization driver specific daemons. Start by default at boot for VM # autostart, but shutdown after 2 mins and socket activated thereafter enable virtqemud.service enable virtxend.service enable virtlxcd.service # Compatibility with libvirtd sockets for old clients and expose TCP sockets enable virtproxyd.socket # Secondary drivers providing supporting functionality to main virtualization # drivers, socket activated only when required enable virtinterfaced.socket enable virtnetworkd.socket enable virtnodedevd.socket enable virtnwfilterd.socket enable virtsecretd.socket enable virtstoraged.socket
In the past I've never added libvirt services/sockets to the openSUSE presets since users could easily enable libvirtd and virtlogd and be done with it. (Note the SLES presets do enable libvirtd and virtlogd.) But it becomes more burdensome with modular daemons, so I plan to add the above presets (excluding virtproxyd) to the openSUSE defaults.
Are you okay with old(ish) clients not being able to connect to the host? Because that's why the virtproxyd.socket is enabled by default in Fedora: if the client predates the introduction of split daemons it will try to connect to the old libvirtd socket, and in a split daemons deployment they only way to make that work is by having virtproxyd listening on it. [1] https://libvirt.org/daemons.html#switching-to-modular-daemons [2] https://src.fedoraproject.org/rpms/fedora-release/blob/rawhide/f/90-default.... -- Andrea Bolognani / Red Hat / Virtualization
On 6/26/23 10:06, Andrea Bolognani wrote:
On Fri, Jun 16, 2023 at 03:57:37PM -0600, Jim Fehlig wrote:
On 6/16/23 02:12, Martin Kletzander wrote:
On Wed, Jun 14, 2023 at 04:45:06PM -0600, Jim Fehlig wrote:
On 6/9/23 03:05, Andrea Bolognani wrote:
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?
The current logic in all packages' %post scriptlet basically translates to: if installing the package from scratch, apply the systemd presets; if upgrading, leave things well alone.
What I would want the witness file to indicate would be: libvirt-daemon is being upgraded, so when running the %post for libvirt-daemon-proxy behave as in the upgrade scenario as opposed to the install-from-scratch scenario.
Is it enough to know that libvirt-daemon is being upgraded? Do we also need to know if libvirtd.service and any of the sockets are enabled? I think Martin already suggested a solution along these lines https://listman.redhat.com/archives/libvir-list/2023-June/240312.html
Detecting whether the monolithic libvirtd and its sockets are enabled could also work, but feels more fragile. Specifically, it would not cover the scenario described by Martin, where the deployment is a split daemon one but with some units that are disabled by default based on presets have been manually enabled by the admin.
Nod. I even apologized to Martin in another response for getting bogged down on your issue :-).
The problem is that I'm not sure we can create and process such a witness file reliably. Figuring that out requires research and real-life testing :)
In an attempt to reproduce Andrea's report, I added the following presets to /usr/lib/systemd/system-preset/90-default-openSUSE.preset
enable virtqemud.socket enable virtlogd.socket enable virtnetworkd.socket enable virtnodedevd.socket enable virtstoraged.socket enable virtsecretd.socket enable virtproxyd.socket
As before, installed packages without commit b1da03b5b3. I then disabled all the virt* presets (with exception of virtlogd) and enabled libvirtd.socket.
I'm not sure this is entirely relevant, but just for completeness' sake: in a monolithic daemon deployment, you want libvirtd.service in addition to its sockets to be enabled. This is needed to make sure domain autostart works as intended.
Thanks for the reminder! BTW, does the same apply for virtnetworkd and virtstoraged, which also have resources that can be autostarted?
Basically, starting from a split daemon deployment, you want to follow the steps outlined here[1] but in reverse.
After updating to packages containing commit b1da03b5b3, virtproxyd.socket was still disabled and libvirt.socket was enabled. No problem connecting to libvirtd, even after the service timeout.
I'll try to reproduce this myself later this week, but what you're describing doesn't match my expectations: virtproxyd.socket should have been enabled during installation.
I would have thought so too and was surprised by the results. That test was done in a VM. I just did the same test on the host (freshly updated Tumbleweed) with the same results.
Are the packages you're using for testing a direct rebuild of the spec file shipped upstream? Or have you integrated the changes into the openSUSE spec file somehow?
I've made the changes to the openSUSE Factory spec file, which closely tracks the upstream one.
Note that the scriptlets in the upstream spec file call out to some standard macros, and it's also possible that the implementation of said macros is not the same across Fedora and openSUSE.
The openSUSE scriptlets use the 'service_add_pre' macro from the systemd-rpm-macros package https://build.opensuse.org/package/view_file/Base:System/systemd-rpm-macros/... In the end, something like 'systemctl --no-reload preset $unit' is called.
Any tips on reproducing the issue? I must be missing something. What are the libvirt-related presets in Fedora? I couldn't easily find them. FYI, the Tumbleweed ones are here
https://build.opensuse.org/package/show/Base:System/systemd-presets-common-S... https://build.opensuse.org/package/show/Base:System/systemd-presets-branding...
They're part of the fedora-release package[2].
Thanks for the pointer and info!
Specifically, we have
# Virtualization driver specific daemons. Start by default at boot for VM # autostart, but shutdown after 2 mins and socket activated thereafter enable virtqemud.service enable virtxend.service enable virtlxcd.service
# Compatibility with libvirtd sockets for old clients and expose TCP sockets enable virtproxyd.socket
# Secondary drivers providing supporting functionality to main virtualization # drivers, socket activated only when required enable virtinterfaced.socket enable virtnetworkd.socket enable virtnodedevd.socket enable virtnwfilterd.socket enable virtsecretd.socket enable virtstoraged.socket
In the past I've never added libvirt services/sockets to the openSUSE presets since users could easily enable libvirtd and virtlogd and be done with it. (Note the SLES presets do enable libvirtd and virtlogd.) But it becomes more burdensome with modular daemons, so I plan to add the above presets (excluding virtproxyd) to the openSUSE defaults.
Are you okay with old(ish) clients not being able to connect to the host? Because that's why the virtproxyd.socket is enabled by default in Fedora: if the client predates the introduction of split daemons it will try to connect to the old libvirtd socket, and in a split daemons deployment they only way to make that work is by having virtproxyd listening on it.
Understood, and I'm fine with that on Tumbleweed for the time being. It would be interesting to hear complaints so I know there's demand for the use-case. Regards, Jim
On Mon, Jun 26, 2023 at 04:16:33PM -0600, Jim Fehlig wrote:
On 6/26/23 10:06, Andrea Bolognani wrote:
The current logic in all packages' %post scriptlet basically translates to: if installing the package from scratch, apply the systemd presets; if upgrading, leave things well alone.
What I would want the witness file to indicate would be: libvirt-daemon is being upgraded, so when running the %post for libvirt-daemon-proxy behave as in the upgrade scenario as opposed to the install-from-scratch scenario.
Is it enough to know that libvirt-daemon is being upgraded? Do we also need to know if libvirtd.service and any of the sockets are enabled? I think Martin already suggested a solution along these lines
https://listman.redhat.com/archives/libvir-list/2023-June/240312.html
If we can detect this upgrade scenario reliably and behave accordingly, I don't think looking at whether any specific service is enabled or running would be necessary and, as I mentioned below, I think this would make for an overall more solid approach. I'm just still unclear on whether that's actually feasible :)
Detecting whether the monolithic libvirtd and its sockets are enabled could also work, but feels more fragile. Specifically, it would not cover the scenario described by Martin, where the deployment is a split daemon one but with some units that are disabled by default based on presets have been manually enabled by the admin.
Nod. I even apologized to Martin in another response for getting bogged down on your issue :-).
I believe the two are actually one and the same, since they're both a consequence of the package manager being convinced that virtproxyd needs to be handled as a newly installed service when it's in fact simply being upgraded.
I'm not sure this is entirely relevant, but just for completeness' sake: in a monolithic daemon deployment, you want libvirtd.service in addition to its sockets to be enabled. This is needed to make sure domain autostart works as intended.
Thanks for the reminder! BTW, does the same apply for virtnetworkd and virtstoraged, which also have resources that can be autostarted?
I'm actually not sure. The Fedora presets seem to indicate it does not, but I can't really rule out that simply being a bug :)
After updating to packages containing commit b1da03b5b3, virtproxyd.socket was still disabled and libvirt.socket was enabled. No problem connecting to libvirtd, even after the service timeout.
I'll try to reproduce this myself later this week, but what you're describing doesn't match my expectations: virtproxyd.socket should have been enabled during installation.
I would have thought so too and was surprised by the results. That test was done in a VM. I just did the same test on the host (freshly updated Tumbleweed) with the same results.
I have managed to reproduce the issue on Fedora 38, with the following steps: * build the libvirt 9.0.0 RPMs from upstream sources and use them to create a very minimal QEMU installation consisting of just libvirt-client libvirt-daemon libvirt-daemon-driver-qemu libvirt-libs * reboot the machine, verify that a basic VM can be started; * turn the deployment into a monolithic one by running # systemctl disable --now virtqemud virtqemud{,-ro,-admin}.socket # systemctl mask virtqemud virtqemud{,-ro,-admin}.socket # systemctl disable --now virtproxyd virtproxyd{,-ro,-admin}.socket # systemctl mask virtproxyd virtproxyd{,-ro,-admin}.socket # systemctl unmask libvirtd libvirtd{,-ro,-admin}.socket # systemctl enable libvirtd libvirtd{,-ro,-admin}.socket * reboot the machine again and repeat the smoke test; * build the libvirt 9.1.0 RPMs from upstream sources and upgrade. As expected, this will result in a few new packages being added to the system: libvirt-client libvirt-daemon libvirt-daemon-common libvirt-daemon-driver-qemu libvirt-daemon-lock libvirt-daemon-log libvirt-daemon-plugin-lockd libvirt-daemon-proxy libvirt-libs * reboot the machine and try again. Things will actually still work! But there's a somewhat subtle reason for that ;) During the upgrade, the following was printed: Running scriptlet: libvirt-daemon-proxy-9.1.0-1.fc38.x86_64 Installing : libvirt-daemon-proxy-9.1.0-1.fc38.x86_64 Running scriptlet: libvirt-daemon-proxy-9.1.0-1.fc38.x86_64 Failed to preset unit: Unit file /etc/systemd/system/virtproxyd.socket is masked. That's because I had *masked* all the split daemons in addition to disabling them, as suggested on [1]. If I take a more naive approach and simply disable the units, then I get Running scriptlet: libvirt-daemon-proxy-9.1.0-1.fc38.x86_64 Installing : libvirt-daemon-proxy-9.1.0-1.fc38.x86_64 Running scriptlet: libvirt-daemon-proxy-9.1.0-1.fc38.x86_64 Created symlink /etc/systemd/system/sockets.target.wants/virtproxyd.socket → /usr/lib/systemd/system/virtproxyd.socket. during upgrade and after a reboot libvirtd is unusable, manifesting the issue I was talking about. Now, we could simply tell users that they need to disable things properly O:-) but that's clearly not very helpful. It also wouldn't do anything to solve Martin's issue. Interestingly, I also get Running scriptlet: libvirt-daemon-log-9.1.0-1.fc38.x86_64 Installing : libvirt-daemon-log-9.1.0-1.fc38.x86_64 Running scriptlet: libvirt-daemon-log-9.1.0-1.fc38.x86_64 Removed "/etc/systemd/system/sockets.target.wants/virtlogd.socket". Running scriptlet: libvirt-daemon-lock-9.1.0-1.fc38.x86_64 Installing : libvirt-daemon-lock-9.1.0-1.fc38.x86_64 Running scriptlet: libvirt-daemon-lock-9.1.0-1.fc38.x86_64 Removed "/etc/systemd/system/sockets.target.wants/virtlockd.socket". because, just as is the case for virtproxyd, the package manager is convinced that these are completely new services and so it applies the presets for them. However, libvirtd.service and virtqemud.service both contain [Unit] Requires=virtlogd.socket Wants=virtlockd.socket [Install] Also=virtlogd.socket Also=virtlockd.socket which I guess ultimately takes precedence over the preset, and the sockets end up remaining enabled after all. Regardless, it would be probably a good idea to apply whatever approach we end up taking for virtproxyd to these two services too.
Are the packages you're using for testing a direct rebuild of the spec file shipped upstream? Or have you integrated the changes into the openSUSE spec file somehow?
I've made the changes to the openSUSE Factory spec file, which closely tracks the upstream one.
Note that the scriptlets in the upstream spec file call out to some standard macros, and it's also possible that the implementation of said macros is not the same across Fedora and openSUSE.
The openSUSE scriptlets use the 'service_add_pre' macro from the systemd-rpm-macros package
https://build.opensuse.org/package/view_file/Base:System/systemd-rpm-macros/...
In the end, something like 'systemctl --no-reload preset $unit' is called.
There might be some additional nuance that we're missing. I will try to play around with openSUSE and understand the situation better tomorrow. [1] https://libvirt.org/daemons.html#switching-to-modular-daemons -- Andrea Bolognani / Red Hat / Virtualization
On Tue, Jun 27, 2023 at 09:21:55AM -0700, Andrea Bolognani wrote:
On Mon, Jun 26, 2023 at 04:16:33PM -0600, Jim Fehlig wrote:
On 6/26/23 10:06, Andrea Bolognani wrote:
Note that the scriptlets in the upstream spec file call out to some standard macros, and it's also possible that the implementation of said macros is not the same across Fedora and openSUSE.
The openSUSE scriptlets use the 'service_add_pre' macro from the systemd-rpm-macros package
https://build.opensuse.org/package/view_file/Base:System/systemd-rpm-macros/...
In the end, something like 'systemctl --no-reload preset $unit' is called.
There might be some additional nuance that we're missing. I will try to play around with openSUSE and understand the situation better tomorrow.
Yup, that was exactly it! I'll omit a bunch of details for readability and because I have to get off the computer shortly :) but you should still get the gist. In Fedora (and I guess upstream systemd) the %systemd_post RPM macro is defined as if [ $1 -eq 1 ]; then \ systemd-update-helper install-system-units %{?*}; \ fi where the install-systemd-units command is implemented as systemctl --no-reload preset "$@" So whenever a new package is installed, the preset is going to be applied to all the units passed to the macro; subsequent runs of the scriptlet, during upgrade, will leave things alone. In openSUSE (I'm talking about Tumbleweed specifically, but Leap implements the same idea, just in a slightly different way) things work differently: the %service_add_pre and %service_add_post macros call systemd-update-helper mark-install-system-units %{?*} and systemd-update-helper install-system-units %{?*} respectively, and those two helper commands are implemented as for unit in "$@" ; do if [ ! -e /usr/lib/systemd/system/"$unit" ]; then touch /run/systemd/rpm/needs-preset/"$unit" fi done and for unit in "$@" ; do if [ -e /run/systemd/rpm/needs-preset/"$unit" ]; then rm /run/systemd/rpm/needs-preset/"$unit" systemctl --no-reload preset "$unit" fi done So at this point it should be obvious why you were unable to reproduce the issue on openSUSE: while in Fedora the decision about whether to apply the presets to a unit is based entirely upon whether the package that ships it is being installed from scratch or is an update, openSUSE looks at whether the unit existed on the machine before the package was installed and only applies presets for units that are newly introduced. Well, that, and having all units disabled by default in the presets, of course :) It seems to me that the openSUSE approach is far superior to the Fedora / upstream one, because as we've seen it deals gracefully with units being moved between packages. And that's not the only scenario: if we were to introduce a new unit to any existing package, for example, Fedora would not enable it by default regardless of presets. In the immediate future, I think we should look into implementing a similar logic to that of openSUSE in the libvirt spec. This would take care of solving both of the issue that I have reported and the one that Martin has. In the longer run, I think it would make a lot of sense to advocate for this approach to be adopted in Fedora and systemd upstream, which would both reduce our maintenance burden and ensure as many other projects as possible can benefit from it. -- Andrea Bolognani / Red Hat / Virtualization
On 6/30/23 10:58, Andrea Bolognani wrote:
On Tue, Jun 27, 2023 at 09:21:55AM -0700, Andrea Bolognani wrote:
On Mon, Jun 26, 2023 at 04:16:33PM -0600, Jim Fehlig wrote:
On 6/26/23 10:06, Andrea Bolognani wrote:
Note that the scriptlets in the upstream spec file call out to some standard macros, and it's also possible that the implementation of said macros is not the same across Fedora and openSUSE.
The openSUSE scriptlets use the 'service_add_pre' macro from the systemd-rpm-macros package
https://build.opensuse.org/package/view_file/Base:System/systemd-rpm-macros/...
In the end, something like 'systemctl --no-reload preset $unit' is called.
There might be some additional nuance that we're missing. I will try to play around with openSUSE and understand the situation better tomorrow.
Yup, that was exactly it!
I'll omit a bunch of details for readability and because I have to get off the computer shortly :) but you should still get the gist.
In Fedora (and I guess upstream systemd) the %systemd_post RPM macro is defined as
if [ $1 -eq 1 ]; then \ systemd-update-helper install-system-units %{?*}; \ fi
where the install-systemd-units command is implemented as
systemctl --no-reload preset "$@"
So whenever a new package is installed, the preset is going to be applied to all the units passed to the macro; subsequent runs of the scriptlet, during upgrade, will leave things alone.
In openSUSE (I'm talking about Tumbleweed specifically, but Leap implements the same idea, just in a slightly different way) things work differently: the %service_add_pre and %service_add_post macros call
systemd-update-helper mark-install-system-units %{?*}
and
systemd-update-helper install-system-units %{?*}
respectively, and those two helper commands are implemented as
for unit in "$@" ; do if [ ! -e /usr/lib/systemd/system/"$unit" ]; then touch /run/systemd/rpm/needs-preset/"$unit" fi done
and
for unit in "$@" ; do if [ -e /run/systemd/rpm/needs-preset/"$unit" ]; then rm /run/systemd/rpm/needs-preset/"$unit" systemctl --no-reload preset "$unit" fi done
I followed the paper trail here and copied that 'systemctl' command in a previous response, but shame on my laziness and not looking at the pre macro to see the bigger picture. Once again, thanks for the snooping.
So at this point it should be obvious why you were unable to reproduce the issue on openSUSE: while in Fedora the decision about whether to apply the presets to a unit is based entirely upon whether the package that ships it is being installed from scratch or is an update, openSUSE looks at whether the unit existed on the machine before the package was installed and only applies presets for units that are newly introduced.
Well, that, and having all units disabled by default in the presets, of course :)
It seems to me that the openSUSE approach is far superior to the Fedora / upstream one, because as we've seen it deals gracefully with units being moved between packages. And that's not the only scenario: if we were to introduce a new unit to any existing package, for example, Fedora would not enable it by default regardless of presets.
Yep, understood. The openSUSE approach is more flexible and avoids per-package hacks to handle such scenarios.
In the immediate future, I think we should look into implementing a similar logic to that of openSUSE in the libvirt spec. This would take care of solving both of the issue that I have reported and the one that Martin has.
Agreed. Were you planning to work on that? If you don't have time in the near future, I should have some cycles next week after the US holiday.
In the longer run, I think it would make a lot of sense to advocate for this approach to be adopted in Fedora and systemd upstream, which would both reduce our maintenance burden and ensure as many other projects as possible can benefit from it.
Agree with this too, but not volunteering to work on it :-). Regards, Jim
On Fri, Jun 30, 2023 at 01:46:05PM -0600, Jim Fehlig wrote:
On 6/30/23 10:58, Andrea Bolognani wrote:
In the immediate future, I think we should look into implementing a similar logic to that of openSUSE in the libvirt spec. This would take care of solving both of the issue that I have reported and the one that Martin has.
Agreed. Were you planning to work on that? If you don't have time in the near future, I should have some cycles next week after the US holiday.
Yeah, I'll take care of it. It only makes sense, considering that Fedora/RHEL are affected by the issue and openSUSE/SLES aren't.
In the longer run, I think it would make a lot of sense to advocate for this approach to be adopted in Fedora and systemd upstream, which would both reduce our maintenance burden and ensure as many other projects as possible can benefit from it.
Agree with this too, but not volunteering to work on it :-).
Fair enough :) -- Andrea Bolognani / Red Hat / Virtualization
On Mon, Jul 03, 2023 at 08:28:16AM +0000, Andrea Bolognani wrote:
On Fri, Jun 30, 2023 at 01:46:05PM -0600, Jim Fehlig wrote:
On 6/30/23 10:58, Andrea Bolognani wrote:
In the immediate future, I think we should look into implementing a similar logic to that of openSUSE in the libvirt spec. This would take care of solving both of the issue that I have reported and the one that Martin has.
Agreed. Were you planning to work on that? If you don't have time in the near future, I should have some cycles next week after the US holiday.
Yeah, I'll take care of it. It only makes sense, considering that Fedora/RHEL are affected by the issue and openSUSE/SLES aren't.
Done. https://listman.redhat.com/archives/libvir-list/2023-July/240723.html -- Andrea Bolognani / Red Hat / Virtualization
participants (4)
-
Andrea Bolognani -
Jim Fehlig -
Martin Kletzander -
Michal Prívozník