9:31 a.m.
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>
---
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
2:55 a.m.
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(a)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(a)redhat.com>
Michal
3:05 a.m.
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(a)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(a)redhat.com>
Thank you, I'll give Jim (original author) and others some time to chime
in too.
Michal
9:52 a.m.
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>
---
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
1:35 p.m.
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(a)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
4:05 a.m.
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
10:05 a.m.
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/#_sa...
> 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
5:45 p.m.
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
3:12 a.m.
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
4:57 p.m.
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-commo...
https://build.opensuse.org/package/show/Base:System/systemd-presets-brand...
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
11:06 a.m.
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-commo...
https://build.opensuse.org/package/show/Base:System/systemd-presets-brand...
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-defau...
--
Andrea Bolognani / Red Hat / Virtualization
5:16 p.m.
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-macr...
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-commo...
>
https://build.opensuse.org/package/show/Base:System/systemd-presets-brand...
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
11:21 a.m.
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-macr...
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
11:58 a.m.
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-macr...
>
> 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
2:46 p.m.
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-macr...
>>
>> 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
3:28 a.m.
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
9:42 a.m.
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
496
days inactive
533
days old
16 comments
4 participants
participants (4)
-
Andrea Bolognani -
Jim Fehlig -
Martin Kletzander -
Michal Prívozník