Re: [libvirt] [PATCH 1/3] Report full errors from virCgroupNew*

On Thu, Jul 18, 2013 at 09:15:28PM -0600, Eric Blake wrote: [snip] Incremental diff is: diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a2b61c7..45e7f63 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1156,6 +1156,7 @@ virCgroupAddTaskController; virCgroupAllowDevice; virCgroupAllowDeviceMajor; virCgroupAllowDevicePath; +virCgroupAvailable; virCgroupControllerTypeFromString; virCgroupControllerTypeToString; virCgroupDenyAllDevices; @@ -1188,6 +1189,7 @@ virCgroupNewDomainDriver; virCgroupNewDomainPartition; virCgroupNewDriver; virCgroupNewEmulator; +virCgroupNewIgnoreError; virCgroupNewPartition; virCgroupNewSelf; virCgroupNewVcpu; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index d20dcc2..2df80bc 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -731,6 +731,9 @@ qemuInitCgroup(virQEMUDriverPtr driver, if (!cfg->privileged) goto done; + if (!virCgroupAvailable()) + goto done; + virCgroupFree(&priv->cgroup); if (!vm->def->resource && startup) { @@ -761,15 +764,8 @@ qemuInitCgroup(virQEMUDriverPtr driver, STREQ(vm->def->resource->partition, "/machine"), cfg->cgroupControllers, &parent) < 0) { - virErrorPtr err = virGetLastError(); - if (err && err->code == VIR_ERR_SYSTEM_ERROR && - (err->int1 == ENXIO || - err->int1 == EPERM || - err->int1 == EACCES)) { /* No cgroups mounts == success */ - VIR_DEBUG("No cgroups present/configured/accessible, ignoring error"); - virResetLastError(); + if (virCgroupNewIgnoreError()) goto done; - } goto cleanup; } @@ -785,15 +781,8 @@ qemuInitCgroup(virQEMUDriverPtr driver, true, cfg->cgroupControllers, &parent) < 0) { - virErrorPtr err = virGetLastError(); - if (err && err->code == VIR_ERR_SYSTEM_ERROR && - (err->int1 == ENXIO || - err->int1 == EPERM || - err->int1 == EACCES)) { /* No cgroups mounts == success */ - VIR_DEBUG("No cgroups present/configured/accessible, ignoring error"); - virResetLastError(); + if (virCgroupNewIgnoreError()) goto done; - } goto cleanup; } diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 12ace2e..3569046 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -67,6 +67,30 @@ typedef enum { */ } virCgroupFlags; +bool virCgroupAvailable(void) +{ + FILE *mounts = NULL; + struct mntent entry; + bool ret = false; + char buf[CGROUP_MAX_VAL]; + + if (!virFileExists("/proc/cgroups")) + return false; + + if (!(mounts = fopen("/proc/mounts", "r"))) + return false; + + while (getmntent_r(mounts, &entry, buf, sizeof(buf)) != NULL) { + if (STREQ(entry.mnt_type, "cgroup")) { + ret = true; + break; + } + } + + VIR_FORCE_FCLOSE(mounts); + return ret; +} + /** * virCgroupFree: * @@ -1585,6 +1609,19 @@ int virCgroupNewEmulator(virCgroupPtr domain ATTRIBUTE_UNUSED, } #endif + +bool virCgroupNewIgnoreError(void) +{ + if (virLastErrorIsSystemErrno(ENXIO) || + virLastErrorIsSystemErrno(EPERM) || + virLastErrorIsSystemErrno(EACCES)) { + virResetLastError(); + VIR_DEBUG("No cgroups present/configured/accessible, ignoring error"); + return true; + } + return false; +} + /** * virCgroupSetBlkioWeight: * @@ -2464,7 +2501,8 @@ static int virCgroupKillRecursiveInternal(virCgroupPtr group, int signum, virHas VIR_DEBUG("Process subdir %s", ent->d_name); if (virCgroupNew(ent->d_name, group, -1, &subgroup) < 0) { - rc = -EIO; + virErrorSetErrnoFromLastError(); + rc = -errno; goto cleanup; } diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index b030e4a..0ec8b67 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -46,6 +46,8 @@ enum { VIR_ENUM_DECL(virCgroupController); +bool virCgroupAvailable(void); + int virCgroupNewPartition(const char *path, bool create, int controllers, @@ -84,6 +86,8 @@ int virCgroupNewEmulator(virCgroupPtr domain, virCgroupPtr *group) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3); +bool virCgroupNewIgnoreError(void); + int virCgroupPathOfController(virCgroupPtr group, int controller, const char *key, diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index 21529a0..a996898 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -140,8 +140,8 @@ cleanup: do { \ if (!virLastErrorIsSystemErrno(en)) { \ virErrorPtr err = virGetLastError(); \ - fprintf(stderr, "Did not get " #en " error code: %d\n", \ - err ? err->code : 0); \ + fprintf(stderr, "Did not get " #en " error code: %d:%d\n", \ + err ? err->code : 0, err ? err->int1 : 0); \ goto cleanup; \ } } while (0) @@ -193,7 +193,6 @@ static int testCgroupNewForDriver(const void *args ATTRIBUTE_UNUSED) fprintf(stderr, "Should not have created LXC cgroup: %d\n", -rv); goto cleanup; } - ENSURE_ERRNO(ENXIO); /* Asking for small combination since devices is not mounted */ -- |: 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 :|