This logic was necessary when socket activation was introduced
in libvirt 5.6.0/5.7.0 in order to guarantee smooth upgrades.
These days, even the oldest platform that we target ships a
version of libvirtd that implements socket activation, so the
additional code is no longer useful and we can treat libvirtd
the same as all other services.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
libvirt.spec.in | 34 +---------------------------------
1 file changed, 1 insertion(+), 33 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index c9317ed0cc..d09c3b3340 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1492,39 +1492,7 @@ fi \
%posttrans daemon
%libvirt_sysconfig_posttrans libvirtd
-if test %libvirt_daemon_needs_restart libvirtd
-then
- # See if user has previously modified their install to
- # tell libvirtd to use --listen
- grep -E '^LIBVIRTD_ARGS=.*--listen' /etc/sysconfig/libvirtd 1>/dev/null 2>&1
- if test $? = 0
- then
- # Then lets keep honouring --listen and *not* use
- # systemd socket activation, because switching things
- # might confuse mgmt tool like puppet/ansible that
- # expect the old style libvirtd
- /bin/systemctl mask \
- libvirtd.socket \
- libvirtd-ro.socket \
- libvirtd-admin.socket \
- libvirtd-tls.socket \
- libvirtd-tcp.socket >/dev/null 2>&1 || :
- /bin/systemctl try-restart libvirtd.service >/dev/null 2>&1 || :
- else
- # Old libvirtd owns the sockets and will delete them on
- # shutdown. Can't use a try-restart as libvirtd will simply
- # own the sockets again when it comes back up. Thus we must
- # do this particular ordering, so that we get libvirtd
- # running with socket activation in use
- /bin/systemctl stop libvirtd.service >/dev/null 2>&1 || :
- /bin/systemctl try-restart \
- libvirtd.socket \
- libvirtd-ro.socket \
- libvirtd-admin.socket >/dev/null 2>&1 || :
- /bin/systemctl start libvirtd.service >/dev/null 2>&1 || :
- fi
-fi
-%libvirt_daemon_finish_restart libvirtd
+%libvirt_daemon_perform_restart libvirtd
%preun daemon
%libvirt_daemon_systemd_preun_inet libvirtd
--
2.41.0On Fri, Jul 14, 2023 at 04:39:39PM +0200, Andrea Bolognani wrote: >This logic was necessary when socket activation was introduced >in libvirt 5.6.0/5.7.0 in order to guarantee smooth upgrades. > >These days, even the oldest platform that we target ships a >version of libvirtd that implements socket activation, so the >additional code is no longer useful and we can treat libvirtd >the same as all other services. > On top of that it was also a little bit fragile. At least from myself, I am fine with this. Reviewed-by: Martin Kletzander <mkletzan@redhat.com> >Signed-off-by: Andrea Bolognani <abologna@redhat.com> >--- > libvirt.spec.in | 34 +--------------------------------- > 1 file changed, 1 insertion(+), 33 deletions(-) > >diff --git a/libvirt.spec.in b/libvirt.spec.in >index c9317ed0cc..d09c3b3340 100644 >--- a/libvirt.spec.in >+++ b/libvirt.spec.in >@@ -1492,39 +1492,7 @@ fi \ > > %posttrans daemon > %libvirt_sysconfig_posttrans libvirtd >-if test %libvirt_daemon_needs_restart libvirtd >-then >- # See if user has previously modified their install to >- # tell libvirtd to use --listen >- grep -E '^LIBVIRTD_ARGS=.*--listen' /etc/sysconfig/libvirtd 1>/dev/null 2>&1 >- if test $? = 0 >- then >- # Then lets keep honouring --listen and *not* use >- # systemd socket activation, because switching things >- # might confuse mgmt tool like puppet/ansible that >- # expect the old style libvirtd >- /bin/systemctl mask \ >- libvirtd.socket \ >- libvirtd-ro.socket \ >- libvirtd-admin.socket \ >- libvirtd-tls.socket \ >- libvirtd-tcp.socket >/dev/null 2>&1 || : >- /bin/systemctl try-restart libvirtd.service >/dev/null 2>&1 || : >- else >- # Old libvirtd owns the sockets and will delete them on >- # shutdown. Can't use a try-restart as libvirtd will simply >- # own the sockets again when it comes back up. Thus we must >- # do this particular ordering, so that we get libvirtd >- # running with socket activation in use >- /bin/systemctl stop libvirtd.service >/dev/null 2>&1 || : >- /bin/systemctl try-restart \ >- libvirtd.socket \ >- libvirtd-ro.socket \ >- libvirtd-admin.socket >/dev/null 2>&1 || : >- /bin/systemctl start libvirtd.service >/dev/null 2>&1 || : >- fi >-fi >-%libvirt_daemon_finish_restart libvirtd >+%libvirt_daemon_perform_restart libvirtd > > %preun daemon > %libvirt_daemon_systemd_preun_inet libvirtd >-- >2.41.0 >
On Fri, Jul 14, 2023 at 04:39:39PM +0200, Andrea Bolognani wrote: > This logic was necessary when socket activation was introduced > in libvirt 5.6.0/5.7.0 in order to guarantee smooth upgrades. > > These days, even the oldest platform that we target ships a > version of libvirtd that implements socket activation, so the > additional code is no longer useful and we can treat libvirtd > the same as all other services. The upgrade path though can come from a platform that we don't support, but we do support upgrade from. eg we don't support RHEL-8, but upgrades from 8 -> 9 are supported. I think it is premature to declare this upgrade code no longer useful. > > Signed-off-by: Andrea Bolognani <abologna@redhat.com> > --- > libvirt.spec.in | 34 +--------------------------------- > 1 file changed, 1 insertion(+), 33 deletions(-) > > diff --git a/libvirt.spec.in b/libvirt.spec.in > index c9317ed0cc..d09c3b3340 100644 > --- a/libvirt.spec.in > +++ b/libvirt.spec.in > @@ -1492,39 +1492,7 @@ fi \ > > %posttrans daemon > %libvirt_sysconfig_posttrans libvirtd > -if test %libvirt_daemon_needs_restart libvirtd > -then > - # See if user has previously modified their install to > - # tell libvirtd to use --listen > - grep -E '^LIBVIRTD_ARGS=.*--listen' /etc/sysconfig/libvirtd 1>/dev/null 2>&1 > - if test $? = 0 > - then > - # Then lets keep honouring --listen and *not* use > - # systemd socket activation, because switching things > - # might confuse mgmt tool like puppet/ansible that > - # expect the old style libvirtd > - /bin/systemctl mask \ > - libvirtd.socket \ > - libvirtd-ro.socket \ > - libvirtd-admin.socket \ > - libvirtd-tls.socket \ > - libvirtd-tcp.socket >/dev/null 2>&1 || : > - /bin/systemctl try-restart libvirtd.service >/dev/null 2>&1 || : > - else > - # Old libvirtd owns the sockets and will delete them on > - # shutdown. Can't use a try-restart as libvirtd will simply > - # own the sockets again when it comes back up. Thus we must > - # do this particular ordering, so that we get libvirtd > - # running with socket activation in use > - /bin/systemctl stop libvirtd.service >/dev/null 2>&1 || : > - /bin/systemctl try-restart \ > - libvirtd.socket \ > - libvirtd-ro.socket \ > - libvirtd-admin.socket >/dev/null 2>&1 || : > - /bin/systemctl start libvirtd.service >/dev/null 2>&1 || : > - fi > -fi > -%libvirt_daemon_finish_restart libvirtd > +%libvirt_daemon_perform_restart libvirtd > > %preun daemon > %libvirt_daemon_systemd_preun_inet libvirtd > -- > 2.41.0 > With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Fri, Jul 14, 2023 at 03:45:11PM +0100, Daniel P. Berrangé wrote: > On Fri, Jul 14, 2023 at 04:39:39PM +0200, Andrea Bolognani wrote: > > This logic was necessary when socket activation was introduced > > in libvirt 5.6.0/5.7.0 in order to guarantee smooth upgrades. > > > > These days, even the oldest platform that we target ships a > > version of libvirtd that implements socket activation, so the > > additional code is no longer useful and we can treat libvirtd > > the same as all other services. > > The upgrade path though can come from a platform that we > don't support, but we do support upgrade from. > > eg we don't support RHEL-8, but upgrades from 8 -> 9 are > supported. I think it is premature to declare this upgrade > code no longer useful. We do target RHEL 8 still :) RHEL 8 got libvirt 6.0.0, which comes with socket activation support, back in 2020 with RHEL 8.3. Based on our support policy we only consider the latest point release a valid target anyway, but in this case we should absolutely be in the clear. -- Andrea Bolognani / Red Hat / Virtualization
On Fri, Jul 14, 2023 at 08:13:11AM -0700, Andrea Bolognani wrote: > On Fri, Jul 14, 2023 at 03:45:11PM +0100, Daniel P. Berrangé wrote: > > On Fri, Jul 14, 2023 at 04:39:39PM +0200, Andrea Bolognani wrote: > > > This logic was necessary when socket activation was introduced > > > in libvirt 5.6.0/5.7.0 in order to guarantee smooth upgrades. > > > > > > These days, even the oldest platform that we target ships a > > > version of libvirtd that implements socket activation, so the > > > additional code is no longer useful and we can treat libvirtd > > > the same as all other services. > > > > The upgrade path though can come from a platform that we > > don't support, but we do support upgrade from. > > > > eg we don't support RHEL-8, but upgrades from 8 -> 9 are > > supported. I think it is premature to declare this upgrade > > code no longer useful. > > We do target RHEL 8 still :) /facepalm > RHEL 8 got libvirt 6.0.0, which comes with socket activation support, > back in 2020 with RHEL 8.3. Based on our support policy we only > consider the latest point release a valid target anyway, but in this > case we should absolutely be in the clear. I'm still not convinced that makes this obsolete though. We introduced it but that's not ensuring every deployment is actually using it. We spent some time explaining to people how to stick with non-activation scenarios if they weren't ready to change things like ansible admin scripts from earlier RHEL-8.x Of course with any distro version at all, people could have turned off activation, but with RHEL-9 I'd be more comfortable saying it is unlikely because activation was the default from day 1, unlike 8. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Fri, Jul 14, 2023 at 04:25:48PM +0100, Daniel P. Berrangé wrote:
> On Fri, Jul 14, 2023 at 08:13:11AM -0700, Andrea Bolognani wrote:
> > RHEL 8 got libvirt 6.0.0, which comes with socket activation support,
> > back in 2020 with RHEL 8.3. Based on our support policy we only
> > consider the latest point release a valid target anyway, but in this
> > case we should absolutely be in the clear.
>
> I'm still not convinced that makes this obsolete though.
>
> We introduced it but that's not ensuring every deployment is actually
> using it. We spent some time explaining to people how to stick with
> non-activation scenarios if they weren't ready to change things like
> ansible admin scripts from earlier RHEL-8.x
>
> Of course with any distro version at all, people could have turned
> off activation, but with RHEL-9 I'd be more comfortable saying it
> is unlikely because activation was the default from day 1, unlike
> 8.
Just to make sure we're on the same page. The code I'm deleting looks
like this:
# See if user has previously modified their install to
# tell libvirtd to use --listen
grep -E '^LIBVIRTD_ARGS=.*--listen' /etc/sysconfig/libvirtd 1>/dev/null 2>&1
if test $? = 0
then
# Then lets keep honouring --listen and *not* use
# systemd socket activation, because switching things
# might confuse mgmt tool like puppet/ansible that
# expect the old style libvirtd
/bin/systemctl mask \
libvirtd.socket \
libvirtd-ro.socket \
libvirtd-admin.socket \
libvirtd-tls.socket \
libvirtd-tcp.socket >/dev/null 2>&1 || :
/bin/systemctl try-restart libvirtd.service >/dev/null 2>&1 || :
else
# Old libvirtd owns the sockets and will delete them on
# shutdown. Can't use a try-restart as libvirtd will simply
# own the sockets again when it comes back up. Thus we must
# do this particular ordering, so that we get libvirtd
# running with socket activation in use
/bin/systemctl stop libvirtd.service >/dev/null 2>&1 || :
/bin/systemctl try-restart \
libvirtd.socket \
libvirtd-ro.socket \
libvirtd-admin.socket >/dev/null 2>&1 || :
/bin/systemctl start libvirtd.service >/dev/null 2>&1 || :
fi
The 'else' branch deals with switching libvirtd < 5.6.0 to use proper
socket activation. All of our targets come with much newer versions
of libvirt at this point, so either they already had socket
activation enabled out of the box or they must have been switched
over by now. Which makes at least this branch obsolete, agreed?
The 'if' branch deals with the scenario in which --listen has been
configured, which is a non-default configuration that we don't want
to mess with. Fair enough. But, for that configuration to work at
all, the various sockets must have already been masked, either by
some deploy-time automation or by this scriptlet running at some
point between when libvirt < 5.6.0 was installed and now. And since
we're not touching socket during upgrades from the regular
%libvirt_daemon_perform_restart macro anyway, we can't really mess
this stuff up.
So, what scenario are you concerned with?
--
Andrea Bolognani / Red Hat / Virtualization
On Fri, Jul 14, 2023 at 08:44:04AM -0700, Andrea Bolognani wrote: > On Fri, Jul 14, 2023 at 04:25:48PM +0100, Daniel P. Berrangé wrote: > > On Fri, Jul 14, 2023 at 08:13:11AM -0700, Andrea Bolognani wrote: > > > RHEL 8 got libvirt 6.0.0, which comes with socket activation support, > > > back in 2020 with RHEL 8.3. Based on our support policy we only > > > consider the latest point release a valid target anyway, but in this > > > case we should absolutely be in the clear. > > > > I'm still not convinced that makes this obsolete though. > > > > We introduced it but that's not ensuring every deployment is actually > > using it. We spent some time explaining to people how to stick with > > non-activation scenarios if they weren't ready to change things like > > ansible admin scripts from earlier RHEL-8.x > > > > Of course with any distro version at all, people could have turned > > off activation, but with RHEL-9 I'd be more comfortable saying it > > is unlikely because activation was the default from day 1, unlike > > 8. > > Just to make sure we're on the same page. The code I'm deleting looks > like this: > > # See if user has previously modified their install to > # tell libvirtd to use --listen > grep -E '^LIBVIRTD_ARGS=.*--listen' /etc/sysconfig/libvirtd 1>/dev/null 2>&1 > if test $? = 0 > then > # Then lets keep honouring --listen and *not* use > # systemd socket activation, because switching things > # might confuse mgmt tool like puppet/ansible that > # expect the old style libvirtd > /bin/systemctl mask \ > libvirtd.socket \ > libvirtd-ro.socket \ > libvirtd-admin.socket \ > libvirtd-tls.socket \ > libvirtd-tcp.socket >/dev/null 2>&1 || : > /bin/systemctl try-restart libvirtd.service >/dev/null 2>&1 || : > else > # Old libvirtd owns the sockets and will delete them on > # shutdown. Can't use a try-restart as libvirtd will simply > # own the sockets again when it comes back up. Thus we must > # do this particular ordering, so that we get libvirtd > # running with socket activation in use > /bin/systemctl stop libvirtd.service >/dev/null 2>&1 || : > /bin/systemctl try-restart \ > libvirtd.socket \ > libvirtd-ro.socket \ > libvirtd-admin.socket >/dev/null 2>&1 || : > /bin/systemctl start libvirtd.service >/dev/null 2>&1 || : > fi > > The 'else' branch deals with switching libvirtd < 5.6.0 to use proper > socket activation. All of our targets come with much newer versions > of libvirt at this point, so either they already had socket > activation enabled out of the box or they must have been switched > over by now. Which makes at least this branch obsolete, agreed? > > The 'if' branch deals with the scenario in which --listen has been > configured, which is a non-default configuration that we don't want > to mess with. Fair enough. But, for that configuration to work at > all, the various sockets must have already been masked, either by > some deploy-time automation or by this scriptlet running at some > point between when libvirt < 5.6.0 was installed and now. And since > we're not touching socket during upgrades from the regular > %libvirt_daemon_perform_restart macro anyway, we can't really mess > this stuff up. Ok, so its only a problem if they are still running < 5.6.0 when they upgrade, which is probably an unreasonably upgrade gap to be credibly concerned about. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
© 2016 - 2026 Red Hat, Inc.