[libvirt] [PATCH] Check if systemd is the init before creating machines

If systemd is installed, but not the init system, systemd-machined fails with an unhelpful error message: Launch helper exited with unknown return code 1 Fall back to manual cgroup creation if systemd is installed, but it's not PID 1. [1] https://bugs.freedesktop.org/show_bug.cgi?id=69962 --- (Yes, Gentoo.) src/util/virsystemd.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index 8adf209..0404a63 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -30,6 +30,7 @@ #include "virstring.h" #include "viralloc.h" #include "virutil.h" +#include "virfile.h" #include "virlog.h" #include "virerror.h" @@ -142,6 +143,28 @@ cleanup: return machinename; } +/* + * Returns 0 if systemd is the init, -2 if not + * -1 on fatal error + */ +static int +virSystemdIsInit(void) +{ + char *buf = NULL; + int ret = -2; + + if (virFileReadAll("/proc/1/comm", sizeof("systemd\n "), &buf) < 0) + return -1; + + if (STREQ(buf, "systemd\n")) + ret = 0; + else + VIR_DEBUG("systemd is not the init"); + + VIR_FREE(buf); + return ret; +} + /** * virSystemdCreateMachine: * @name: driver unique name of the machine @@ -173,6 +196,9 @@ int virSystemdCreateMachine(const char *name, if (ret < 0) return ret; + if ((ret = virSystemdIsInit()) < 0) + return ret; + if (!(conn = virDBusGetSystemBus())) return -1; -- 1.8.3.2

On 02/27/2014 09:29 PM, Ján Tomko wrote:
If systemd is installed, but not the init system, systemd-machined fails with an unhelpful error message: Launch helper exited with unknown return code 1
Fall back to manual cgroup creation if systemd is installed, but it's not PID 1.
Hello, This fails virsystemdtest on machines where systemd is not the init. Jan

On Thu, Feb 27, 2014 at 09:29:24PM +0100, Ján Tomko wrote:
If systemd is installed, but not the init system, systemd-machined fails with an unhelpful error message: Launch helper exited with unknown return code 1
Fall back to manual cgroup creation if systemd is installed, but it's not PID 1.
Huh, that bug doesn't mention libvirt or systemd-machined at all ? 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/28/2014 12:05 PM, Daniel P. Berrange wrote:
On Thu, Feb 27, 2014 at 09:29:24PM +0100, Ján Tomko wrote:
If systemd is installed, but not the init system, systemd-machined fails with an unhelpful error message: Launch helper exited with unknown return code 1
Fall back to manual cgroup creation if systemd is installed, but it's not PID 1.
Huh, that bug doesn't mention libvirt or systemd-machined at all ?
Sorry, I must've been writing faster than thinking. Some libvirt users ([1][2]) have reported this error with installed but inactive systemd, where system dbus is running and the machine1 service shows up in ListActivatableNames output, but it's not actually usable. Perhaps https://bugs.gentoo.org/show_bug.cgi?id=493246 would be a better bug for the commit message, but it seems to track multiple issues and it only mentions the error in the comments. Jan [1] https://www.redhat.com/archives/libvirt-users/2014-January/msg00043.html [2] https://www.redhat.com/archives/libvirt-users/2014-February/msg00128.html

On Thu, Feb 27, 2014 at 09:29:24PM +0100, Ján Tomko wrote:
If systemd is installed, but not the init system, systemd-machined fails with an unhelpful error message: Launch helper exited with unknown return code 1
Fall back to manual cgroup creation if systemd is installed, but it's not PID 1.
[1] https://bugs.freedesktop.org/show_bug.cgi?id=69962 ---
(Yes, Gentoo.)
src/util/virsystemd.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)
diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index 8adf209..0404a63 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -30,6 +30,7 @@ #include "virstring.h" #include "viralloc.h" #include "virutil.h" +#include "virfile.h" #include "virlog.h" #include "virerror.h"
@@ -142,6 +143,28 @@ cleanup: return machinename; }
+/* + * Returns 0 if systemd is the init, -2 if not + * -1 on fatal error + */ +static int +virSystemdIsInit(void) +{ + char *buf = NULL; + int ret = -2; + + if (virFileReadAll("/proc/1/comm", sizeof("systemd\n "), &buf) < 0) + return -1; + + if (STREQ(buf, "systemd\n")) + ret = 0; + else + VIR_DEBUG("systemd is not the init"); + + VIR_FREE(buf); + return ret; +} + /** * virSystemdCreateMachine: * @name: driver unique name of the machine @@ -173,6 +196,9 @@ int virSystemdCreateMachine(const char *name, if (ret < 0) return ret;
+ if ((ret = virSystemdIsInit()) < 0) + return ret; +
We already do a call ret = virDBusIsServiceEnabled("org.freedesktop.machine1"); I'd suggest that we perhaps also call ret = virDBusIsServiceEnabled("org.freedesktop.systemd1"); instead of looking in /proc/1/comm 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/28/2014 06:36 PM, Daniel P. Berrange wrote:
On Thu, Feb 27, 2014 at 09:29:24PM +0100, Ján Tomko wrote:
If systemd is installed, but not the init system, systemd-machined fails with an unhelpful error message: Launch helper exited with unknown return code 1
Fall back to manual cgroup creation if systemd is installed, but it's not PID 1.
[1] https://bugs.freedesktop.org/show_bug.cgi?id=69962 ---
(Yes, Gentoo.)
src/util/virsystemd.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)
diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index 8adf209..0404a63 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -30,6 +30,7 @@ #include "virstring.h" #include "viralloc.h" #include "virutil.h" +#include "virfile.h" #include "virlog.h" #include "virerror.h"
@@ -142,6 +143,28 @@ cleanup: return machinename; }
+/* + * Returns 0 if systemd is the init, -2 if not + * -1 on fatal error + */ +static int +virSystemdIsInit(void) +{ + char *buf = NULL; + int ret = -2; + + if (virFileReadAll("/proc/1/comm", sizeof("systemd\n "), &buf) < 0) + return -1; + + if (STREQ(buf, "systemd\n")) + ret = 0; + else + VIR_DEBUG("systemd is not the init"); + + VIR_FREE(buf); + return ret; +} + /** * virSystemdCreateMachine: * @name: driver unique name of the machine @@ -173,6 +196,9 @@ int virSystemdCreateMachine(const char *name, if (ret < 0) return ret;
+ if ((ret = virSystemdIsInit()) < 0) + return ret; +
We already do a call
ret = virDBusIsServiceEnabled("org.freedesktop.machine1");
I'd suggest that we perhaps also call
ret = virDBusIsServiceEnabled("org.freedesktop.systemd1");
instead of looking in /proc/1/comm
systemd1 also shows up in the list of activatable names even if systemd is not running. Jan

On Mon, Mar 03, 2014 at 10:35:36AM +0100, Ján Tomko wrote:
On 02/28/2014 06:36 PM, Daniel P. Berrange wrote:
On Thu, Feb 27, 2014 at 09:29:24PM +0100, Ján Tomko wrote:
If systemd is installed, but not the init system, systemd-machined fails with an unhelpful error message: Launch helper exited with unknown return code 1
Fall back to manual cgroup creation if systemd is installed, but it's not PID 1.
[1] https://bugs.freedesktop.org/show_bug.cgi?id=69962 ---
(Yes, Gentoo.)
src/util/virsystemd.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)
diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index 8adf209..0404a63 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -30,6 +30,7 @@ #include "virstring.h" #include "viralloc.h" #include "virutil.h" +#include "virfile.h" #include "virlog.h" #include "virerror.h"
@@ -142,6 +143,28 @@ cleanup: return machinename; }
+/* + * Returns 0 if systemd is the init, -2 if not + * -1 on fatal error + */ +static int +virSystemdIsInit(void) +{ + char *buf = NULL; + int ret = -2; + + if (virFileReadAll("/proc/1/comm", sizeof("systemd\n "), &buf) < 0) + return -1; + + if (STREQ(buf, "systemd\n")) + ret = 0; + else + VIR_DEBUG("systemd is not the init"); + + VIR_FREE(buf); + return ret; +} + /** * virSystemdCreateMachine: * @name: driver unique name of the machine @@ -173,6 +196,9 @@ int virSystemdCreateMachine(const char *name, if (ret < 0) return ret;
+ if ((ret = virSystemdIsInit()) < 0) + return ret; +
We already do a call
ret = virDBusIsServiceEnabled("org.freedesktop.machine1");
I'd suggest that we perhaps also call
ret = virDBusIsServiceEnabled("org.freedesktop.systemd1");
instead of looking in /proc/1/comm
systemd1 also shows up in the list of activatable names even if systemd is not running.
Opps, I thought IsServiceEnabled() listed only running instances. We should add a second method, which calls "ListNames" instead of "ListActivatableNames", so we can see if systemd is actually running. 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 :|
participants (2)
-
Daniel P. Berrange
-
Ján Tomko