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(a)redhat.com&gt; --- 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 02/21/2014 07:32 AM, Michal Privoznik wrote: Ahh, so this does indeed work all along and I just messed up the gvariant :/ ACK - Cole

On Fri, Feb 21, 2014 at 02:25:24PM +0100, Michal Privoznik wrote: Hmm, i guess that would work in fact, provided we ensured that the main libvirtd.service was still always enabled by default too. 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. 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 :|

On 02/21/2014 05:32 AM, Michal Privoznik wrote: s/libirtd/libvirtd/ in subject fractional second sleep is a GNU sleep extension; it is not portable to all platforms. 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

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(a)redhat.com&gt; --- 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