Re: [libvirt PATCH 5/8] rpm: Remove custom libvirtd restart logic

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.