[libvirt] [PATCH 4/4] rpm: don't enable socket activation in upgrade if --listen present

Daniel P. Berrangé posted 4 patches 6 years, 5 months ago
[libvirt] [PATCH 4/4] rpm: don't enable socket activation in upgrade if --listen present
Posted by Daniel P. Berrangé 6 years, 5 months ago
Currently during RPM upgrade we restart libvirtd and unconditionally
enable use of systemd socket activation for the UNIX sockets.

If the user had previously given the --listen arg to libvirtd though,
this will no longer be honoured if socket activation is used.

We could start libvirtd-tcp.socket or libvirtd-tls.socket for this,
but mgmt tools like puppet/ansible might not be expecting this.
So for now we silently disable socket activation if we see --listen
was previously set on the host.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 libvirt.spec.in | 44 +++++++++++++++++++++++++++++++-------------
 1 file changed, 31 insertions(+), 13 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index ee4b408510..e6c85a706b 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1379,19 +1379,37 @@ fi
 
 %posttrans daemon
 if [ -f %{_localstatedir}/lib/rpm-state/libvirt/restart ]; then
-    # 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
-    /bin/systemctl is-active libvirtd.service 1>/dev/null 2>&1
-    if test $? = 0 ; then
-        /bin/systemctl stop libvirtd.service >/dev/null 2>&1 || :
-
-        /bin/systemctl try-restart libvirtd.socket >/dev/null 2>&1 || :
-        /bin/systemctl try-restart libvirtd-ro.socket >/dev/null 2>&1 || :
-        /bin/systemctl try-restart libvirtd-admin.socket >/dev/null 2>&1 || :
-
-        /bin/systemctl start libvirtd.service >/dev/null 2>&1 || :
+    # 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 >/dev/null 2>&1 || :
+        /bin/systemctl mask libvirtd-ro.socket >/dev/null 2>&1 || :
+        /bin/systemctl mask libvirtd-admin.socket >/dev/null 2>&1 || :
+        /bin/systemctl mask libvirtd-tls.socket >/dev/null 2>&1 || :
+        /bin/systemctl mask libvirtd-tcp.socket >/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 is-active libvirtd.service 1>/dev/null 2>&1
+        if test $? = 0
+        then
+            /bin/systemctl stop libvirtd.service >/dev/null 2>&1 || :
+
+            /bin/systemctl try-restart libvirtd.socket >/dev/null 2>&1 || :
+            /bin/systemctl try-restart libvirtd-ro.socket >/dev/null 2>&1 || :
+            /bin/systemctl try-restart libvirtd-admin.socket >/dev/null 2>&1 || :
+
+            /bin/systemctl start libvirtd.service >/dev/null 2>&1 || :
+        fi
     fi
 fi
 rm -rf %{_localstatedir}/lib/rpm-state/libvirt || :
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] rpm: don't enable socket activation in upgrade if --listen present
Posted by Ján Tomko 6 years, 5 months ago
On Fri, Aug 23, 2019 at 04:11:43PM +0100, Daniel P. Berrangé wrote:
>Currently during RPM upgrade we restart libvirtd and unconditionally
>enable use of systemd socket activation for the UNIX sockets.
>
>If the user had previously given the --listen arg to libvirtd though,
>this will no longer be honoured if socket activation is used.
>
>We could start libvirtd-tcp.socket or libvirtd-tls.socket for this,
>but mgmt tools like puppet/ansible might not be expecting this.

In that case, wouldn't it be better to fail as early as possible? That is,
leave --listen in the config file and let libvirtd startup fail with
the error from the previous commit so that people know to fix their
scripts?

Otherwise this might bite them much later in the future when they need
to e.g. reinstall the VM instead of just upgrading.

>So for now we silently disable socket activation if we see --listen
>was previously set on the host.
>
>Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>---
> libvirt.spec.in | 44 +++++++++++++++++++++++++++++++-------------
> 1 file changed, 31 insertions(+), 13 deletions(-)
>

But doing this also should make sense to some people.
Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] rpm: don't enable socket activation in upgrade if --listen present
Posted by Daniel P. Berrangé 6 years, 5 months ago
On Mon, Aug 26, 2019 at 04:21:34PM +0200, Ján Tomko wrote:
> On Fri, Aug 23, 2019 at 04:11:43PM +0100, Daniel P. Berrangé wrote:
> > Currently during RPM upgrade we restart libvirtd and unconditionally
> > enable use of systemd socket activation for the UNIX sockets.
> > 
> > If the user had previously given the --listen arg to libvirtd though,
> > this will no longer be honoured if socket activation is used.
> > 
> > We could start libvirtd-tcp.socket or libvirtd-tls.socket for this,
> > but mgmt tools like puppet/ansible might not be expecting this.
> 
> In that case, wouldn't it be better to fail as early as possible? That is,
> leave --listen in the config file and let libvirtd startup fail with
> the error from the previous commit so that people know to fix their
> scripts?

Actively breaking a host during an "yum upgrade" is very undesirable
as that will cause service outage on the existing system.

> Otherwise this might bite them much later in the future when they need
> to e.g. reinstall the VM instead of just upgrading.

Yes, that is true. There is no answer here that is perfect.

When it fails at time of deploying a new host you are less likely to be
causing a production service outage though, as the service does not
yet exist on the host in question.

> > So for now we silently disable socket activation if we see --listen
> > was previously set on the host.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> > libvirt.spec.in | 44 +++++++++++++++++++++++++++++++-------------
> > 1 file changed, 31 insertions(+), 13 deletions(-)
> > 
> 
> But doing this also should make sense to some people.
> Reviewed-by: Ján Tomko <jtomko@redhat.com>

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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list