[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Fix launching of VMs on when only logind part of systemd is present



On Wed, Sep 11, 2013 at 1:18 PM, Daniel P. Berrange <berrange redhat com> wrote:
> From: "Daniel P. Berrange" <berrange redhat com>
>
> Debian systems may run the 'systemd-logind' daemon, which causes the
> /sys/fs/cgroup/systemd  mount to be setup, but no other cgroup
> controllers are created. While the LXC driver considers cgroups to
> be mandatory, the QEMU driver is supposed to accept them as optional.
>
> We detect whether they are present by looking in /proc/mounts for
> any mounts of type 'cgroups', but this is not sufficient. We need to
> skip any named mounts (as seen by a name=XXX string in the mount
> options), so that we only detect actual resource controllers.
>
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=721979
>
> Signed-off-by: Daniel P. Berrange <berrange redhat com>
> ---
>  src/util/vircgroup.c  |  5 +++-
>  tests/vircgroupmock.c | 40 ++++++++++++++++++++++++---
>  tests/vircgrouptest.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 116 insertions(+), 4 deletions(-)
>
> diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
> index a260356..626bbc6 100644
> --- a/src/util/vircgroup.c
> +++ b/src/util/vircgroup.c
> @@ -91,7 +91,10 @@ virCgroupAvailable(void)
>          return false;
>
>      while (getmntent_r(mounts, &entry, buf, sizeof(buf)) != NULL) {
> -        if (STREQ(entry.mnt_type, "cgroup")) {
> +        /* We're looking for at least one 'cgroup' fs mount,
> +         * which is *not* a named mount. */
> +        if (STREQ(entry.mnt_type, "cgroup") &&
> +            !strstr(entry.mnt_opts, "name=")) {
>              ret = true;
>              break;
>          }
> diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c
> index d83496c..adc1718 100644
> --- a/tests/vircgroupmock.c
> +++ b/tests/vircgroupmock.c
> @@ -124,6 +124,27 @@ const char *proccgroupsallinone =
>      "devices  6   1  1\n"
>      "blkio    6   1  1\n";
>
> +const char *procmountslogind =
> +    "none /not/really/sys/fs/cgroup tmpfs rw,rootcontext=system_u:object_r:sysfs_t:s0,seclabel,relatime,size=4k,mode=755 0 0\n"
> +    "systemd /not/really/sys/fs/cgroup/systemd cgroup rw,nosuid,nodev,noexec,relatime,name=systemd 0 0\n";
> +
> +const char *procselfcgroupslogind =
> +    "1:name=systemd:/\n";
> +
> +const char *proccgroupslogind =
> +    "#subsys_name    hierarchy       num_cgroups     enabled\n"
> +    "cpuset    0  1  1\n"
> +    "cpu       0  1  1\n"
> +    "cpuacct   0  1  1\n"
> +    "memory    0  1  0\n"
> +    "devices   0  1  1\n"
> +    "freezer   0  1  1\n"
> +    "net_cls   0  1  1\n"
> +    "blkio     0  1  1\n"
> +    "perf_event  0  1  1\n";
> +
> +
> +
>  static int make_file(const char *path,
>                       const char *name,
>                       const char *value)
> @@ -400,18 +421,25 @@ static void init_sysfs(void)
>  FILE *fopen(const char *path, const char *mode)
>  {
>      const char *mock;
> -    bool allinone = false;
> +    bool allinone = false, logind = false;
>      init_syms();
>
>      mock = getenv("VIR_CGROUP_MOCK_MODE");
> -    if (mock && STREQ(mock, "allinone"))
> -        allinone = true;
> +    if (mock) {
> +        if (STREQ(mock, "allinone"))
> +            allinone = true;
> +        else if (STREQ(mock, "logind"))
> +            logind = true;
> +    }
>
>      if (STREQ(path, "/proc/mounts")) {
>          if (STREQ(mode, "r")) {
>              if (allinone)
>                  return fmemopen((void *)procmountsallinone,
>                                  strlen(procmountsallinone), mode);
> +            else if (logind)
> +                return fmemopen((void *)procmountslogind,
> +                                strlen(procmountslogind), mode);
>              else
>                  return fmemopen((void *)procmounts, strlen(procmounts), mode);
>          } else {
> @@ -424,6 +452,9 @@ FILE *fopen(const char *path, const char *mode)
>              if (allinone)
>                  return fmemopen((void *)proccgroupsallinone,
>                                  strlen(proccgroupsallinone), mode);
> +            else if (logind)
> +                return fmemopen((void *)proccgroupslogind,
> +                                strlen(proccgroupslogind), mode);
>              else
>                  return fmemopen((void *)proccgroups, strlen(proccgroups), mode);
>          } else {
> @@ -436,6 +467,9 @@ FILE *fopen(const char *path, const char *mode)
>              if (allinone)
>                  return fmemopen((void *)procselfcgroupsallinone,
>                                  strlen(procselfcgroupsallinone), mode);
> +            else if (logind)
> +                return fmemopen((void *)procselfcgroupslogind,
> +                                strlen(procselfcgroupslogind), mode);
>              else
>                  return fmemopen((void *)procselfcgroups, strlen(procselfcgroups), mode);
>          } else {
> diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c
> index f12587c..570e061 100644
> --- a/tests/vircgrouptest.c
> +++ b/tests/vircgrouptest.c
> @@ -109,6 +109,16 @@ const char *mountsAllInOne[VIR_CGROUP_CONTROLLER_LAST] = {
>      [VIR_CGROUP_CONTROLLER_BLKIO] = "/not/really/sys/fs/cgroup",
>      [VIR_CGROUP_CONTROLLER_SYSTEMD] = NULL,
>  };
> +const char *mountsLogind[VIR_CGROUP_CONTROLLER_LAST] = {
> +    [VIR_CGROUP_CONTROLLER_CPU] = NULL,
> +    [VIR_CGROUP_CONTROLLER_CPUACCT] = NULL,
> +    [VIR_CGROUP_CONTROLLER_CPUSET] = NULL,
> +    [VIR_CGROUP_CONTROLLER_MEMORY] = NULL,
> +    [VIR_CGROUP_CONTROLLER_DEVICES] = NULL,
> +    [VIR_CGROUP_CONTROLLER_FREEZER] = NULL,
> +    [VIR_CGROUP_CONTROLLER_BLKIO] = NULL,
> +    [VIR_CGROUP_CONTROLLER_SYSTEMD] = "/not/really/sys/fs/cgroup/systemd",
> +};
>
>  const char *links[VIR_CGROUP_CONTROLLER_LAST] = {
>      [VIR_CGROUP_CONTROLLER_CPU] = "/not/really/sys/fs/cgroup/cpu",
> @@ -132,6 +142,17 @@ const char *linksAllInOne[VIR_CGROUP_CONTROLLER_LAST] = {
>      [VIR_CGROUP_CONTROLLER_SYSTEMD] = NULL,
>  };
>
> +const char *linksLogind[VIR_CGROUP_CONTROLLER_LAST] = {
> +    [VIR_CGROUP_CONTROLLER_CPU] = NULL,
> +    [VIR_CGROUP_CONTROLLER_CPUACCT] = NULL,
> +    [VIR_CGROUP_CONTROLLER_CPUSET] = NULL,
> +    [VIR_CGROUP_CONTROLLER_MEMORY] = NULL,
> +    [VIR_CGROUP_CONTROLLER_DEVICES] = NULL,
> +    [VIR_CGROUP_CONTROLLER_FREEZER] = NULL,
> +    [VIR_CGROUP_CONTROLLER_BLKIO] = NULL,
> +    [VIR_CGROUP_CONTROLLER_SYSTEMD] = NULL,
> +};
> +
>
>  static int testCgroupNewForSelf(const void *args ATTRIBUTE_UNUSED)
>  {
> @@ -466,6 +487,48 @@ cleanup:
>  }
>
>
> +static int testCgroupNewForSelfLogind(const void *args ATTRIBUTE_UNUSED)
> +{
> +    virCgroupPtr cgroup = NULL;
> +    int ret = -1;
> +    const char *placement[VIR_CGROUP_CONTROLLER_LAST] = {
> +        [VIR_CGROUP_CONTROLLER_CPU] = NULL,
> +        [VIR_CGROUP_CONTROLLER_CPUACCT] = NULL,
> +        [VIR_CGROUP_CONTROLLER_CPUSET] = NULL,
> +        [VIR_CGROUP_CONTROLLER_MEMORY] = NULL,
> +        [VIR_CGROUP_CONTROLLER_DEVICES] = NULL,
> +        [VIR_CGROUP_CONTROLLER_FREEZER] = NULL,
> +        [VIR_CGROUP_CONTROLLER_BLKIO] = NULL,
> +        [VIR_CGROUP_CONTROLLER_SYSTEMD] = "/",
> +    };
> +
> +    if (virCgroupNewSelf(&cgroup) < 0) {
> +        fprintf(stderr, "Cannot create cgroup for self\n");
> +        goto cleanup;
> +    }
> +
> +    ret = validateCgroup(cgroup, "", mountsLogind, linksLogind, placement);
> +
> +cleanup:
> +    virCgroupFree(&cgroup);
> +    return ret;
> +}
> +
> +
> +static int testCgroupAvailable(const void *args)
> +{
> +    bool got = virCgroupAvailable();
> +    bool want = args == (void*)0x1;
> +
> +    if (got != want) {
> +        fprintf(stderr, "Expected cgroup %savailable, but state was wrong\n",

Missing a space between the %s and available.

> +                want ? "" : "not ");
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
>
>  # define FAKESYSFSDIRTEMPLATE abs_builddir "/fakesysfsdir-XXXXXX"
>
> @@ -505,9 +568,21 @@ mymain(void)
>      if (virtTestRun("New cgroup for domain partition escaped", 1, testCgroupNewForPartitionDomainEscaped, NULL) < 0)
>          ret = -1;
>
> +    if (virtTestRun("Cgroup available", 1, testCgroupAvailable, (void*)0x1) < 0)
> +        ret = -1;
> +
>      setenv("VIR_CGROUP_MOCK_MODE", "allinone", 1);
>      if (virtTestRun("New cgroup for self (allinone)", 1, testCgroupNewForSelfAllInOne, NULL) < 0)
>          ret = -1;
> +    if (virtTestRun("Cgroup available", 1, testCgroupAvailable, (void*)0x1) < 0)
> +        ret = -1;
> +    unsetenv("VIR_CGROUP_MOCK_MODE");
> +
> +    setenv("VIR_CGROUP_MOCK_MODE", "logind", 1);
> +    if (virtTestRun("New cgroup for self (logind)", 1, testCgroupNewForSelfLogind, NULL) < 0)
> +        ret = -1;
> +    if (virtTestRun("Cgroup available", 1, testCgroupAvailable, (void*)0x0) < 0)
> +        ret = -1;
>      unsetenv("VIR_CGROUP_MOCK_MODE");
>
>      if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL)
> --
> 1.8.3.1
>
> --
> libvir-list mailing list
> libvir-list redhat com
> https://www.redhat.com/mailman/listinfo/libvir-list

ACK with the fix in the white space.

-- 
Doug Goldstein


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]