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

[Daniel P. Berrange]

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