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@xxxxxxxx>
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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list