[libvirt] [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 | 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) { 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); -- 1.8.0.1
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. 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 :|
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
On Fri, May 10, 2013 at 01:13:20PM -0600, Jim Fehlig wrote:
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
s/propogate/propagate/
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(-)
ACK 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 :|
Daniel P. Berrange wrote:
On Fri, May 10, 2013 at 01:13:20PM -0600, Jim Fehlig wrote:
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
s/propogate/propagate/
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(-)
ACK
Thanks. Pushed after fixing the typo. Regards, Jim
participants (2)
-
Daniel P. Berrange -
Jim Fehlig