[libvirt] [PATCH 0/3] Couple of libvirt-guests fixes
*** BLURB HERE *** Michal Privoznik (3): virSystemdCreateMachine: Set dependencies for slices libvirt-guests: Wait for libirtd to initialize virNetServerRun: Notify systemd that we're accepting clients configure.ac | 2 ++ daemon/libvirtd.service.in | 1 + m4/virt-systemd-daemon.m4 | 34 ++++++++++++++++++++++++++++++++++ src/Makefile.am | 4 ++-- src/libvirt_private.syms | 1 + src/rpc/virnetserver.c | 7 +++++++ src/util/virsystemd.c | 18 ++++++++++++++++-- src/util/virsystemd.h | 2 ++ tools/libvirt-guests.sh.in | 19 +++++++++++++------ 9 files changed, 78 insertions(+), 10 deletions(-) create mode 100644 m4/virt-systemd-daemon.m4 -- 1.9.0
https://bugzilla.redhat.com/show_bug.cgi?id=1031696 When creating a new domain, we let systemd know about it by calling CreateMachine() function via dbus. Systemd then creates a scope and places domain into it. However, later when the host is shutting down, systemd computes the shutdown order to see what processes can be shut down in parallel. And since we were not setting dependencies at all, the slices (and thus domains) were most likely killed before libvirt-guests.service. So user domains that had to be saved, shut off, whatever were in fact killed. This problem can be solved by letting systemd know that scopes we're creating must not be killed before libvirt-guests.service. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virsystemd.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index 503fff7..9247c92 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -243,8 +243,10 @@ int virSystemdCreateMachine(const char *name, iscontainer ? "container" : "vm", (unsigned int)pidleader, rootdir ? rootdir : "", - 1, "Slice", "s", - slicename) < 0) + 3, + "Slice", "s", slicename, + "After", "as", 1, "libvirtd.service", + "Before", "as", 1, "libvirt-guests.service") < 0) goto cleanup; ret = 0; -- 1.9.0
On Fri, Feb 21, 2014 at 01:32:35PM +0100, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1031696
When creating a new domain, we let systemd know about it by calling CreateMachine() function via dbus. Systemd then creates a scope and places domain into it. However, later when the host is shutting down, systemd computes the shutdown order to see what processes can be shut down in parallel. And since we were not setting dependencies at all, the slices (and thus domains) were most likely killed before libvirt-guests.service. So user domains that had to be saved, shut off, whatever were in fact killed. This problem can be solved by letting systemd know that scopes we're creating must not be killed before libvirt-guests.service.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 02/21/2014 07:32 AM, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1031696
When creating a new domain, we let systemd know about it by calling CreateMachine() function via dbus. Systemd then creates a scope and places domain into it. However, later when the host is shutting down, systemd computes the shutdown order to see what processes can be shut down in parallel. And since we were not setting dependencies at all, the slices (and thus domains) were most likely killed before libvirt-guests.service. So user domains that had to be saved, shut off, whatever were in fact killed. This problem can be solved by letting systemd know that scopes we're creating must not be killed before libvirt-guests.service.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virsystemd.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index 503fff7..9247c92 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -243,8 +243,10 @@ int virSystemdCreateMachine(const char *name, iscontainer ? "container" : "vm", (unsigned int)pidleader, rootdir ? rootdir : "", - 1, "Slice", "s", - slicename) < 0) + 3, + "Slice", "s", slicename, + "After", "as", 1, "libvirtd.service", + "Before", "as", 1, "libvirt-guests.service") < 0) goto cleanup;
ret = 0;
Ahh, so this does indeed work all along and I just messed up the gvariant :/ ACK - Cole
I've noticed that in some cases systemd was quick enough and even if libvirt-guests.service is marked to be started after the libvirtd.service my guests were not resumed as libvirt-guests.sh failed to connect. This is because of a simple fact: systemd correctly starts libvirt-guests after it execs libvirtd. However, the daemon is not able to accept connections right from the start. It's doing some initialization which may take ages. This problem is not limited to systemd only, indeed. Any init system that is able to startup services in parallel (e.g. OpenRC) may run into this situation. The fix is to try connecting not only once, but continuously a few times with a small sleep in between tries. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/libvirt-guests.sh.in | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/tools/libvirt-guests.sh.in b/tools/libvirt-guests.sh.in index 38e93c5..f14598e 100644 --- a/tools/libvirt-guests.sh.in +++ b/tools/libvirt-guests.sh.in @@ -37,6 +37,8 @@ SHUTDOWN_TIMEOUT=300 PARALLEL_SHUTDOWN=0 START_DELAY=0 BYPASS_CACHE=0 +CONNECT_RETRIES=10 +RETRIES_SLEEP=.5 test -f "$sysconfdir"/sysconfig/libvirt-guests && . "$sysconfdir"/sysconfig/libvirt-guests @@ -87,12 +89,17 @@ test_connect() { uri=$1 - run_virsh "$uri" connect 2>/dev/null - if [ $? -ne 0 ]; then - eval_gettext "Can't connect to \$uri. Skipping." - echo - return 1 - fi + for ((i = 0; i < ${CONNECT_RETRIES}; i++)); do + run_virsh "$uri" connect 2>/dev/null + if [ $? -eq 0 ]; then + return 0; + fi + sleep ${RETRIES_SLEEP} + eval_gettext "Unable to connect to libvirt currently. Retrying .. \$i" + done + eval_gettext "Can't connect to \$uri. Skipping." + echo + return 1 } # list_guests URI PERSISTENT -- 1.9.0
On Fri, Feb 21, 2014 at 01:32:36PM +0100, Michal Privoznik wrote:
I've noticed that in some cases systemd was quick enough and even if libvirt-guests.service is marked to be started after the libvirtd.service my guests were not resumed as libvirt-guests.sh failed to connect. This is because of a simple fact: systemd correctly starts libvirt-guests after it execs libvirtd. However, the daemon is not able to accept connections right from the start. It's doing some initialization which may take ages. This problem is not limited to systemd only, indeed. Any init system that is able to startup services in parallel (e.g. OpenRC) may run into this situation. The fix is to try connecting not only once, but continuously a few times with a small sleep in between tries.
The long term fix for this will be to use socket activation for libvirtd. Unfortunately we can't do that yet since then VMs marked to autostart on boot won't be booted ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 21.02.2014 13:50, Daniel P. Berrange wrote:
On Fri, Feb 21, 2014 at 01:32:36PM +0100, Michal Privoznik wrote:
I've noticed that in some cases systemd was quick enough and even if libvirt-guests.service is marked to be started after the libvirtd.service my guests were not resumed as libvirt-guests.sh failed to connect. This is because of a simple fact: systemd correctly starts libvirt-guests after it execs libvirtd. However, the daemon is not able to accept connections right from the start. It's doing some initialization which may take ages. This problem is not limited to systemd only, indeed. Any init system that is able to startup services in parallel (e.g. OpenRC) may run into this situation. The fix is to try connecting not only once, but continuously a few times with a small sleep in between tries.
The long term fix for this will be to use socket activation for libvirtd. Unfortunately we can't do that yet since then VMs marked to autostart on boot won't be booted
I think that we can create a .socket within libvirt and set libvirt-guests.service to be dependent on the libvirt.socket. Something among: diff --git a/daemon/libvirtd.service.in b/daemon/libvirtd.service.in index e1f2a07..35963ce 100644 --- a/daemon/libvirtd.service.in +++ b/daemon/libvirtd.service.in @@ -22,5 +22,9 @@ Restart=on-failure # Override the maximum number of opened files #LimitNOFILE=2048 +[Socket] +ListenStream=/var/run/libvirt/libvirt-sock +ListenStream=0.0.0.0:16509 + [Install] WantedBy=multi-user.target diff --git a/tools/libvirt-guests.service.in b/tools/libvirt-guests.service.in index d8d7adf..03120da 100644 --- a/tools/libvirt-guests.service.in +++ b/tools/libvirt-guests.service.in @@ -1,6 +1,6 @@ [Unit] Description=Suspend Active Libvirt Guests -After=network.target libvirtd.service +After=network.target libvirtd.socket Documentation=man:libvirtd(8) Documentation=http://libvirt.org My inspiration comes from here: https://bugzilla.redhat.com/show_bug.cgi?id=714426 However, I was unable to make this work with this particular change. Moreover, it would require users to adjust libvirtd.service file if they ever change socket path or listen address. That's why I've decided for approach implemented in 3/3. Michal
On Fri, Feb 21, 2014 at 02:25:24PM +0100, Michal Privoznik wrote:
On 21.02.2014 13:50, Daniel P. Berrange wrote:
On Fri, Feb 21, 2014 at 01:32:36PM +0100, Michal Privoznik wrote:
I've noticed that in some cases systemd was quick enough and even if libvirt-guests.service is marked to be started after the libvirtd.service my guests were not resumed as libvirt-guests.sh failed to connect. This is because of a simple fact: systemd correctly starts libvirt-guests after it execs libvirtd. However, the daemon is not able to accept connections right from the start. It's doing some initialization which may take ages. This problem is not limited to systemd only, indeed. Any init system that is able to startup services in parallel (e.g. OpenRC) may run into this situation. The fix is to try connecting not only once, but continuously a few times with a small sleep in between tries.
The long term fix for this will be to use socket activation for libvirtd. Unfortunately we can't do that yet since then VMs marked to autostart on boot won't be booted
I think that we can create a .socket within libvirt and set libvirt-guests.service to be dependent on the libvirt.socket. Something among:
diff --git a/daemon/libvirtd.service.in b/daemon/libvirtd.service.in index e1f2a07..35963ce 100644 --- a/daemon/libvirtd.service.in +++ b/daemon/libvirtd.service.in @@ -22,5 +22,9 @@ Restart=on-failure # Override the maximum number of opened files #LimitNOFILE=2048
+[Socket] +ListenStream=/var/run/libvirt/libvirt-sock +ListenStream=0.0.0.0:16509 + [Install] WantedBy=multi-user.target diff --git a/tools/libvirt-guests.service.in b/tools/libvirt-guests.service.in index d8d7adf..03120da 100644 --- a/tools/libvirt-guests.service.in +++ b/tools/libvirt-guests.service.in @@ -1,6 +1,6 @@ [Unit] Description=Suspend Active Libvirt Guests -After=network.target libvirtd.service +After=network.target libvirtd.socket Documentation=man:libvirtd(8) Documentation=http://libvirt.org
Hmm, i guess that would work in fact, provided we ensured that the main libvirtd.service was still always enabled by default too.
My inspiration comes from here:
https://bugzilla.redhat.com/show_bug.cgi?id=714426
However, I was unable to make this work with this particular change.
You need to also make libvirtd receive the socket file handle from systemd, and create the libvirtd.socket file too. See what the virtlockd daemon does in this regard.
Moreover, it would require users to adjust libvirtd.service file if they ever change socket path or listen address. That's why I've decided for approach implemented in 3/3.
Sure, patch 3 is fine regardless because it gives fine grained synchronization of startup beyond simply the socket availability. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
s/libirtd/libvirtd/ in the subject Jan
On 02/21/2014 05:32 AM, Michal Privoznik wrote: s/libirtd/libvirtd/ in subject
I've noticed that in some cases systemd was quick enough and even if libvirt-guests.service is marked to be started after the libvirtd.service my guests were not resumed as libvirt-guests.sh failed to connect. This is because of a simple fact: systemd correctly starts libvirt-guests after it execs libvirtd. However, the daemon is not able to accept connections right from the start. It's doing some initialization which may take ages. This problem is not limited to systemd only, indeed. Any init system that is able to startup services in parallel (e.g. OpenRC) may run into this situation. The fix is to try connecting not only once, but continuously a few times with a small sleep in between tries.
+++ b/tools/libvirt-guests.sh.in @@ -37,6 +37,8 @@ SHUTDOWN_TIMEOUT=300 PARALLEL_SHUTDOWN=0 START_DELAY=0 BYPASS_CACHE=0 +CONNECT_RETRIES=10 +RETRIES_SLEEP=.5
fractional second sleep is a GNU sleep extension; it is not portable to all platforms.
+ fi + sleep ${RETRIES_SLEEP}
You may need to rework this to be a bit more robust to a sleep failure, if sleep refuses to parse the fractional second. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 21.02.2014 18:28, Eric Blake wrote:
On 02/21/2014 05:32 AM, Michal Privoznik wrote:
s/libirtd/libvirtd/ in subject
I've noticed that in some cases systemd was quick enough and even if libvirt-guests.service is marked to be started after the libvirtd.service my guests were not resumed as libvirt-guests.sh failed to connect. This is because of a simple fact: systemd correctly starts libvirt-guests after it execs libvirtd. However, the daemon is not able to accept connections right from the start. It's doing some initialization which may take ages. This problem is not limited to systemd only, indeed. Any init system that is able to startup services in parallel (e.g. OpenRC) may run into this situation. The fix is to try connecting not only once, but continuously a few times with a small sleep in between tries.
+++ b/tools/libvirt-guests.sh.in @@ -37,6 +37,8 @@ SHUTDOWN_TIMEOUT=300 PARALLEL_SHUTDOWN=0 START_DELAY=0 BYPASS_CACHE=0 +CONNECT_RETRIES=10 +RETRIES_SLEEP=.5
fractional second sleep is a GNU sleep extension; it is not portable to all platforms.
+ fi + sleep ${RETRIES_SLEEP}
You may need to rework this to be a bit more robust to a sleep failure, if sleep refuses to parse the fractional second.
I've s/\.5/1/ and pushed. Thanks! Michal
Systemd does not forget about the cases, where client service needs to wait for daemon service to initialize and start accepting new clients. Setting a dependency in client is not enough as systemd doesn't know when the daemon has initialized itself and started accepting new clients. However, it offers a mechanism to solve this. The daemon needs to call a special systemd function by which the daemon tells "I'm ready to accept new clients". This is exactly what we need with libvirtd-guests (client) and libvirtd (daemon). So now, with this change, libvirt-guests.service is invoked not any sooner than libvirtd.service calls the systemd notify function. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- configure.ac | 2 ++ daemon/libvirtd.service.in | 1 + m4/virt-systemd-daemon.m4 | 34 ++++++++++++++++++++++++++++++++++ src/Makefile.am | 4 ++-- src/libvirt_private.syms | 1 + src/rpc/virnetserver.c | 7 +++++++ src/util/virsystemd.c | 12 ++++++++++++ src/util/virsystemd.h | 2 ++ 8 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 m4/virt-systemd-daemon.m4 diff --git a/configure.ac b/configure.ac index 9e76353..f30ac76 100644 --- a/configure.ac +++ b/configure.ac @@ -239,6 +239,7 @@ LIBVIRT_CHECK_SANLOCK LIBVIRT_CHECK_SASL LIBVIRT_CHECK_SELINUX LIBVIRT_CHECK_SSH2 +LIBVIRT_CHECK_SYSTEMD_DAEMON LIBVIRT_CHECK_UDEV LIBVIRT_CHECK_YAJL @@ -2773,6 +2774,7 @@ LIBVIRT_RESULT_SANLOCK LIBVIRT_RESULT_SASL LIBVIRT_RESULT_SELINUX LIBVIRT_RESULT_SSH2 +LIBVIRT_RESULT_SYSTEMD_DAEMON LIBVIRT_RESULT_UDEV LIBVIRT_RESULT_YAJL AC_MSG_NOTICE([ libxml: $LIBXML_CFLAGS $LIBXML_LIBS]) diff --git a/daemon/libvirtd.service.in b/daemon/libvirtd.service.in index dc2433a..e1f2a07 100644 --- a/daemon/libvirtd.service.in +++ b/daemon/libvirtd.service.in @@ -13,6 +13,7 @@ Documentation=man:libvirtd(8) Documentation=http://libvirt.org [Service] +Type=notify EnvironmentFile=-/etc/sysconfig/libvirtd ExecStart=@sbindir@/libvirtd $LIBVIRTD_ARGS ExecReload=/bin/kill -HUP $MAINPID diff --git a/m4/virt-systemd-daemon.m4 b/m4/virt-systemd-daemon.m4 new file mode 100644 index 0000000..8516e41 --- /dev/null +++ b/m4/virt-systemd-daemon.m4 @@ -0,0 +1,34 @@ +dnl The libsystemd-daemon.so library +dnl +dnl Copyright (C) 2012-2013 Red Hat, Inc. +dnl +dnl This library is free software; you can redistribute it and/or +dnl modify it under the terms of the GNU Lesser General Public +dnl License as published by the Free Software Foundation; either +dnl version 2.1 of the License, or (at your option) any later version. +dnl +dnl This library is distributed in the hope that it will be useful, +dnl but WITHOUT ANY WARRANTY; without even the implied warranty of +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +dnl Lesser General Public License for more details. +dnl +dnl You should have received a copy of the GNU Lesser General Public +dnl License along with this library. If not, see +dnl <http://www.gnu.org/licenses/>. +dnl + +AC_DEFUN([LIBVIRT_CHECK_SYSTEMD_DAEMON],[ + LIBVIRT_CHECK_PKG([SYSTEMD_DAEMON], [libsystemd-daemon], [0.27.1]) + + old_CFLAGS="$CFLAGS" + old_LIBS="$LIBS" + CFLAGS="$CFLAGS $SYSTEMD_DAEMON_CFLAGS" + LIBS="$LIBS $SYSTEMD_DAEMON_LIBS" + AC_CHECK_FUNCS([sd_notify]) + CFLAGS="$old_CFLAGS" + LIBS="$old_LIBS" +]) + +AC_DEFUN([LIBVIRT_RESULT_SYSTEMD_DAEMON],[ + LIBVIRT_RESULT_LIB([SYSTEMD_DAEMON]) +]) diff --git a/src/Makefile.am b/src/Makefile.am index 6d21e5d..6ef32ee 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -958,11 +958,11 @@ libvirt_util_la_SOURCES = \ libvirt_util_la_CFLAGS = $(CAPNG_CFLAGS) $(YAJL_CFLAGS) $(LIBNL_CFLAGS) \ $(AM_CFLAGS) $(AUDIT_CFLAGS) $(DEVMAPPER_CFLAGS) \ $(DBUS_CFLAGS) $(LDEXP_LIBM) $(NUMACTL_CFLAGS) \ - -I$(top_srcdir)/src/conf + $(SYSTEMD_DAEMON_CFLAGS) -I$(top_srcdir)/src/conf libvirt_util_la_LIBADD = $(CAPNG_LIBS) $(YAJL_LIBS) $(LIBNL_LIBS) \ $(THREAD_LIBS) $(AUDIT_LIBS) $(DEVMAPPER_LIBS) \ $(LIB_CLOCK_GETTIME) $(DBUS_LIBS) $(MSCOM_LIBS) $(LIBXML_LIBS) \ - $(SECDRIVER_LIBS) $(NUMACTL_LIBS) + $(SECDRIVER_LIBS) $(NUMACTL_LIBS) $(SYSTEMD_DAEMON_LIBS) noinst_LTLIBRARIES += libvirt_conf.la diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0896287..21c6b66 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1826,6 +1826,7 @@ virSystemdCreateMachine; virSystemdMakeMachineName; virSystemdMakeScopeName; virSystemdMakeSliceName; +virSystemdNotifyStartup; virSystemdTerminateMachine; diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 8907768..be27913 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -38,6 +38,7 @@ #include "virnetservermdns.h" #include "virdbus.h" #include "virstring.h" +#include "virsystemd.h" #ifndef SA_SIGINFO # define SA_SIGINFO 0 @@ -1085,6 +1086,12 @@ void virNetServerRun(virNetServerPtr srv) goto cleanup; } +#ifdef WITH_SYSTEMD_DAEMON + /* We are accepting connections now. Notify systemd + * so it can start dependent services. */ + virSystemdNotifyStartup(); +#endif + VIR_DEBUG("srv=%p quit=%d", srv, srv->quit); while (!srv->quit) { /* A shutdown timeout is specified, so check diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index 9247c92..8adf209 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -21,6 +21,10 @@ #include <config.h> +#ifdef WITH_SYSTEMD_DAEMON +# include <systemd/sd-daemon.h> +#endif + #include "virsystemd.h" #include "virdbus.h" #include "virstring.h" @@ -304,3 +308,11 @@ cleanup: VIR_FREE(machinename); return ret; } + +void +virSystemdNotifyStartup(void) +{ +#ifdef WITH_SYSTEMD_DAEMON + sd_notify(0, "READY=1"); +#endif +} diff --git a/src/util/virsystemd.h b/src/util/virsystemd.h index d9845e1..7fed456 100644 --- a/src/util/virsystemd.h +++ b/src/util/virsystemd.h @@ -46,4 +46,6 @@ int virSystemdTerminateMachine(const char *name, const char *drivername, bool privileged); +void virSystemdNotifyStartup(void); + #endif /* __VIR_SYSTEMD_H__ */ -- 1.9.0
On Fri, Feb 21, 2014 at 01:32:37PM +0100, Michal Privoznik wrote:
Systemd does not forget about the cases, where client service needs to wait for daemon service to initialize and start accepting new clients. Setting a dependency in client is not enough as systemd doesn't know when the daemon has initialized itself and started accepting new clients. However, it offers a mechanism to solve this. The daemon needs to call a special systemd function by which the daemon tells "I'm ready to accept new clients". This is exactly what we need with libvirtd-guests (client) and libvirtd (daemon). So now, with this change, libvirt-guests.service is invoked not any sooner than libvirtd.service calls the systemd notify function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- configure.ac | 2 ++ daemon/libvirtd.service.in | 1 + m4/virt-systemd-daemon.m4 | 34 ++++++++++++++++++++++++++++++++++ src/Makefile.am | 4 ++-- src/libvirt_private.syms | 1 + src/rpc/virnetserver.c | 7 +++++++ src/util/virsystemd.c | 12 ++++++++++++ src/util/virsystemd.h | 2 ++ 8 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 m4/virt-systemd-daemon.m4
diff --git a/configure.ac b/configure.ac index 9e76353..f30ac76 100644 --- a/configure.ac +++ b/configure.ac @@ -239,6 +239,7 @@ LIBVIRT_CHECK_SANLOCK LIBVIRT_CHECK_SASL LIBVIRT_CHECK_SELINUX LIBVIRT_CHECK_SSH2 +LIBVIRT_CHECK_SYSTEMD_DAEMON LIBVIRT_CHECK_UDEV LIBVIRT_CHECK_YAJL
@@ -2773,6 +2774,7 @@ LIBVIRT_RESULT_SANLOCK LIBVIRT_RESULT_SASL LIBVIRT_RESULT_SELINUX LIBVIRT_RESULT_SSH2 +LIBVIRT_RESULT_SYSTEMD_DAEMON LIBVIRT_RESULT_UDEV LIBVIRT_RESULT_YAJL AC_MSG_NOTICE([ libxml: $LIBXML_CFLAGS $LIBXML_LIBS]) diff --git a/daemon/libvirtd.service.in b/daemon/libvirtd.service.in index dc2433a..e1f2a07 100644 --- a/daemon/libvirtd.service.in +++ b/daemon/libvirtd.service.in @@ -13,6 +13,7 @@ Documentation=man:libvirtd(8) Documentation=http://libvirt.org
[Service] +Type=notify EnvironmentFile=-/etc/sysconfig/libvirtd ExecStart=@sbindir@/libvirtd $LIBVIRTD_ARGS ExecReload=/bin/kill -HUP $MAINPID diff --git a/m4/virt-systemd-daemon.m4 b/m4/virt-systemd-daemon.m4 new file mode 100644 index 0000000..8516e41 --- /dev/null +++ b/m4/virt-systemd-daemon.m4 @@ -0,0 +1,34 @@ +dnl The libsystemd-daemon.so library +dnl +dnl Copyright (C) 2012-2013 Red Hat, Inc. +dnl +dnl This library is free software; you can redistribute it and/or +dnl modify it under the terms of the GNU Lesser General Public +dnl License as published by the Free Software Foundation; either +dnl version 2.1 of the License, or (at your option) any later version. +dnl +dnl This library is distributed in the hope that it will be useful, +dnl but WITHOUT ANY WARRANTY; without even the implied warranty of +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +dnl Lesser General Public License for more details. +dnl +dnl You should have received a copy of the GNU Lesser General Public +dnl License along with this library. If not, see +dnl <http://www.gnu.org/licenses/>. +dnl + +AC_DEFUN([LIBVIRT_CHECK_SYSTEMD_DAEMON],[ + LIBVIRT_CHECK_PKG([SYSTEMD_DAEMON], [libsystemd-daemon], [0.27.1]) + + old_CFLAGS="$CFLAGS" + old_LIBS="$LIBS" + CFLAGS="$CFLAGS $SYSTEMD_DAEMON_CFLAGS" + LIBS="$LIBS $SYSTEMD_DAEMON_LIBS" + AC_CHECK_FUNCS([sd_notify]) + CFLAGS="$old_CFLAGS" + LIBS="$old_LIBS" +]) + +AC_DEFUN([LIBVIRT_RESULT_SYSTEMD_DAEMON],[ + LIBVIRT_RESULT_LIB([SYSTEMD_DAEMON]) +]) diff --git a/src/Makefile.am b/src/Makefile.am index 6d21e5d..6ef32ee 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -958,11 +958,11 @@ libvirt_util_la_SOURCES = \ libvirt_util_la_CFLAGS = $(CAPNG_CFLAGS) $(YAJL_CFLAGS) $(LIBNL_CFLAGS) \ $(AM_CFLAGS) $(AUDIT_CFLAGS) $(DEVMAPPER_CFLAGS) \ $(DBUS_CFLAGS) $(LDEXP_LIBM) $(NUMACTL_CFLAGS) \ - -I$(top_srcdir)/src/conf + $(SYSTEMD_DAEMON_CFLAGS) -I$(top_srcdir)/src/conf libvirt_util_la_LIBADD = $(CAPNG_LIBS) $(YAJL_LIBS) $(LIBNL_LIBS) \ $(THREAD_LIBS) $(AUDIT_LIBS) $(DEVMAPPER_LIBS) \ $(LIB_CLOCK_GETTIME) $(DBUS_LIBS) $(MSCOM_LIBS) $(LIBXML_LIBS) \ - $(SECDRIVER_LIBS) $(NUMACTL_LIBS) + $(SECDRIVER_LIBS) $(NUMACTL_LIBS) $(SYSTEMD_DAEMON_LIBS)
noinst_LTLIBRARIES += libvirt_conf.la diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0896287..21c6b66 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1826,6 +1826,7 @@ virSystemdCreateMachine; virSystemdMakeMachineName; virSystemdMakeScopeName; virSystemdMakeSliceName; +virSystemdNotifyStartup; virSystemdTerminateMachine;
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 8907768..be27913 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -38,6 +38,7 @@ #include "virnetservermdns.h" #include "virdbus.h" #include "virstring.h" +#include "virsystemd.h"
#ifndef SA_SIGINFO # define SA_SIGINFO 0 @@ -1085,6 +1086,12 @@ void virNetServerRun(virNetServerPtr srv) goto cleanup; }
+#ifdef WITH_SYSTEMD_DAEMON + /* We are accepting connections now. Notify systemd + * so it can start dependent services. */ + virSystemdNotifyStartup(); +#endif
This method is already a no-op if systemd-daemon isn't present, so the #ifdef is not required.
+ VIR_DEBUG("srv=%p quit=%d", srv, srv->quit); while (!srv->quit) { /* A shutdown timeout is specified, so check diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index 9247c92..8adf209 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -21,6 +21,10 @@
#include <config.h>
+#ifdef WITH_SYSTEMD_DAEMON +# include <systemd/sd-daemon.h> +#endif + #include "virsystemd.h" #include "virdbus.h" #include "virstring.h" @@ -304,3 +308,11 @@ cleanup: VIR_FREE(machinename); return ret; } + +void +virSystemdNotifyStartup(void) +{ +#ifdef WITH_SYSTEMD_DAEMON + sd_notify(0, "READY=1"); +#endif +} diff --git a/src/util/virsystemd.h b/src/util/virsystemd.h index d9845e1..7fed456 100644 --- a/src/util/virsystemd.h +++ b/src/util/virsystemd.h @@ -46,4 +46,6 @@ int virSystemdTerminateMachine(const char *name, const char *drivername, bool privileged);
+void virSystemdNotifyStartup(void);
ACK if you fix the nitpick Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 02/21/2014 05:32 AM, Michal Privoznik wrote:
Systemd does not forget about the cases, where client service needs to wait for daemon service to initialize and start accepting new clients. Setting a dependency in client is not enough as systemd doesn't know when the daemon has initialized itself and started accepting new clients. However, it offers a mechanism to solve this. The daemon needs to call a special systemd function by which the daemon tells "I'm ready to accept new clients". This is exactly what we need with libvirtd-guests (client) and libvirtd (daemon). So now, with this change, libvirt-guests.service is invoked not any sooner than libvirtd.service calls the systemd notify function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- configure.ac | 2 ++ daemon/libvirtd.service.in | 1 + m4/virt-systemd-daemon.m4 | 34 ++++++++++++++++++++++++++++++++++ src/Makefile.am | 4 ++-- src/libvirt_private.syms | 1 + src/rpc/virnetserver.c | 7 +++++++ src/util/virsystemd.c | 12 ++++++++++++ src/util/virsystemd.h | 2 ++ 8 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 m4/virt-systemd-daemon.m4
Adds a new configure option, but doesn't change the spec file. This is bad, because it means that someone doing 'make rpm' now has an indeterminate behavior based on whether they have the library installed. We need a followup patch.
+ +AC_DEFUN([LIBVIRT_CHECK_SYSTEMD_DAEMON],[ + LIBVIRT_CHECK_PKG([SYSTEMD_DAEMON], [libsystemd-daemon], [0.27.1])
Ouch - this results in: $ ./configure --help | grep daemon --with-systemd_daemon with libsystemd-daemon (>= 0.27.1) support which violates configure conventions of spelling all options with '-' instead of '_'. I'll prepare a followup for that. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 25.02.2014 16:18, Eric Blake wrote:
On 02/21/2014 05:32 AM, Michal Privoznik wrote:
Systemd does not forget about the cases, where client service needs to wait for daemon service to initialize and start accepting new clients. Setting a dependency in client is not enough as systemd doesn't know when the daemon has initialized itself and started accepting new clients. However, it offers a mechanism to solve this. The daemon needs to call a special systemd function by which the daemon tells "I'm ready to accept new clients". This is exactly what we need with libvirtd-guests (client) and libvirtd (daemon). So now, with this change, libvirt-guests.service is invoked not any sooner than libvirtd.service calls the systemd notify function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- configure.ac | 2 ++ daemon/libvirtd.service.in | 1 + m4/virt-systemd-daemon.m4 | 34 ++++++++++++++++++++++++++++++++++ src/Makefile.am | 4 ++-- src/libvirt_private.syms | 1 + src/rpc/virnetserver.c | 7 +++++++ src/util/virsystemd.c | 12 ++++++++++++ src/util/virsystemd.h | 2 ++ 8 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 m4/virt-systemd-daemon.m4
Adds a new configure option, but doesn't change the spec file. This is bad, because it means that someone doing 'make rpm' now has an indeterminate behavior based on whether they have the library installed. We need a followup patch.
Not necessarily true. We already have: BuildRequires: systemd-devel >= 185 in our spec file. So 'make rpm' will fail if the systemd-devel package is not installed.
+ +AC_DEFUN([LIBVIRT_CHECK_SYSTEMD_DAEMON],[ + LIBVIRT_CHECK_PKG([SYSTEMD_DAEMON], [libsystemd-daemon], [0.27.1])
Ouch - this results in:
$ ./configure --help | grep daemon --with-systemd_daemon with libsystemd-daemon (>= 0.27.1) support
which violates configure conventions of spelling all options with '-' instead of '_'. I'll prepare a followup for that.
Yes, this one is nasty. Michal
On 02/25/2014 08:33 AM, Michal Privoznik wrote:
Adds a new configure option, but doesn't change the spec file. This is bad, because it means that someone doing 'make rpm' now has an indeterminate behavior based on whether they have the library installed. We need a followup patch.
Not necessarily true. We already have:
BuildRequires: systemd-devel >= 185
Ah, but that only occurs inside %{with_udev}. We need to hoist that outside, as we now have a usage of systemd-devel that is independent of whether we are also using udev. It's also good form to modify the %configure line in the spec file to explicitly state our expectations, rather than implicitly relying on things, even if the build happens to always implicitly choose correctly based on BuildRequires. At any rate, since I pointed it out, I don't mind working on the followup. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (5)
-
Cole Robinson -
Daniel P. Berrange -
Eric Blake -
Ján Tomko -
Michal Privoznik