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

Re: [libvirt] [PATCH] Fix starting domains when kernel has no cgroups support



Daniel P. Berrange wrote:
> On Fri, May 10, 2013 at 12:40:40PM -0600, Jim Fehlig wrote:
>   
>> Found that I was unable to start existing domains after updating
>> to a kernel with no cgroups support
>>
>>   # zgrep CGROUP /proc/config.gz
>>   # CONFIG_CGROUPS is not set
>>   # virsh start test
>>   error: Failed to start domain test
>>   error: Unable to initialize /machine cgroup: Cannot allocate memory
>>
>> virCgroupPartitionNeedsEscaping() correctly returns errno (ENOENT) when
>> attempting to open /proc/cgroups on such a system, but it was being
>> dropped in virCgroupSetPartitionSuffix().
>>
>> Change virCgroupSetPartitionSuffix() to propogate errors returned by
>> its callees.  Also check for ENOENT in qemuInitCgroup() when determining
>> if cgroups support is available.
>> ---
>>  src/qemu/qemu_cgroup.c |  3 ++-
>>  src/util/vircgroup.c   | 19 +++++++++++++------
>>  2 files changed, 15 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
>> index 9c45b76..40777aa 100644
>> --- a/src/qemu/qemu_cgroup.c
>> +++ b/src/qemu/qemu_cgroup.c
>> @@ -414,7 +414,8 @@ int qemuInitCgroup(virQEMUDriverPtr driver,
>>          if (rc != 0) {
>>              if (rc == -ENXIO ||
>>                  rc == -EPERM ||
>> -                rc == -EACCES) { /* No cgroups mounts == success */
>> +                rc == -EACCES ||
>> +                rc == -ENOENT) { /* No cgroups mounts == success */
>>                  VIR_DEBUG("No cgroups present/configured/accessible, ignoring error");
>>                  goto done;
>>              }
>> diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
>> index 473d2fc..ef619dc 100644
>> --- a/src/util/vircgroup.c
>> +++ b/src/util/vircgroup.c
>> @@ -1167,14 +1167,14 @@ static int virCgroupPartitionEscape(char **path)
>>      return 0;
>>  }
>>  
>> -static char *virCgroupSetPartitionSuffix(const char *path)
>> +static int virCgroupSetPartitionSuffix(const char *path, char **res)
>>     
>
> You're changing the signature, but I don't see you changing any
> callers to adapt.
>   

Opps, that wouldn't even compile :).  I created/tested this patch on a
test machine and somehow dropped a hunk when merging on my dev machine. 
Unfortunately, I also didn't run my pre-patch-submit-tests on my dev
setup, totally missing this.

Here's the updated patch.

Regards,
Jim

>From 63e878b287b98e418b65db46888c1f5d1a01d805 Mon Sep 17 00:00:00 2001
From: Jim Fehlig <jfehlig suse com>
Date: Fri, 10 May 2013 12:05:00 -0600
Subject: [PATCH] Fix starting domains when kernel has no cgroups support

Found that I was unable to start existing domains after updating
to a kernel with no cgroups support

  # zgrep CGROUP /proc/config.gz
  # CONFIG_CGROUPS is not set
  # virsh start test
  error: Failed to start domain test
  error: Unable to initialize /machine cgroup: Cannot allocate memory

virCgroupPartitionNeedsEscaping() correctly returns errno (ENOENT) when
attempting to open /proc/cgroups on such a system, but it was being
dropped in virCgroupSetPartitionSuffix().

Change virCgroupSetPartitionSuffix() to propogate errors returned by
its callees.  Also check for ENOENT in qemuInitCgroup() when determining
if cgroups support is available.
---
 src/qemu/qemu_cgroup.c |  3 ++-
 src/util/vircgroup.c   | 23 +++++++++++++++--------
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 9c45b76..40777aa 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -414,7 +414,8 @@ int qemuInitCgroup(virQEMUDriverPtr driver,
         if (rc != 0) {
             if (rc == -ENXIO ||
                 rc == -EPERM ||
-                rc == -EACCES) { /* No cgroups mounts == success */
+                rc == -EACCES ||
+                rc == -ENOENT) { /* No cgroups mounts == success */
                 VIR_DEBUG("No cgroups present/configured/accessible, ignoring error");
                 goto done;
             }
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 473d2fc..a8470c3 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -1167,14 +1167,14 @@ static int virCgroupPartitionEscape(char **path)
     return 0;
 }
 
-static char *virCgroupSetPartitionSuffix(const char *path)
+static int virCgroupSetPartitionSuffix(const char *path, char **res)
 {
     char **tokens = virStringSplit(path, "/", 0);
     size_t i;
-    char *ret = NULL;
+    int ret = -1;
 
     if (!tokens)
-        return NULL;
+        return ret;
 
     for (i = 0 ; tokens[i] != NULL ; i++) {
         /* Whitelist the 3 top level fixed dirs
@@ -1193,20 +1193,27 @@ static char *virCgroupSetPartitionSuffix(const char *path)
             !strchr(tokens[i], '.')) {
             if (VIR_REALLOC_N(tokens[i],
                               strlen(tokens[i]) + strlen(".partition") + 1) < 0) {
+                ret = -ENOMEM;
                 virReportOOMError();
                 goto cleanup;
             }
             strcat(tokens[i], ".partition");
         }
 
-        if (virCgroupPartitionEscape(&(tokens[i])) < 0) {
-            virReportOOMError();
+        ret = virCgroupPartitionEscape(&(tokens[i]));
+        if (ret < 0) {
+            if (ret == -ENOMEM)
+                virReportOOMError();
             goto cleanup;
         }
     }
 
-    if (!(ret = virStringJoin((const char **)tokens, "/")))
+    if (!(*res = virStringJoin((const char **)tokens, "/"))) {
+        ret = -ENOMEM;
         goto cleanup;
+    }
+
+    ret = 0;
 
 cleanup:
     virStringFreeList(tokens);
@@ -1241,9 +1248,9 @@ int virCgroupNewPartition(const char *path,
 
     /* XXX convert all cgroups APIs to use error report
      * APIs instead of returning errno */
-    if (!(newpath = virCgroupSetPartitionSuffix(path))) {
+    rc = virCgroupSetPartitionSuffix(path, &newpath);
+    if (rc < 0) {
         virResetLastError();
-        rc = -ENOMEM;
         goto cleanup;
     }
 
-- 
1.8.0.1


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