On Thu, Jul 18, 2013 at 09:15:28PM -0600, Eric Blake wrote:
> On 07/18/2013 09:00 AM, Daniel P. Berrange wrote:
> > From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
> >
> > Instead of returning raw errno values, report full libvirt
> > errors in virCgroupNew* functions.
> >
> > Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> > ---
> > src/libvirt_private.syms | 1 +
> > src/lxc/lxc_cgroup.c | 83 +++-----
> > src/lxc/lxc_container.c | 6 +-
> > src/lxc/lxc_fuse.c | 6 +-
> > src/qemu/qemu_cgroup.c | 87 +++-----
> > src/qemu/qemu_driver.c | 76 ++-----
> > src/util/vircgroup.c | 515 ++++++++++++++++++++++++-----------------------
> > src/util/virerror.c | 46 +++++
> > src/util/virerror.h | 4 +
> > tests/vircgrouptest.c | 44 +++-
> > 10 files changed, 431 insertions(+), 437 deletions(-)
>
> Big, but in the right direction.
[snip]
> Overall, looks like a decent improvement. I pointed out a few things
> worth cleaning up, and the idea of splitting into two patches may have
> merit. It may be faster to post the changes you will squash in than a
> full-blown v2, considering the size of this patch, and the most of the
> conversions made sense.
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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list