Re: [libvirt] [PATCH] rpm: explicitly enable & start virlockd/virtlogd sockets on install

On Mon, Dec 07, 2015 at 01:57:20PM +0100, Michal Privoznik wrote: > On 07.12.2015 13:55, Daniel P. Berrange wrote: >> When installing the libvirt-daemon RPM, we have a %post rule to >> enable the libvirtd.service, virtlockd.socket and virtlogd.socket >> files. This is only done, however, when the RPM is first installed, >> not when upgrading RPMs. >> >> This adds a separate rule to explicitly enable the virtlockd.socket >> and virtlogd.socket units upon RPM upgrade, if libvirtd.service >> is marked as enabled. It also adds a rule to start the socket >> units if libvirtd.service is running. >> >> The downside with taking this approach is that it is impossible >> for a user to have libvirtd.service enabled and disable socket >> activation, because we'll re-enable it on every upgrade. Similarly >> we'll be starting the socket listeners on every upgrade if libvirtd >> is running. This is a pretty nasty thing to have to keep in libvirt >> for the long term, given this upgrade problem is a one-time only >> pain point. >> >> For this reason I am recommending *NOT* applying this patch to GIT. >> >> Instead, I think we just have to release note the fact that after >> upgrading you should enable the virtlockd.socket unit. >> >> --- >> libvirt.spec.in | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/libvirt.spec.in b/libvirt.spec.in >> index 1f76f24..48e5632 100644 >> --- a/libvirt.spec.in >> +++ b/libvirt.spec.in >> @@ -1703,6 +1703,18 @@ if [ $1 -eq 1 ] ; then >> libvirtd.service >/dev/null 2>&1 || : >> fi >> %endif >> + >> +# In upgrade scenario we must explicitly enable virtlockd/virtlogd >> +# sockets, if libvirtd is already enabled and start them if >> +# libvirtd is running, otherwise you'll get failures to start >> +# guests >> +if [ $1 -ge 1 ] ; then >> + /bin/systemctl is-enabled libvirtd.service 1>/dev/null 2>&1 && >> + /bin/systemctl enable virtlockd.socket virtlogd.socket >> + /bin/systemctl is-active libvirtd.service 1>/dev/null 2>&1 && >> + /bin/systemctl start virtlockd.socket virtlogd.socket >> +fi >> + >> %else >> %if %{with_cgconfig} >> # Starting with Fedora 16/RHEL-7, systemd automounts all cgroups, >> > > ACK Presumably you didn't read my commit message, or are you ACK'ing my suggestion that we *NOT* merge this ?