[libvirt] [PATCH 0/4] remote: better handle system activation and upgrades
This improves the upgrade path to systemd socket activation - Disable socket activation in RPM %post if we see use of --listen arg on existing install - Report fatal error if --listen is used with socket activation, since it is not honoured Daniel P. Berrangé (4): remote: use Wants instead of Requires for libvirtd sockets remote: move timeout arg into sysconf file remote: forbid the --listen arg when systemd socket activation rpm: don't enable socket activation in upgrade if --listen present libvirt.spec.in | 44 ++++++++++++++++++++++++---------- src/remote/libvirtd.pod | 33 ++++++++++++++++++++++++- src/remote/libvirtd.service.in | 15 ++++++------ src/remote/libvirtd.sysconf | 12 +++++++--- src/remote/remote_daemon.c | 7 ++++++ 5 files changed, 86 insertions(+), 25 deletions(-) -- 2.21.0
To facilitate upgrades from earlier versions of libvirt which did not use socket activation for libvirtd, we want to allow the libvirtd socket units to be disabled (masked). This can only be supported if we use the warker Wants statement instead of Requires. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/remote/libvirtd.service.in | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/remote/libvirtd.service.in b/src/remote/libvirtd.service.in index 4c5b28b478..82892b4f70 100644 --- a/src/remote/libvirtd.service.in +++ b/src/remote/libvirtd.service.in @@ -2,9 +2,12 @@ Description=Virtualization daemon Requires=virtlogd.socket Requires=virtlockd.socket -Requires=libvirtd.socket -Requires=libvirtd-ro.socket -Requires=libvirtd-admin.socket +# Use Wants instead of Requires so that users +# can disable these three .socket units to revert +# to a traditional non-activation deployment setup +Wants=libvirtd.socket +Wants=libvirtd-ro.socket +Wants=libvirtd-admin.socket Wants=systemd-machined.service Before=libvirt-guests.service After=network.target -- 2.21.0
On Fri, Aug 23, 2019 at 04:11:40PM +0100, Daniel P. Berrangé wrote:
To facilitate upgrades from earlier versions of libvirt which did not use socket activation for libvirtd, we want to allow the libvirtd socket units to be disabled (masked). This can only be supported if we use the warker Wants statement instead of Requires.
s/warker/weaker/
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/remote/libvirtd.service.in | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
We need to give users the ability to customize the length of the shutdown timeout, or even disable timeouts entirely. Thus we must move the timeout arg into the sysconf file, instead of the service unit. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/remote/libvirtd.service.in | 6 +----- src/remote/libvirtd.sysconf | 12 +++++++++--- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/remote/libvirtd.service.in b/src/remote/libvirtd.service.in index 82892b4f70..9c8c54a2ef 100644 --- a/src/remote/libvirtd.service.in +++ b/src/remote/libvirtd.service.in @@ -26,11 +26,7 @@ Documentation=https://libvirt.org [Service] Type=notify EnvironmentFile=-@sysconfdir@/sysconfig/libvirtd -# libvirtd.service is set to run on boot so that autostart of -# VMs can be performed. We don't want it to stick around if -# unused though, so we set a timeout. The socket activation -# then ensures it gets started again if anything needs it -ExecStart=@sbindir@/libvirtd --timeout 120 $LIBVIRTD_ARGS +ExecStart=@sbindir@/libvirtd $LIBVIRTD_ARGS ExecReload=/bin/kill -HUP $MAINPID KillMode=process Restart=on-failure diff --git a/src/remote/libvirtd.sysconf b/src/remote/libvirtd.sysconf index 5969518bf2..2ad1fcf5d5 100644 --- a/src/remote/libvirtd.sysconf +++ b/src/remote/libvirtd.sysconf @@ -1,8 +1,14 @@ # Customizations for the libvirtd.service systemd unit -# Listen for TCP/IP connections. This is not required if using systemd -# socket activation. -# NB. must setup TLS/SSL keys prior to using this +# Default behaviour is for libvirtd.service to start on boot +# so that VM autostart can be performed. We then want it to +# shutdown again if nothing was started and rely on systemd +# socket activation to start it again when some client app +# connects. +LIBVIRT_ARGS="--timeout 120" + +# If systemd socket activation is disabled, then the following +# can be used to listen on TCP/TLS sockets #LIBVIRTD_ARGS="--listen" # Override Kerberos service keytab for SASL/GSSAPI -- 2.21.0
On Fri, Aug 23, 2019 at 04:11:41PM +0100, Daniel P. Berrangé wrote:
We need to give users the ability to customize the length of the shutdown timeout, or even disable timeouts entirely. Thus we must move the timeout arg into the sysconf file, instead of the service unit.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/remote/libvirtd.service.in | 6 +----- src/remote/libvirtd.sysconf | 12 +++++++++--- 2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/src/remote/libvirtd.service.in b/src/remote/libvirtd.service.in index 82892b4f70..9c8c54a2ef 100644 --- a/src/remote/libvirtd.service.in +++ b/src/remote/libvirtd.service.in @@ -26,11 +26,7 @@ Documentation=https://libvirt.org [Service] Type=notify EnvironmentFile=-@sysconfdir@/sysconfig/libvirtd -# libvirtd.service is set to run on boot so that autostart of -# VMs can be performed. We don't want it to stick around if -# unused though, so we set a timeout. The socket activation -# then ensures it gets started again if anything needs it -ExecStart=@sbindir@/libvirtd --timeout 120 $LIBVIRTD_ARGS +ExecStart=@sbindir@/libvirtd $LIBVIRTD_ARGS ExecReload=/bin/kill -HUP $MAINPID KillMode=process Restart=on-failure diff --git a/src/remote/libvirtd.sysconf b/src/remote/libvirtd.sysconf index 5969518bf2..2ad1fcf5d5 100644 --- a/src/remote/libvirtd.sysconf +++ b/src/remote/libvirtd.sysconf @@ -1,8 +1,14 @@ # Customizations for the libvirtd.service systemd unit
-# Listen for TCP/IP connections. This is not required if using systemd -# socket activation. -# NB. must setup TLS/SSL keys prior to using this +# Default behaviour is for libvirtd.service to start on boot +# so that VM autostart can be performed. We then want it to +# shutdown again if nothing was started and rely on systemd +# socket activation to start it again when some client app +# connects. +LIBVIRT_ARGS="--timeout 120"
LIBVIRTD_ARGS
+ +# If systemd socket activation is disabled, then the following +# can be used to listen on TCP/TLS sockets #LIBVIRTD_ARGS="--listen"
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
When using systemd socket activation the --listen arg has no effect. This is confusing to users upgrading from previous versions of libvirt as their config is silently ignored. Turn use of --listen into a fatal error when sockets are passed from systemd. This helps the admin discover the change in behaviour and thus decide whether to stick with socket activation or revert to previous behaviour. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/remote/libvirtd.pod | 33 ++++++++++++++++++++++++++++++++- src/remote/remote_daemon.c | 7 +++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/src/remote/libvirtd.pod b/src/remote/libvirtd.pod index 4721e0f4ec..fa30d6a37a 100644 --- a/src/remote/libvirtd.pod +++ b/src/remote/libvirtd.pod @@ -30,6 +30,35 @@ and will be picked up automatically if their XML configuration has been defined. Any guests whose XML configuration has not been defined will be lost from the configuration. +=head1 SYSTEM SOCKET ACTIVATION + +The B<libvirtd> daemon is capable of starting in two modes. + +In the traditional mode, it will create and listen on UNIX sockets itself. +If the B<--listen> parameter is given, it will also listen on TCP/IP socket(s), +according to the B<listen_tcp> and B<listen_tls> options in +B</etc/libvirt/libvirtd.conf> + +In socket activation mode, it will rely on systemd to create and listen +on the UNIX, and optionally TCP/IP, sockets and pass them as pre-opened +file descriptors. In this mode, it is not permitted to pass the B<--listen> +parameter, and most of the socket related config options in +B</etc/libvirt/libvirtd.conf> will no longer have any effect. To enable +TCP or TLS sockets use either + +B<$ systemctl start libvirtd-tls.socket> + +Or + +B<$ systemctl start libvirtd-tcp.socket> + +Socket activation mode is generally the default when running on a host +OS that uses systemd. To revert to the traditional mode, all the socket +unit files must be masked: + +B<$ systemctl mask libvirtd.socket libvirtd-ro.socket \ + libvirtd-admin.socket libvirtd-tls.socket libvirtd-tcp.socket> + =head1 OPTIONS =over @@ -48,7 +77,9 @@ Use this configuration file, overriding the default value. =item B<-l, --listen> -Listen for TCP/IP connections. +Listen for TCP/IP connections. This should not be set if using systemd +socket activation. Instead activate the libvirtd-tls.socket or +libvirtd-tcp.socket unit files. =item B<-p, --pid-file> I<FILE> diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index 1138485870..3970db09c0 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -422,6 +422,13 @@ daemonSetupNetworking(virNetServerPtr srv, if (virSystemdGetActivation(actmap, ARRAY_CARDINALITY(actmap), &act) < 0) return -1; +#ifdef WITH_IP + if (act && ipsock) { + VIR_ERROR(_("--listen parameter not permitted with systemd activation sockets")); + return -1; + } +#endif /* ! WITH_IP */ + if (config->unix_sock_group) { if (virGetGroupID(config->unix_sock_group, &unix_sock_gid) < 0) return ret; -- 2.21.0
On Fri, Aug 23, 2019 at 04:11:42PM +0100, Daniel P. Berrangé wrote:
When using systemd socket activation the --listen arg has no effect. This is confusing to users upgrading from previous versions of libvirt as their config is silently ignored. Turn use of --listen into a fatal error when sockets are passed from systemd.
This helps the admin discover the change in behaviour and thus decide whether to stick with socket activation or revert to previous behaviour.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/remote/libvirtd.pod | 33 ++++++++++++++++++++++++++++++++- src/remote/remote_daemon.c | 7 +++++++ 2 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/src/remote/libvirtd.pod b/src/remote/libvirtd.pod index 4721e0f4ec..fa30d6a37a 100644 --- a/src/remote/libvirtd.pod +++ b/src/remote/libvirtd.pod @@ -30,6 +30,35 @@ and will be picked up automatically if their XML configuration has been defined. Any guests whose XML configuration has not been defined will be lost from the configuration.
+=head1 SYSTEM SOCKET ACTIVATION + +The B<libvirtd> daemon is capable of starting in two modes. + +In the traditional mode, it will create and listen on UNIX sockets itself. +If the B<--listen> parameter is given, it will also listen on TCP/IP socket(s), +according to the B<listen_tcp> and B<listen_tls> options in +B</etc/libvirt/libvirtd.conf> + +In socket activation mode, it will rely on systemd to create and listen +on the UNIX, and optionally TCP/IP, sockets and pass them as pre-opened +file descriptors. In this mode, it is not permitted to pass the B<--listen> +parameter, and most of the socket related config options in +B</etc/libvirt/libvirtd.conf> will no longer have any effect. To enable +TCP or TLS sockets use either + +B<$ systemctl start libvirtd-tls.socket> + +Or + +B<$ systemctl start libvirtd-tcp.socket> + +Socket activation mode is generally the default when running on a host +OS that uses systemd. To revert to the traditional mode, all the socket +unit files must be masked: + +B<$ systemctl mask libvirtd.socket libvirtd-ro.socket \ + libvirtd-admin.socket libvirtd-tls.socket libvirtd-tcp.socket> + =head1 OPTIONS
=over @@ -48,7 +77,9 @@ Use this configuration file, overriding the default value.
=item B<-l, --listen>
-Listen for TCP/IP connections. +Listen for TCP/IP connections. This should not be set if using systemd +socket activation. Instead activate the libvirtd-tls.socket or +libvirtd-tcp.socket unit files.
=item B<-p, --pid-file> I<FILE>
diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index 1138485870..3970db09c0 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -422,6 +422,13 @@ daemonSetupNetworking(virNetServerPtr srv, if (virSystemdGetActivation(actmap, ARRAY_CARDINALITY(actmap), &act) < 0) return -1;
+#ifdef WITH_IP + if (act && ipsock) { + VIR_ERROR(_("--listen parameter not permitted with systemd activation sockets"));
Would it be possible to somehow fit in a reference to the man page? Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
On Mon, Aug 26, 2019 at 04:06:47PM +0200, Ján Tomko wrote:
On Fri, Aug 23, 2019 at 04:11:42PM +0100, Daniel P. Berrangé wrote:
When using systemd socket activation the --listen arg has no effect. This is confusing to users upgrading from previous versions of libvirt as their config is silently ignored. Turn use of --listen into a fatal error when sockets are passed from systemd.
This helps the admin discover the change in behaviour and thus decide whether to stick with socket activation or revert to previous behaviour.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/remote/libvirtd.pod | 33 ++++++++++++++++++++++++++++++++- src/remote/remote_daemon.c | 7 +++++++ 2 files changed, 39 insertions(+), 1 deletion(-)
+#ifdef WITH_IP + if (act && ipsock) { + VIR_ERROR(_("--listen parameter not permitted with systemd activation sockets"));
Would it be possible to somehow fit in a reference to the man page?
Sure VIR_ERROR(_("--listen parameter not permitted with systemd activation " "sockets, please see 'man libvirtd' for further guidance."));
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jano
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 :|
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
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
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 :|
participants (2)
-
Daniel P. Berrangé -
Ján Tomko