From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Instead of returning raw errno values, report full libvirt errors in virCgroupNew* functions. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- 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(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 542424d..a31512f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1302,6 +1302,7 @@ ebtablesRemoveForwardAllowIn; # util/virerror.h virDispatchError; virErrorInitialize; +virLastErrorIsSystemErrno; virRaiseErrorFull; virReportErrorHelper; virReportOOMErrorFull; diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index fd27601..3cec8e2 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -306,33 +306,29 @@ cleanup: int virLXCCgroupGetMeminfo(virLXCMeminfoPtr meminfo) { - int ret; + int ret = -1, rc; virCgroupPtr cgroup; - ret = virCgroupNewSelf(&cgroup); - if (ret < 0) { - virReportSystemError(-ret, "%s", - _("Unable to get cgroup for container")); - return ret; - } + if (virCgroupNewSelf(&cgroup) < 0) + return -1; - ret = virLXCCgroupGetMemStat(cgroup, meminfo); - if (ret < 0) { - virReportSystemError(-ret, "%s", + rc = virLXCCgroupGetMemStat(cgroup, meminfo); + if (rc < 0) { + virReportSystemError(-rc, "%s", _("Unable to get memory cgroup stat info")); goto cleanup; } - ret = virLXCCgroupGetMemTotal(cgroup, meminfo); - if (ret < 0) { - virReportSystemError(-ret, "%s", + rc = virLXCCgroupGetMemTotal(cgroup, meminfo); + if (rc < 0) { + virReportSystemError(-rc, "%s", _("Unable to get memory cgroup total")); goto cleanup; } - ret = virLXCCgroupGetMemUsage(cgroup, meminfo); - if (ret < 0) { - virReportSystemError(-ret, "%s", + rc = virLXCCgroupGetMemUsage(cgroup, meminfo); + if (rc < 0) { + virReportSystemError(-rc, "%s", _("Unable to get memory cgroup stat usage")); goto cleanup; } @@ -541,7 +537,6 @@ cleanup: virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def, bool startup) { - int rc; virCgroupPtr parent = NULL; virCgroupPtr cgroup = NULL; @@ -569,50 +564,30 @@ virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def, bool startup) } /* We only auto-create the default partition. In other * cases we expec the sysadmin/app to have done so */ - rc = virCgroupNewPartition(def->resource->partition, - STREQ(def->resource->partition, "/machine"), - -1, - &parent); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to initialize %s cgroup"), - def->resource->partition); + if (virCgroupNewPartition(def->resource->partition, + STREQ(def->resource->partition, "/machine"), + -1, + &parent) < 0) goto cleanup; - } - rc = virCgroupNewDomainPartition(parent, - "lxc", - def->name, - true, - &cgroup); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to create cgroup for %s"), - def->name); + if (virCgroupNewDomainPartition(parent, + "lxc", + def->name, + true, + &cgroup) < 0) goto cleanup; - } } else { - rc = virCgroupNewDriver("lxc", - true, - -1, - &parent); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to create cgroup for %s"), - def->name); + if (virCgroupNewDriver("lxc", + true, + -1, + &parent) < 0) goto cleanup; - } - rc = virCgroupNewDomainDriver(parent, - def->name, - true, - &cgroup); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to create cgroup for %s"), - def->name); + if (virCgroupNewDomainDriver(parent, + def->name, + true, + &cgroup) < 0) goto cleanup; - } } cleanup: diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 004a6ff..c033c7a 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1463,7 +1463,6 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, virSecurityManagerPtr securityDriver) { virCgroupPtr cgroup = NULL; - int rc; int ret = -1; char *sec_mount_options; char *stateDir = NULL; @@ -1475,11 +1474,8 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, /* Before pivoting we need to identify any * cgroups controllers that are mounted */ - if ((rc = virCgroupNewSelf(&cgroup)) != 0) { - virReportSystemError(-rc, "%s", - _("Cannot identify cgroup placement")); + if (virCgroupNewSelf(&cgroup) < 0) goto cleanup; - } if (virFileResolveAllLinks(LXC_STATE_DIR, &stateDir) < 0) goto cleanup; diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c index ea4ab7a..e672b0f 100644 --- a/src/lxc/lxc_fuse.c +++ b/src/lxc/lxc_fuse.c @@ -139,8 +139,10 @@ static int lxcProcReadMeminfo(char *hostpath, virDomainDefPtr def, virBuffer buffer = VIR_BUFFER_INITIALIZER; virBufferPtr new_meminfo = &buffer; - if ((res = virLXCCgroupGetMeminfo(&meminfo)) < 0) - return res; + if (virLXCCgroupGetMeminfo(&meminfo) < 0) { + virErrorSetErrnoFromLastError(); + return -errno; + } fd = fopen(hostpath, "r"); if (fd == NULL) { diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index fbe7e6f..f40131a 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -727,64 +727,52 @@ int qemuInitCgroup(virQEMUDriverPtr driver, } /* We only auto-create the default partition. In other * cases we expec the sysadmin/app to have done so */ - rc = virCgroupNewPartition(vm->def->resource->partition, - STREQ(vm->def->resource->partition, "/machine"), - cfg->cgroupControllers, - &parent); - if (rc != 0) { - if (rc == -ENXIO || - rc == -EPERM || - rc == -EACCES) { /* No cgroups mounts == success */ + if (virCgroupNewPartition(vm->def->resource->partition, + 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(); goto done; } - virReportSystemError(-rc, - _("Unable to initialize %s cgroup"), - vm->def->resource->partition); goto cleanup; } - rc = virCgroupNewDomainPartition(parent, - "qemu", - vm->def->name, - true, - &priv->cgroup); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to create cgroup for %s"), - vm->def->name); + if (virCgroupNewDomainPartition(parent, + "qemu", + vm->def->name, + true, + &priv->cgroup) < 0) goto cleanup; - } } else { - rc = virCgroupNewDriver("qemu", - true, - cfg->cgroupControllers, - &parent); - if (rc != 0) { - if (rc == -ENXIO || - rc == -EPERM || - rc == -EACCES) { /* No cgroups mounts == success */ + if (virCgroupNewDriver("qemu", + 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(); goto done; } - virReportSystemError(-rc, - _("Unable to create cgroup for %s"), - vm->def->name); goto cleanup; } - rc = virCgroupNewDomainDriver(parent, - vm->def->name, - true, - &priv->cgroup); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to create cgroup for %s"), - vm->def->name); + if (virCgroupNewDomainDriver(parent, + vm->def->name, + true, + &priv->cgroup) < 0) goto cleanup; - } } done: @@ -953,14 +941,8 @@ int qemuSetupCgroupForVcpu(virDomainObjPtr vm) } for (i = 0; i < priv->nvcpupids; i++) { - rc = virCgroupNewVcpu(priv->cgroup, i, true, &cgroup_vcpu); - if (rc < 0) { - virReportSystemError(-rc, - _("Unable to create vcpu cgroup for %s(vcpu:" - " %zu)"), - vm->def->name, i); + if (virCgroupNewVcpu(priv->cgroup, i, true, &cgroup_vcpu) < 0) goto cleanup; - } /* move the thread for vcpu to sub dir */ rc = virCgroupAddTask(cgroup_vcpu, priv->vcpupids[i]); @@ -1019,7 +1001,7 @@ int qemuSetupCgroupForEmulator(virQEMUDriverPtr driver, qemuDomainObjPrivatePtr priv = vm->privateData; unsigned long long period = vm->def->cputune.emulator_period; long long quota = vm->def->cputune.emulator_quota; - int rc; + int rc = -1; if ((period || quota) && !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { @@ -1031,13 +1013,8 @@ int qemuSetupCgroupForEmulator(virQEMUDriverPtr driver, if (priv->cgroup == NULL) return 0; /* Not supported, so claim success */ - rc = virCgroupNewEmulator(priv->cgroup, true, &cgroup_emulator); - if (rc < 0) { - virReportSystemError(-rc, - _("Unable to create emulator cgroup for %s"), - vm->def->name); + if (virCgroupNewEmulator(priv->cgroup, true, &cgroup_emulator) < 0) goto cleanup; - } rc = virCgroupMoveTask(priv->cgroup, cgroup_emulator); if (rc < 0) { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b7b066d..5c84ce4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3940,14 +3940,8 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, if (priv->cgroup) { int rv = -1; /* Create cgroup for the onlined vcpu */ - rv = virCgroupNewVcpu(priv->cgroup, i, true, &cgroup_vcpu); - if (rv < 0) { - virReportSystemError(-rv, - _("Unable to create vcpu cgroup for %s(vcpu:" - " %zu)"), - vm->def->name, i); + if (virCgroupNewVcpu(priv->cgroup, i, true, &cgroup_vcpu) < 0) goto cleanup; - } /* Add vcpu thread to the cgroup */ rv = virCgroupAddTask(cgroup_vcpu, cpupids[i]); @@ -4008,16 +4002,8 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, virDomainVcpuPinDefPtr vcpupin = NULL; if (priv->cgroup) { - int rv = -1; - - rv = virCgroupNewVcpu(priv->cgroup, i, false, &cgroup_vcpu); - if (rv < 0) { - virReportSystemError(-rv, - _("Unable to access vcpu cgroup for %s(vcpu:" - " %zu)"), - vm->def->name, i); + if (virCgroupNewVcpu(priv->cgroup, i, false, &cgroup_vcpu) < 0) goto cleanup; - } /* Remove cgroup for the offlined vcpu */ virCgroupRemove(cgroup_vcpu); @@ -4358,8 +4344,9 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, /* Configure the corresponding cpuset cgroup before set affinity. */ if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) { - if (virCgroupNewVcpu(priv->cgroup, vcpu, false, &cgroup_vcpu) == 0 && - qemuSetupCgroupVcpuPin(cgroup_vcpu, newVcpuPin, newVcpuPinNum, vcpu) < 0) { + if (virCgroupNewVcpu(priv->cgroup, vcpu, false, &cgroup_vcpu) < 0) + goto cleanup; + if (qemuSetupCgroupVcpuPin(cgroup_vcpu, newVcpuPin, newVcpuPinNum, vcpu) < 0) { virReportError(VIR_ERR_OPERATION_INVALID, _("failed to set cpuset.cpus in cgroup" " for vcpu %d"), vcpu); @@ -4620,16 +4607,15 @@ qemuDomainPinEmulator(virDomainPtr dom, VIR_CGROUP_CONTROLLER_CPUSET)) { /* * Configure the corresponding cpuset cgroup. - * If no cgroup for domain or hypervisor exists, do nothing. */ - if (virCgroupNewEmulator(priv->cgroup, false, &cgroup_emulator) == 0) { - if (qemuSetupCgroupEmulatorPin(cgroup_emulator, - newVcpuPin[0]->cpumask) < 0) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("failed to set cpuset.cpus in cgroup" - " for emulator threads")); - goto cleanup; - } + if (virCgroupNewEmulator(priv->cgroup, false, &cgroup_emulator) < 0) + goto cleanup; + if (qemuSetupCgroupEmulatorPin(cgroup_emulator, + newVcpuPin[0]->cpumask) < 0) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("failed to set cpuset.cpus in cgroup" + " for emulator threads")); + goto cleanup; } } else { if (virProcessSetAffinity(pid, pcpumap) < 0) { @@ -8590,7 +8576,6 @@ qemuSetVcpusBWLive(virDomainObjPtr vm, virCgroupPtr cgroup, size_t i; qemuDomainObjPrivatePtr priv = vm->privateData; virCgroupPtr cgroup_vcpu = NULL; - int rc; if (period == 0 && quota == 0) return 0; @@ -8601,14 +8586,8 @@ qemuSetVcpusBWLive(virDomainObjPtr vm, virCgroupPtr cgroup, */ if (priv->nvcpupids != 0 && priv->vcpupids[0] != vm->pid) { for (i = 0; i < priv->nvcpupids; i++) { - rc = virCgroupNewVcpu(cgroup, i, false, &cgroup_vcpu); - if (rc < 0) { - virReportSystemError(-rc, - _("Unable to find vcpu cgroup for %s(vcpu:" - " %zu)"), - vm->def->name, i); + if (virCgroupNewVcpu(cgroup, i, false, &cgroup_vcpu) < 0) goto cleanup; - } if (qemuSetupCgroupVcpuBW(cgroup_vcpu, period, quota) < 0) goto cleanup; @@ -8630,7 +8609,6 @@ qemuSetEmulatorBandwidthLive(virDomainObjPtr vm, virCgroupPtr cgroup, { qemuDomainObjPrivatePtr priv = vm->privateData; virCgroupPtr cgroup_emulator = NULL; - int rc; if (period == 0 && quota == 0) return 0; @@ -8639,13 +8617,8 @@ qemuSetEmulatorBandwidthLive(virDomainObjPtr vm, virCgroupPtr cgroup, return 0; } - rc = virCgroupNewEmulator(cgroup, false, &cgroup_emulator); - if (rc < 0) { - virReportSystemError(-rc, - _("Unable to find emulator cgroup for %s"), - vm->def->name); + if (virCgroupNewEmulator(cgroup, false, &cgroup_emulator) < 0) goto cleanup; - } if (qemuSetupCgroupVcpuBW(cgroup_emulator, period, quota) < 0) goto cleanup; @@ -8891,13 +8864,8 @@ qemuGetVcpusBWLive(virDomainObjPtr vm, } /* get period and quota for vcpu0 */ - rc = virCgroupNewVcpu(priv->cgroup, 0, false, &cgroup_vcpu); - if (!cgroup_vcpu) { - virReportSystemError(-rc, - _("Unable to find vcpu cgroup for %s(vcpu: 0)"), - vm->def->name); + if (virCgroupNewVcpu(priv->cgroup, 0, false, &cgroup_vcpu) < 0) goto cleanup; - } rc = qemuGetVcpuBWLive(cgroup_vcpu, period, quota); if (rc < 0) @@ -8929,13 +8897,8 @@ qemuGetEmulatorBandwidthLive(virDomainObjPtr vm, virCgroupPtr cgroup, } /* get period and quota for emulator */ - rc = virCgroupNewEmulator(cgroup, false, &cgroup_emulator); - if (!cgroup_emulator) { - virReportSystemError(-rc, - _("Unable to find emulator cgroup for %s"), - vm->def->name); + if (virCgroupNewEmulator(cgroup, false, &cgroup_emulator) < 0) goto cleanup; - } rc = qemuGetVcpuBWLive(cgroup_emulator, period, quota); if (rc < 0) @@ -15531,11 +15494,8 @@ getSumVcpuPercpuStats(virDomainObjPtr vm, unsigned long long tmp; size_t j; - if (virCgroupNewVcpu(priv->cgroup, i, false, &group_vcpu) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("error accessing cgroup cpuacct for vcpu")); + if (virCgroupNewVcpu(priv->cgroup, i, false, &group_vcpu) < 0) goto cleanup; - } if (virCgroupGetCpuacctPercpuUsage(group_vcpu, &buf) < 0) goto cleanup; diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 9dfe98d..12ace2e 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -116,13 +116,13 @@ static int virCgroupCopyMounts(virCgroupPtr group, if (!parent->controllers[i].mountPoint) continue; - if (VIR_STRDUP_QUIET(group->controllers[i].mountPoint, - parent->controllers[i].mountPoint) < 0) - return -ENOMEM; + if (VIR_STRDUP(group->controllers[i].mountPoint, + parent->controllers[i].mountPoint) < 0) + return -1; - if (VIR_STRDUP_QUIET(group->controllers[i].linkPoint, - parent->controllers[i].linkPoint) < 0) - return -ENOMEM; + if (VIR_STRDUP(group->controllers[i].linkPoint, + parent->controllers[i].linkPoint) < 0) + return -1; } return 0; } @@ -140,8 +140,9 @@ static int virCgroupDetectMounts(virCgroupPtr group) mounts = fopen("/proc/mounts", "r"); if (mounts == NULL) { - VIR_ERROR(_("Unable to open /proc/mounts")); - return -ENOENT; + virReportSystemError(errno, "%s", + _("Unable to open /proc/mounts")); + return -1; } while (getmntent_r(mounts, &entry, buf, sizeof(buf)) != NULL) { @@ -171,13 +172,15 @@ static int virCgroupDetectMounts(virCgroupPtr group) struct stat sb; char *tmp2; - if (VIR_STRDUP_QUIET(group->controllers[i].mountPoint, - entry.mnt_dir) < 0) + if (VIR_STRDUP(group->controllers[i].mountPoint, + entry.mnt_dir) < 0) goto error; tmp2 = strrchr(entry.mnt_dir, '/'); if (!tmp2) { - errno = EINVAL; + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Missing '/' separator in cgroup mount '%s'"), + entry.mnt_dir); goto error; } *tmp2 = '\0'; @@ -195,6 +198,8 @@ static int virCgroupDetectMounts(virCgroupPtr group) typestr, entry.mnt_dir, linksrc); VIR_FREE(linksrc); } else { + virReportSystemError(errno, + _("Cannot stat %s"), linksrc); goto error; } } else { @@ -218,7 +223,7 @@ static int virCgroupDetectMounts(virCgroupPtr group) error: VIR_FORCE_FCLOSE(mounts); - return -errno; + return -1; } @@ -232,8 +237,8 @@ static int virCgroupCopyPlacement(virCgroupPtr group, continue; if (path[0] == '/') { - if (VIR_STRDUP_QUIET(group->controllers[i].placement, path) < 0) - return -ENOMEM; + if (VIR_STRDUP(group->controllers[i].placement, path) < 0) + return -1; } else { /* * parent=="/" + path="" => "/" @@ -246,7 +251,7 @@ static int virCgroupCopyPlacement(virCgroupPtr group, (STREQ(parent->controllers[i].placement, "/") || STREQ(path, "") ? "" : "/"), path) < 0) - return -ENOMEM; + return -1; } } @@ -282,11 +287,13 @@ static int virCgroupDetectPlacement(virCgroupPtr group, size_t i; FILE *mapping = NULL; char line[1024]; + int ret = -1; mapping = fopen("/proc/self/cgroup", "r"); if (mapping == NULL) { - VIR_ERROR(_("Unable to open /proc/self/cgroup")); - return -ENOENT; + virReportSystemError(errno, "%s", + _("Unable to open /proc/self/cgroup")); + return -1; } while (fgets(line, sizeof(line), mapping) != NULL) { @@ -329,7 +336,7 @@ static int virCgroupDetectPlacement(virCgroupPtr group, (STREQ(selfpath, "/") || STREQ(path, "") ? "" : "/"), path) < 0) - goto no_memory; + goto cleanup; } tmp = next; @@ -337,14 +344,12 @@ static int virCgroupDetectPlacement(virCgroupPtr group, } } - VIR_FORCE_FCLOSE(mapping); - - return 0; + ret = 0; -no_memory: +cleanup: VIR_FORCE_FCLOSE(mapping); - return -ENOMEM; + return ret; } static int virCgroupDetect(virCgroupPtr group, @@ -352,19 +357,17 @@ static int virCgroupDetect(virCgroupPtr group, const char *path, virCgroupPtr parent) { - int rc; size_t i; size_t j; VIR_DEBUG("group=%p controllers=%d path=%s parent=%p", group, controllers, path, parent); - if (parent) - rc = virCgroupCopyMounts(group, parent); - else - rc = virCgroupDetectMounts(group); - if (rc < 0) { - VIR_ERROR(_("Failed to detect mounts for %s"), group->path); - return rc; + if (parent) { + if (virCgroupCopyMounts(group, parent) < 0) + return -1; + } else { + if (virCgroupDetectMounts(group) < 0) + return -1; } if (controllers >= 0) { @@ -392,10 +395,11 @@ static int virCgroupDetect(virCgroupPtr group, if (STREQ_NULLABLE(group->controllers[i].mountPoint, group->controllers[j].mountPoint)) { - VIR_DEBUG("Controller '%s' is not wanted, but '%s' is co-mounted", - virCgroupControllerTypeToString(i), - virCgroupControllerTypeToString(j)); - return -EINVAL; + virReportSystemError(EINVAL, + "Controller '%s' is not wanted, but '%s' is co-mounted", + virCgroupControllerTypeToString(i), + virCgroupControllerTypeToString(j)); + return -1; } } VIR_FREE(group->controllers[i].mountPoint); @@ -416,39 +420,39 @@ static int virCgroupDetect(virCgroupPtr group, /* Check that at least 1 controller is available */ if (!controllers) { - VIR_DEBUG("No controllers set"); - return -ENXIO; + virReportSystemError(ENXIO, "%s", + _("At least one cgroup controller is required")); + return -1; } - if (parent || path[0] == '/') - rc = virCgroupCopyPlacement(group, path, parent); - else - rc = virCgroupDetectPlacement(group, path); - - if (rc == 0) { - /* Check that for every mounted controller, we found our placement */ - for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { - if (!group->controllers[i].mountPoint) - continue; + if (parent || path[0] == '/') { + if (virCgroupCopyPlacement(group, path, parent) < 0) + return -1; + } else { + if (virCgroupDetectPlacement(group, path) < 0) + return -1; + } - if (!group->controllers[i].placement) { - VIR_ERROR(_("Could not find placement for controller %s at %s"), - virCgroupControllerTypeToString(i), - group->controllers[i].placement); - rc = -ENOENT; - break; - } + /* Check that for every mounted controller, we found our placement */ + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + if (!group->controllers[i].mountPoint) + continue; - VIR_DEBUG("Detected mount/mapping %zu:%s at %s in %s", i, - virCgroupControllerTypeToString(i), - group->controllers[i].mountPoint, - group->controllers[i].placement); + if (!group->controllers[i].placement) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not find placement for controller %s at %s"), + virCgroupControllerTypeToString(i), + group->controllers[i].placement); + return -1; } - } else { - VIR_ERROR(_("Failed to detect mapping for %s"), group->path); + + VIR_DEBUG("Detected mount/mapping %zu:%s at %s in %s", i, + virCgroupControllerTypeToString(i), + group->controllers[i].mountPoint, + group->controllers[i].placement); } - return rc; + return 0; } #endif @@ -646,8 +650,9 @@ static int virCgroupCpuSetInherit(virCgroupPtr parent, virCgroupPtr group) inherit_values[i], &value); if (rc != 0) { - VIR_ERROR(_("Failed to get %s %d"), inherit_values[i], rc); - break; + virReportSystemError(-rc, + _("Failed to get '%s'"), inherit_values[i]); + return -1; } VIR_DEBUG("Inherit %s = %s", inherit_values[i], value); @@ -659,12 +664,13 @@ static int virCgroupCpuSetInherit(virCgroupPtr parent, virCgroupPtr group) VIR_FREE(value); if (rc != 0) { - VIR_ERROR(_("Failed to set %s %d"), inherit_values[i], rc); - break; + virReportSystemError(-rc, + _("Failed to set '%s'"), inherit_values[i]); + return -1; } } - return rc; + return 0; } static int virCgroupSetMemoryUseHierarchy(virCgroupPtr group) @@ -677,8 +683,9 @@ static int virCgroupSetMemoryUseHierarchy(virCgroupPtr group) VIR_CGROUP_CONTROLLER_MEMORY, filename, &value); if (rc != 0) { - VIR_ERROR(_("Failed to read %s/%s (%d)"), group->path, filename, rc); - return rc; + virReportSystemError(-rc, + _("Failed to get '%s'"), filename); + return -1; } /* Setting twice causes error, so if already enabled, skip setting */ @@ -691,10 +698,12 @@ static int virCgroupSetMemoryUseHierarchy(virCgroupPtr group) filename, 1); if (rc != 0) { - VIR_ERROR(_("Failed to set %s/%s (%d)"), group->path, filename, rc); + virReportSystemError(-rc, + _("Failed to set '%s'"), filename); + return -1; } - return rc; + return 0; } static int virCgroupMakeGroup(virCgroupPtr parent, @@ -703,7 +712,7 @@ static int virCgroupMakeGroup(virCgroupPtr parent, unsigned int flags) { size_t i; - int rc = 0; + int ret = -1; VIR_DEBUG("Make group %s", group->path); for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { @@ -716,11 +725,11 @@ static int virCgroupMakeGroup(virCgroupPtr parent, continue; } - rc = virCgroupPathOfController(group, i, "", &path); - if (rc < 0) { - VIR_DEBUG("Failed to find path of controller %s", - virCgroupControllerTypeToString(i)); - return rc; + if (virCgroupPathOfController(group, i, "", &path) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to find path of controller %s"), + virCgroupControllerTypeToString(i)); + return -1; } /* As of Feb 2011, clang can't see that the above function * call did not modify group. */ @@ -736,25 +745,23 @@ static int virCgroupMakeGroup(virCgroupPtr parent, * treat blkio as unmounted if mkdir fails. */ if (i == VIR_CGROUP_CONTROLLER_BLKIO) { VIR_DEBUG("Ignoring mkdir failure with blkio controller. Kernel probably too old"); - rc = 0; VIR_FREE(group->controllers[i].mountPoint); VIR_FREE(path); continue; } else { - VIR_DEBUG("Failed to create controller %s for group", - virCgroupControllerTypeToString(i)); - rc = -errno; + virReportSystemError(errno, + _("Failed to create controller %s for group"), + virCgroupControllerTypeToString(i)); VIR_FREE(path); - break; + goto cleanup; } } if (group->controllers[VIR_CGROUP_CONTROLLER_CPUSET].mountPoint != NULL && (i == VIR_CGROUP_CONTROLLER_CPUSET || STREQ(group->controllers[i].mountPoint, group->controllers[VIR_CGROUP_CONTROLLER_CPUSET].mountPoint))) { - rc = virCgroupCpuSetInherit(parent, group); - if (rc != 0) { + if (virCgroupCpuSetInherit(parent, group) < 0) { VIR_FREE(path); - break; + goto cleanup; } } /* @@ -765,10 +772,9 @@ static int virCgroupMakeGroup(virCgroupPtr parent, (group->controllers[VIR_CGROUP_CONTROLLER_MEMORY].mountPoint != NULL) && (i == VIR_CGROUP_CONTROLLER_MEMORY || STREQ(group->controllers[i].mountPoint, group->controllers[VIR_CGROUP_CONTROLLER_MEMORY].mountPoint))) { - rc = virCgroupSetMemoryUseHierarchy(group); - if (rc != 0) { + if (virCgroupSetMemoryUseHierarchy(group) < 0) { VIR_FREE(path); - break; + goto cleanup; } } } @@ -777,7 +783,10 @@ static int virCgroupMakeGroup(virCgroupPtr parent, } VIR_DEBUG("Done making controllers for group"); - return rc; + ret = 0; + +cleanup: + return ret; } @@ -795,51 +804,41 @@ static int virCgroupMakeGroup(virCgroupPtr parent, * @parent is NULL, then the placement of the current * process is used. * + * Returns 0 on success, -1 on error */ static int virCgroupNew(const char *path, virCgroupPtr parent, int controllers, virCgroupPtr *group) { - int rc = 0; - char *typpath = NULL; - VIR_DEBUG("parent=%p path=%s controllers=%d", parent, path, controllers); *group = NULL; - if (VIR_ALLOC((*group)) != 0) { - rc = -ENOMEM; - goto err; - } + if (VIR_ALLOC((*group)) < 0) + goto error; if (path[0] == '/' || !parent) { - if (VIR_STRDUP_QUIET((*group)->path, path) < 0) { - rc = -ENOMEM; - goto err; - } + if (VIR_STRDUP((*group)->path, path) < 0) + goto error; } else { if (virAsprintf(&(*group)->path, "%s%s%s", parent->path, STREQ(parent->path, "") ? "" : "/", - path) < 0) { - rc = -ENOMEM; - goto err; - } + path) < 0) + goto error; } - rc = virCgroupDetect(*group, controllers, path, parent); - if (rc < 0) - goto err; + if (virCgroupDetect(*group, controllers, path, parent) < 0) + goto error; - return rc; -err: + return 0; + +error: virCgroupFree(group); *group = NULL; - VIR_FREE(typpath); - - return rc; + return -1; } static int virCgroupAppRoot(virCgroupPtr *group, @@ -847,22 +846,21 @@ static int virCgroupAppRoot(virCgroupPtr *group, int controllers) { virCgroupPtr selfgrp = NULL; - int rc; - - rc = virCgroupNewSelf(&selfgrp); + int ret = -1; - if (rc != 0) - return rc; + if (virCgroupNewSelf(&selfgrp) < 0) + return -1; - rc = virCgroupNew("libvirt", selfgrp, controllers, group); - if (rc != 0) + if (virCgroupNew("libvirt", selfgrp, controllers, group) < 0) goto cleanup; - rc = virCgroupMakeGroup(selfgrp, *group, create, VIR_CGROUP_NONE); + if (virCgroupMakeGroup(selfgrp, *group, create, VIR_CGROUP_NONE) < 0) + goto cleanup; + ret = 0; cleanup: virCgroupFree(&selfgrp); - return rc; + return ret; } #endif @@ -918,8 +916,9 @@ int virCgroupRemoveRecursively(char *grppath) #else int virCgroupRemoveRecursively(char *grppath ATTRIBUTE_UNUSED) { - /* Claim no support */ - return -ENXIO; + virReportSystemError(ENXIO, "%s", + _("Control groups not supported on this platform")); + return -1; } #endif @@ -1119,7 +1118,9 @@ static int virCgroupPartitionNeedsEscaping(const char *path) * if cgroups are not available on a host */ if (errno == ENOENT) errno = ENXIO; - return -errno; + virReportSystemError(errno, "%s", + _("Cannot open /proc/cgroups")); + return -1; } /* @@ -1152,7 +1153,8 @@ static int virCgroupPartitionNeedsEscaping(const char *path) } if (ferror(fp)) { - ret = -EIO; + virReportSystemError(errno, "%s", + _("Error while reading /proc/cgroups")); goto cleanup; } @@ -1172,18 +1174,18 @@ static int virCgroupPartitionEscape(char **path) return rc; if (VIR_INSERT_ELEMENT(*path, 0, len, escape) < 0) - return -ENOMEM; + return -1; return 0; } static int virCgroupSetPartitionSuffix(const char *path, char **res) { - char **tokens = virStringSplit(path, "/", 0); + char **tokens; size_t i; int ret = -1; - if (!tokens) + if (!(tokens = virStringSplit(path, "/", 0))) return ret; for (i = 0; tokens[i] != NULL; i++) { @@ -1202,23 +1204,17 @@ static int virCgroupSetPartitionSuffix(const char *path, char **res) if (STRNEQ(tokens[i], "") && !strchr(tokens[i], '.')) { if (VIR_REALLOC_N(tokens[i], - strlen(tokens[i]) + strlen(".partition") + 1) < 0) { - ret = -ENOMEM; + strlen(tokens[i]) + strlen(".partition") + 1) < 0) goto cleanup; - } strcat(tokens[i], ".partition"); } - ret = virCgroupPartitionEscape(&(tokens[i])); - if (ret < 0) { + if (virCgroupPartitionEscape(&(tokens[i])) < 0) goto cleanup; - } } - if (!(*res = virStringJoin((const char **)tokens, "/"))) { - ret = -ENOMEM; + if (!(*res = virStringJoin((const char **)tokens, "/"))) goto cleanup; - } ret = 0; @@ -1236,64 +1232,59 @@ cleanup: * Creates a new cgroup to represent the resource * partition path identified by @name. * - * Returns 0 on success, -errno on failure + * Returns 0 on success, -1 on failure */ int virCgroupNewPartition(const char *path, bool create, int controllers, virCgroupPtr *group) { - int rc; + int ret = -1; char *parentPath = NULL; virCgroupPtr parent = NULL; char *newpath = NULL; VIR_DEBUG("path=%s create=%d controllers=%x", path, create, controllers); - if (path[0] != '/') - return -EINVAL; + if (path[0] != '/') { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Partition path '%s' must start with '/'"), + path); + return -1; + } - /* XXX convert all cgroups APIs to use error report - * APIs instead of returning errno */ - rc = virCgroupSetPartitionSuffix(path, &newpath); - if (rc < 0) { - virResetLastError(); + if (virCgroupSetPartitionSuffix(path, &newpath) < 0) goto cleanup; - } - rc = virCgroupNew(newpath, NULL, controllers, group); - if (rc != 0) + if (virCgroupNew(newpath, NULL, controllers, group) < 0) goto cleanup; if (STRNEQ(newpath, "/")) { char *tmp; - if (VIR_STRDUP_QUIET(parentPath, newpath) < 0) { - rc = -ENOMEM; + if (VIR_STRDUP(parentPath, newpath) < 0) goto cleanup; - } tmp = strrchr(parentPath, '/'); tmp++; *tmp = '\0'; - rc = virCgroupNew(parentPath, NULL, controllers, &parent); - if (rc != 0) + if (virCgroupNew(parentPath, NULL, controllers, &parent) < 0) goto cleanup; - rc = virCgroupMakeGroup(parent, *group, create, VIR_CGROUP_NONE); - if (rc != 0) { + if (virCgroupMakeGroup(parent, *group, create, VIR_CGROUP_NONE) < 0) { virCgroupRemove(*group); goto cleanup; } } + ret = 0; cleanup: - if (rc != 0) + if (ret != 0) virCgroupFree(group); virCgroupFree(&parent); VIR_FREE(parentPath); VIR_FREE(newpath); - return rc; + return ret; } #else int virCgroupNewPartition(const char *path ATTRIBUTE_UNUSED, @@ -1301,8 +1292,9 @@ int virCgroupNewPartition(const char *path ATTRIBUTE_UNUSED, int controllers ATTRIBUTE_UNUSED, virCgroupPtr *group ATTRIBUTE_UNUSED) { - /* Claim no support */ - return -ENXIO; + virReportSystemError(ENXIO, "%s", + _("Control groups not supported on this platform")); + return -1; } #endif @@ -1312,7 +1304,7 @@ int virCgroupNewPartition(const char *path ATTRIBUTE_UNUSED, * @name: name of this driver (e.g., xen, qemu, lxc) * @group: Pointer to returned virCgroupPtr * - * Returns 0 on success + * Returns 0 on success, or -1 on error */ #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R int virCgroupNewDriver(const char *name, @@ -1320,26 +1312,27 @@ int virCgroupNewDriver(const char *name, int controllers, virCgroupPtr *group) { - int rc; + int ret = -1; virCgroupPtr rootgrp = NULL; - rc = virCgroupAppRoot(&rootgrp, - create, controllers); - if (rc != 0) - goto out; + if (virCgroupAppRoot(&rootgrp, + create, controllers) < 0) + goto cleanup; - rc = virCgroupNew(name, rootgrp, -1, group); - if (rc == 0) { - rc = virCgroupMakeGroup(rootgrp, *group, create, VIR_CGROUP_NONE); - if (rc != 0) { - virCgroupRemove(*group); - virCgroupFree(group); - } + if (virCgroupNew(name, rootgrp, -1, group) < 0) + goto cleanup; + + if (virCgroupMakeGroup(rootgrp, *group, create, VIR_CGROUP_NONE) < 0) { + virCgroupRemove(*group); + virCgroupFree(group); + goto cleanup; } -out: - virCgroupFree(&rootgrp); - return rc; + ret = 0; + +cleanup: + virCgroupFree(&rootgrp); + return ret; } #else int virCgroupNewDriver(const char *name ATTRIBUTE_UNUSED, @@ -1347,8 +1340,9 @@ int virCgroupNewDriver(const char *name ATTRIBUTE_UNUSED, int controllers ATTRIBUTE_UNUSED, virCgroupPtr *group ATTRIBUTE_UNUSED) { - /* Claim no support */ - return -ENXIO; + virReportSystemError(ENXIO, "%s", + _("Control groups not supported on this platform")); + return -1; } #endif @@ -1360,7 +1354,7 @@ int virCgroupNewDriver(const char *name ATTRIBUTE_UNUSED, * Obtain a cgroup representing the config of the * current process * -* Returns 0 on success +* Returns 0 on success, or -1 on error */ #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R int virCgroupNewSelf(virCgroupPtr *group) @@ -1370,7 +1364,9 @@ int virCgroupNewSelf(virCgroupPtr *group) #else int virCgroupNewSelf(virCgroupPtr *group ATTRIBUTE_UNUSED) { - return -ENXIO; + virReportSystemError(ENXIO, "%s", + _("Control groups not supported on this platform")); + return -1; } #endif @@ -1381,7 +1377,7 @@ int virCgroupNewSelf(virCgroupPtr *group ATTRIBUTE_UNUSED) * @name: name of the domain * @group: Pointer to returned virCgroupPtr * - * Returns 0 on success + * Returns 0 on success, or -1 on error */ #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R int virCgroupNewDomainDriver(virCgroupPtr driver, @@ -1389,29 +1385,30 @@ int virCgroupNewDomainDriver(virCgroupPtr driver, bool create, virCgroupPtr *group) { - int rc; + int ret = -1; - rc = virCgroupNew(name, driver, -1, group); - - if (rc == 0) { - /* - * Create a cgroup with memory.use_hierarchy enabled to - * surely account memory usage of lxc with ns subsystem - * enabled. (To be exact, memory and ns subsystems are - * enabled at the same time.) - * - * The reason why doing it here, not a upper group, say - * a group for driver, is to avoid overhead to track - * cumulative usage that we don't need. - */ - rc = virCgroupMakeGroup(driver, *group, create, VIR_CGROUP_MEM_HIERACHY); - if (rc != 0) { - virCgroupRemove(*group); - virCgroupFree(group); - } + if (virCgroupNew(name, driver, -1, group) < 0) + goto cleanup; + + /* + * Create a cgroup with memory.use_hierarchy enabled to + * surely account memory usage of lxc with ns subsystem + * enabled. (To be exact, memory and ns subsystems are + * enabled at the same time.) + * + * The reason why doing it here, not a upper group, say + * a group for driver, is to avoid overhead to track + * cumulative usage that we don't need. + */ + if (virCgroupMakeGroup(driver, *group, create, VIR_CGROUP_MEM_HIERACHY) < 0) { + virCgroupRemove(*group); + virCgroupFree(group); + goto cleanup; } - return rc; + ret = 0; +cleanup: + return ret; } #else int virCgroupNewDomainDriver(virCgroupPtr driver ATTRIBUTE_UNUSED, @@ -1419,7 +1416,9 @@ int virCgroupNewDomainDriver(virCgroupPtr driver ATTRIBUTE_UNUSED, bool create ATTRIBUTE_UNUSED, virCgroupPtr *group ATTRIBUTE_UNUSED) { - return -ENXIO; + virReportSystemError(ENXIO, "%s", + _("Control groups not supported on this platform")); + return -1; } #endif @@ -1431,7 +1430,7 @@ int virCgroupNewDomainDriver(virCgroupPtr driver ATTRIBUTE_UNUSED, * @name: name of the domain * @group: Pointer to returned virCgroupPtr * - * Returns 0 on success + * Returns 0 on success, or -1 on error */ #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R int virCgroupNewDomainPartition(virCgroupPtr partition, @@ -1440,38 +1439,40 @@ int virCgroupNewDomainPartition(virCgroupPtr partition, bool create, virCgroupPtr *group) { - int rc; + int ret = -1; char *grpname = NULL; if (virAsprintf(&grpname, "%s.libvirt-%s", name, driver) < 0) - return -ENOMEM; + goto cleanup; - if ((rc = virCgroupPartitionEscape(&grpname)) < 0) - return rc; + if (virCgroupPartitionEscape(&grpname) < 0) + goto cleanup; - rc = virCgroupNew(grpname, partition, -1, group); - - if (rc == 0) { - /* - * Create a cgroup with memory.use_hierarchy enabled to - * surely account memory usage of lxc with ns subsystem - * enabled. (To be exact, memory and ns subsystems are - * enabled at the same time.) - * - * The reason why doing it here, not a upper group, say - * a group for driver, is to avoid overhead to track - * cumulative usage that we don't need. - */ - rc = virCgroupMakeGroup(partition, *group, create, VIR_CGROUP_MEM_HIERACHY); - if (rc != 0) { - virCgroupRemove(*group); - virCgroupFree(group); - } + if (virCgroupNew(grpname, partition, -1, group) < 0) + goto cleanup; + + /* + * Create a cgroup with memory.use_hierarchy enabled to + * surely account memory usage of lxc with ns subsystem + * enabled. (To be exact, memory and ns subsystems are + * enabled at the same time.) + * + * The reason why doing it here, not a upper group, say + * a group for driver, is to avoid overhead to track + * cumulative usage that we don't need. + */ + if (virCgroupMakeGroup(partition, *group, create, VIR_CGROUP_MEM_HIERACHY) < 0) { + virCgroupRemove(*group); + virCgroupFree(group); + goto cleanup; } + ret = 0; + +cleanup: VIR_FREE(grpname); - return rc; + return ret; } #else int virCgroupNewDomainPartition(virCgroupPtr partition ATTRIBUTE_UNUSED, @@ -1480,7 +1481,9 @@ int virCgroupNewDomainPartition(virCgroupPtr partition ATTRIBUTE_UNUSED, bool create ATTRIBUTE_UNUSED, virCgroupPtr *group ATTRIBUTE_UNUSED) { - return -ENXIO; + virReportSystemError(ENXIO, "%s", + _("Control groups not supported on this platform")); + return -1; } #endif @@ -1492,7 +1495,7 @@ int virCgroupNewDomainPartition(virCgroupPtr partition ATTRIBUTE_UNUSED, * @create: true to create if not already existing * @group: Pointer to returned virCgroupPtr * - * Returns 0 on success + * Returns 0 on success, or -1 on error */ #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R int virCgroupNewVcpu(virCgroupPtr domain, @@ -1500,29 +1503,30 @@ int virCgroupNewVcpu(virCgroupPtr domain, bool create, virCgroupPtr *group) { - int rc; - char *name; + int ret = -1; + char *name = NULL; int controllers; if (virAsprintf(&name, "vcpu%d", vcpuid) < 0) - return -ENOMEM; + goto cleanup; controllers = ((1 << VIR_CGROUP_CONTROLLER_CPU) | (1 << VIR_CGROUP_CONTROLLER_CPUACCT) | (1 << VIR_CGROUP_CONTROLLER_CPUSET)); - rc = virCgroupNew(name, domain, controllers, group); - VIR_FREE(name); + if (virCgroupNew(name, domain, controllers, group) < 0) + goto cleanup; - if (rc == 0) { - rc = virCgroupMakeGroup(domain, *group, create, VIR_CGROUP_NONE); - if (rc != 0) { - virCgroupRemove(*group); - virCgroupFree(group); - } + if (virCgroupMakeGroup(domain, *group, create, VIR_CGROUP_NONE) < 0) { + virCgroupRemove(*group); + virCgroupFree(group); + goto cleanup; } - return rc; + ret = 0; +cleanup: + VIR_FREE(name); + return ret; } #else int virCgroupNewVcpu(virCgroupPtr domain ATTRIBUTE_UNUSED, @@ -1530,7 +1534,9 @@ int virCgroupNewVcpu(virCgroupPtr domain ATTRIBUTE_UNUSED, bool create ATTRIBUTE_UNUSED, virCgroupPtr *group ATTRIBUTE_UNUSED) { - return -ENXIO; + virReportSystemError(ENXIO, "%s", + _("Control groups not supported on this platform")); + return -1; } #endif @@ -1541,38 +1547,41 @@ int virCgroupNewVcpu(virCgroupPtr domain ATTRIBUTE_UNUSED, * @create: true to create if not already existing * @group: Pointer to returned virCgroupPtr * - * Returns: 0 on success or -errno on failure + * Returns: 0 on success or -1 on error */ #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R int virCgroupNewEmulator(virCgroupPtr domain, bool create, virCgroupPtr *group) { - int rc; + int ret = -1; int controllers; controllers = ((1 << VIR_CGROUP_CONTROLLER_CPU) | (1 << VIR_CGROUP_CONTROLLER_CPUACCT) | (1 << VIR_CGROUP_CONTROLLER_CPUSET)); - rc = virCgroupNew("emulator", domain, controllers, group); + if (virCgroupNew("emulator", domain, controllers, group) < 0) + goto cleanup; - if (rc == 0) { - rc = virCgroupMakeGroup(domain, *group, create, VIR_CGROUP_NONE); - if (rc != 0) { - virCgroupRemove(*group); - virCgroupFree(group); - } + if (virCgroupMakeGroup(domain, *group, create, VIR_CGROUP_NONE) < 0) { + virCgroupRemove(*group); + virCgroupFree(group); + goto cleanup; } - return rc; + ret = 0; +cleanup: + return ret; } #else int virCgroupNewEmulator(virCgroupPtr domain ATTRIBUTE_UNUSED, bool create ATTRIBUTE_UNUSED, virCgroupPtr *group ATTRIBUTE_UNUSED) { - return -ENXIO; + virReportSystemError(ENXIO, "%s", + _("Control groups not supported on this platform")); + return -1; } #endif @@ -2454,8 +2463,10 @@ static int virCgroupKillRecursiveInternal(virCgroupPtr group, int signum, virHas VIR_DEBUG("Process subdir %s", ent->d_name); - if ((rc = virCgroupNew(ent->d_name, group, -1, &subgroup)) != 0) + if (virCgroupNew(ent->d_name, group, -1, &subgroup) < 0) { + rc = -EIO; goto cleanup; + } if ((rc = virCgroupKillRecursiveInternal(subgroup, signum, pids, true)) < 0) goto cleanup; diff --git a/src/util/virerror.c b/src/util/virerror.c index ce3ab85..de4479e 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -1397,3 +1397,49 @@ void virSetErrorLogPriorityFunc(virErrorLogPriorityFunc func) { virErrorLogPriorityFilter = func; } + + +/** + * virErrorSetErrnoFromLastError: + * + * If the last error had a code of VIR_ERR_SYSTEM_ERROR + * then set errno to the value saved in the error object. + * + * If the last error had a code of VIR_ERR_NO_MEMORY + * then set errno to ENOMEM + * + * Otherwise set errno to EIO. + */ +void virErrorSetErrnoFromLastError(void) +{ + virErrorPtr err = virGetLastError(); + if (err && err->code == VIR_ERR_SYSTEM_ERROR) { + errno = err->int1; + } else if (err && err->code == VIR_ERR_NO_MEMORY) { + errno = ENOMEM; + } else { + errno = EIO; + } +} + + +/** + * virLastErrorIsSystemErrno: + * @errnum: the errno value + * + * Check if the last error reported is a system + * error with the specific errno value + * + * Returns true if the last errr was a system error with errno == @errnum + */ +bool virLastErrorIsSystemErrno(int errnum) +{ + virErrorPtr err = virGetLastError(); + if (!err) + return false; + if (err->code != VIR_ERR_SYSTEM_ERROR) + return false; + if (err->int1 != errnum) + return false; + return true; +} diff --git a/src/util/virerror.h b/src/util/virerror.h index 332a5eb..b4f2f04 100644 --- a/src/util/virerror.h +++ b/src/util/virerror.h @@ -164,4 +164,8 @@ const char *virStrerror(int theerrno, char *errBuf, size_t errBufLen); typedef int (*virErrorLogPriorityFunc)(virErrorPtr, int); void virSetErrorLogPriorityFunc(virErrorLogPriorityFunc func); +void virErrorSetErrnoFromLastError(void); + +bool virLastErrorIsSystemErrno(int errnum); + #endif diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index 22b40b4..21529a0 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -136,6 +136,18 @@ cleanup: } +#define ENSURE_ERRNO(en) \ + do { \ + if (!virLastErrorIsSystemErrno(en)) { \ + virErrorPtr err = virGetLastError(); \ + fprintf(stderr, "Did not get " #en " error code: %d\n", \ + err ? err->code : 0); \ + goto cleanup; \ + } } while (0) + + /* Asking for impossible combination since CPU is co-mounted */ + + static int testCgroupNewForDriver(const void *args ATTRIBUTE_UNUSED) { virCgroupPtr cgroup = NULL; @@ -160,27 +172,30 @@ static int testCgroupNewForDriver(const void *args ATTRIBUTE_UNUSED) [VIR_CGROUP_CONTROLLER_BLKIO] = "/libvirt/lxc", }; - if ((rv = virCgroupNewDriver("lxc", false, -1, &cgroup)) != -ENOENT) { + if ((rv = virCgroupNewDriver("lxc", false, -1, &cgroup)) != -1) { fprintf(stderr, "Unexpected found LXC cgroup: %d\n", -rv); goto cleanup; } + ENSURE_ERRNO(ENOENT); - /* Asking for impossible combination since CPU is co-mounted */ if ((rv = virCgroupNewDriver("lxc", true, (1 << VIR_CGROUP_CONTROLLER_CPU), - &cgroup)) != -EINVAL) { + &cgroup)) != -1) { fprintf(stderr, "Should not have created LXC cgroup: %d\n", -rv); goto cleanup; } + ENSURE_ERRNO(EINVAL); /* Asking for impossible combination since devices is not mounted */ if ((rv = virCgroupNewDriver("lxc", true, (1 << VIR_CGROUP_CONTROLLER_DEVICES), - &cgroup)) != -ENXIO) { + &cgroup)) != -1) { 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 */ if ((rv = virCgroupNewDriver("lxc", true, (1 << VIR_CGROUP_CONTROLLER_CPU) | @@ -264,26 +279,29 @@ static int testCgroupNewForPartition(const void *args ATTRIBUTE_UNUSED) [VIR_CGROUP_CONTROLLER_BLKIO] = "/virtualmachines.partition", }; - if ((rv = virCgroupNewPartition("/virtualmachines", false, -1, &cgroup)) != -ENOENT) { + if ((rv = virCgroupNewPartition("/virtualmachines", false, -1, &cgroup)) != -1) { fprintf(stderr, "Unexpected found /virtualmachines cgroup: %d\n", -rv); goto cleanup; } + ENSURE_ERRNO(ENOENT); /* Asking for impossible combination since CPU is co-mounted */ if ((rv = virCgroupNewPartition("/virtualmachines", true, (1 << VIR_CGROUP_CONTROLLER_CPU), - &cgroup)) != -EINVAL) { + &cgroup)) != -1) { fprintf(stderr, "Should not have created /virtualmachines cgroup: %d\n", -rv); goto cleanup; } + ENSURE_ERRNO(EINVAL); /* Asking for impossible combination since devices is not mounted */ if ((rv = virCgroupNewPartition("/virtualmachines", true, (1 << VIR_CGROUP_CONTROLLER_DEVICES), - &cgroup)) != -ENXIO) { + &cgroup)) != -1) { fprintf(stderr, "Should not have created /virtualmachines cgroup: %d\n", -rv); goto cleanup; } + ENSURE_ERRNO(ENXIO); /* Asking for small combination since devices is not mounted */ if ((rv = virCgroupNewPartition("/virtualmachines", true, @@ -324,16 +342,18 @@ static int testCgroupNewForPartitionNested(const void *args ATTRIBUTE_UNUSED) [VIR_CGROUP_CONTROLLER_BLKIO] = "/deployment.partition/production.partition", }; - if ((rv = virCgroupNewPartition("/deployment/production", false, -1, &cgroup)) != -ENOENT) { + if ((rv = virCgroupNewPartition("/deployment/production", false, -1, &cgroup)) != -1) { fprintf(stderr, "Unexpected found /deployment/production cgroup: %d\n", -rv); goto cleanup; } + ENSURE_ERRNO(ENOENT); /* Should not work, since we require /deployment to be pre-created */ - if ((rv = virCgroupNewPartition("/deployment/production", true, -1, &cgroup)) != -ENOENT) { + if ((rv = virCgroupNewPartition("/deployment/production", true, -1, &cgroup)) != -1) { fprintf(stderr, "Unexpected created /deployment/production cgroup: %d\n", -rv); goto cleanup; } + ENSURE_ERRNO(ENOENT); if ((rv = virCgroupNewPartition("/deployment", true, -1, &cgroup)) != 0) { fprintf(stderr, "Failed to create /deployment cgroup: %d\n", -rv); @@ -370,16 +390,18 @@ static int testCgroupNewForPartitionNestedDeep(const void *args ATTRIBUTE_UNUSED [VIR_CGROUP_CONTROLLER_BLKIO] = "/user/berrange.user/production.partition", }; - if ((rv = virCgroupNewPartition("/user/berrange.user/production", false, -1, &cgroup)) != -ENOENT) { + if ((rv = virCgroupNewPartition("/user/berrange.user/production", false, -1, &cgroup)) != -1) { fprintf(stderr, "Unexpected found /user/berrange.user/production cgroup: %d\n", -rv); goto cleanup; } + ENSURE_ERRNO(ENOENT); /* Should not work, since we require /user/berrange.user to be pre-created */ - if ((rv = virCgroupNewPartition("/user/berrange.user/production", true, -1, &cgroup)) != -ENOENT) { + if ((rv = virCgroupNewPartition("/user/berrange.user/production", true, -1, &cgroup)) != -1) { fprintf(stderr, "Unexpected created /user/berrange.user/production cgroup: %d\n", -rv); goto cleanup; } + ENSURE_ERRNO(ENOENT); if ((rv = virCgroupNewPartition("/user", true, -1, &cgroup)) != 0) { fprintf(stderr, "Failed to create /user/berrange.user cgroup: %d\n", -rv); -- 1.8.1.4

On Thu, Jul 18, 2013 at 09:15:28PM -0600, Eric Blake wrote: Yep, I'll split out the virerror.{c,h} additions. There are a fair few more cgroups changes to come, so I'm preferring not to try to change too much at least time. Actually the point of virErrorSetErrnoFromLastError is that it will always ensure that an errno is set - it'll default to EIO if there was no error object raised. That way if virLXCCgroupGetMeminfo forgot to raise an error, we're still going to report EIO back to the caller of lxcProcReadMeminfo, which is critical in this error path. No, it means I added virLastErrorIsSystemErrno only recently after I had done this change & didn't think to go back and use it here too :-) It'll be hit on every VM start. We could optimize a little bit so that we check for existance of /proc/cgroups, and are a complete no-op in that case. Then this would only spam logs if cgroups was enabled in the kernel, but not mounted on the host, which is probably a reasonable tradeoff. Yep, could do. The 3rd patch in this series will remove the virReportSystemError call entirely, at the same time that virCgrouPGetValueStr stops returning errno values. So we'll be ok here. Possibly because VIR_FREE expands to virFree(&typepath); and it hasn't figured out that virFree doesn't have side-effects on the rest of the system state. Well this goes away in the next patch, so it doesn't matter in the end. I was only really considering error codes used by the cgroups code here. The only reason we need this at all is to simplify integration with FUSE & I don't expect we'll use it anywhere else in libvirt, so lets keep it simple. Actually for the usage scenario: if (foobar() < 0) { virErrorSetErrnoFromLastError(); return -errno; } I wanted to guarantee that an errno would always be set even if foobar() forgot a virReportError() call in its error path. I guess we could add that. Good poi nt. 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 :|

On 07/18/2013 09:00 AM, Daniel P. Berrange wrote: So we previously reported -errno or +1 for two different types of failure... ...but now don't report any failure at all. Is this right? Shouldn't this be: if (virCgroupKillPainfully(priv->cgroup) < 0) goto cleanup; or even still distinguish the error message for processes that couldn't be killed (but maybe hoisting it into vircgroup.c)? s/kill/are killed/ s/non/none/ copy-and-paste Here, you can still return 1 without reporting any error, if you timed out. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 07/18/2013 09:00 AM, Daniel P. Berrange wrote: s/remainiing/remaining/ Aha - this cleans up something I noticed in 1/3. Can code like this be simplified to: return virCgroupSetValueU64(group, VIR_CGROUP_CONTROLLER_MEMORY, filename, 1); Ouch - at least src/qemu/qemu_cgroup.c calls this function, and still expects errno values: rc = virCgroupAddTask(cgroup_vcpu, priv->vcpupids[i]); if (rc < 0) { virReportSystemError(-rc, _("unable to add vcpu %zu task %d to cgroup"), i, priv->vcpupids[i]); goto cleanup; } I didn't look elsewhere; all I needed was one counterexample to state your patch is incomplete until you audit all callers impacted by semantic changes. shorter as 'if (virLastErrorIsSystemErrno(ESRCH))' errno is wrong here - stat() succeeded. You want to explicitly report EINVAL. You can still use ignore_value() here rather than 'if', if desired, since ret will be NULL if VIR_STRNDUP fails. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Fri, Jul 19, 2013 at 03:51:25PM +0100, Daniel P. Berrange wrote: This time with the diff attached. diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 3cec8e2..d15fa67 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -605,13 +605,8 @@ virCgroupPtr virLXCCgroupJoin(virDomainDefPtr def) if (!(cgroup = virLXCCgroupCreate(def, true))) return NULL; - rc = virCgroupAddTask(cgroup, getpid()); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to add task %d to cgroup for domain %s"), - getpid(), def->name); + if (virCgroupAddTask(cgroup, getpid()) < 0) goto cleanup; - } ret = 0; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 2df80bc..c6364fc 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -975,13 +975,8 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm) goto cleanup; /* move the thread for vcpu to sub dir */ - rc = virCgroupAddTask(cgroup_vcpu, priv->vcpupids[i]); - if (rc < 0) { - virReportSystemError(-rc, - _("unable to add vcpu %zu task %d to cgroup"), - i, priv->vcpupids[i]); + if (virCgroupAddTask(cgroup_vcpu, priv->vcpupids[i]) < 0) goto cleanup; - } if (period || quota) { if (qemuSetupCgroupVcpuBW(cgroup_vcpu, period, quota) < 0) @@ -1118,13 +1113,8 @@ qemuAddToCgroup(virDomainObjPtr vm) if (priv->cgroup == NULL) return 0; /* Not supported, so claim success */ - rc = virCgroupAddTask(priv->cgroup, getpid()); - if (rc != 0) { - virReportSystemError(-rc, - _("unable to add domain %s task %d to cgroup"), - vm->def->name, getpid()); + if (virCgroupAddTask(priv->cgroup, getpid()) < 0) return -1; - } return 0; } diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index bab7503..9d63edd 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1062,12 +1062,10 @@ static int virCgroupAddTaskStrController(virCgroupPtr group, virErrorPtr err = virGetLastError(); /* A thread that exits between when we first read the source * tasks and now is not fatal. */ - if (err && err->code == VIR_ERR_SYSTEM_ERROR && - err->int1 == ESRCH) { + if (virLastErrorIsSystemErrno(ESRCH)) virResetLastError(); - } else { + else goto cleanup; - } } next = strchr(cur, '\n'); @@ -1711,7 +1709,7 @@ int virCgroupSetBlkioDeviceWeight(virCgroupPtr group, } if (!S_ISBLK(sb.st_mode)) { - virReportSystemError(errno, + virReportSystemError(EINVAL, _("Path '%s' must be a block device"), path); return -1; 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 :|

On Fri, Jul 19, 2013 at 09:51:03AM -0600, Eric Blake wrote: Sigh, you're right. I clearly forgot to change any of the callers. diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 3cec8e2..025720d 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -36,33 +36,18 @@ static int virLXCCgroupSetupCpuTune(virDomainDefPtr def, virCgroupPtr cgroup) { int ret = -1; - if (def->cputune.shares != 0) { - int rc = virCgroupSetCpuShares(cgroup, def->cputune.shares); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to set io cpu shares for domain %s"), - def->name); - goto cleanup; - } - } - if (def->cputune.quota != 0) { - int rc = virCgroupSetCpuCfsQuota(cgroup, def->cputune.quota); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to set io cpu quota for domain %s"), - def->name); - goto cleanup; - } - } - if (def->cputune.period != 0) { - int rc = virCgroupSetCpuCfsPeriod(cgroup, def->cputune.period); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to set io cpu period for domain %s"), - def->name); - goto cleanup; - } - } + if (def->cputune.shares != 0 && + virCgroupSetCpuShares(cgroup, def->cputune.shares) < 0) + goto cleanup; + + if (def->cputune.quota != 0 && + virCgroupSetCpuCfsQuota(cgroup, def->cputune.quota) < 0) + goto cleanup; + + if (def->cputune.period != 0 && + virCgroupSetCpuCfsPeriod(cgroup, def->cputune.period) < 0) + goto cleanup; + ret = 0; cleanup: return ret; @@ -73,7 +58,7 @@ static int virLXCCgroupSetupCpusetTune(virDomainDefPtr def, virCgroupPtr cgroup, virBitmapPtr nodemask) { - int rc = 0; + int ret = -1; char *mask = NULL; if (def->placement_mode != VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO && @@ -85,12 +70,8 @@ static int virLXCCgroupSetupCpusetTune(virDomainDefPtr def, return -1; } - rc = virCgroupSetCpusetCpus(cgroup, mask); - if (rc < 0) { - virReportSystemError(-rc, "%s", - _("Unable to set cpuset.cpus")); + if (virCgroupSetCpusetCpus(cgroup, mask) < 0) goto cleanup; - } } if ((def->numatune.memory.nodemask || @@ -109,14 +90,14 @@ static int virLXCCgroupSetupCpusetTune(virDomainDefPtr def, return -1; } - rc = virCgroupSetCpusetMems(cgroup, mask); - if (rc < 0) - virReportSystemError(-rc, "%s", _("Unable to set cpuset.mems")); + if (virCgroupSetCpusetMems(cgroup, mask) < 0) + goto cleanup; } + ret = 0; cleanup: VIR_FREE(mask); - return rc; + return ret; } @@ -124,31 +105,18 @@ static int virLXCCgroupSetupBlkioTune(virDomainDefPtr def, virCgroupPtr cgroup) { size_t i; - int rc; - - if (def->blkio.weight) { - rc = virCgroupSetBlkioWeight(cgroup, def->blkio.weight); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to set Blkio weight for domain %s"), - def->name); - return -1; - } - } + + if (def->blkio.weight && + virCgroupSetBlkioWeight(cgroup, def->blkio.weight) < 0) + return -1; if (def->blkio.ndevices) { for (i = 0; i < def->blkio.ndevices; i++) { virBlkioDeviceWeightPtr dw = &def->blkio.devices[i]; if (!dw->weight) continue; - rc = virCgroupSetBlkioDeviceWeight(cgroup, dw->path, dw->weight); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to set io device weight " - "for domain %s"), - def->name); + if (virCgroupSetBlkioDeviceWeight(cgroup, dw->path, dw->weight) < 0) return -1; - } } } @@ -160,45 +128,21 @@ static int virLXCCgroupSetupMemTune(virDomainDefPtr def, virCgroupPtr cgroup) { int ret = -1; - int rc; - rc = virCgroupSetMemory(cgroup, def->mem.max_balloon); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to set memory limit for domain %s"), - def->name); + if (virCgroupSetMemory(cgroup, def->mem.max_balloon) < 0) goto cleanup; - } - if (def->mem.hard_limit) { - rc = virCgroupSetMemoryHardLimit(cgroup, def->mem.hard_limit); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to set memory hard limit for domain %s"), - def->name); - goto cleanup; - } - } + if (def->mem.hard_limit && + virCgroupSetMemoryHardLimit(cgroup, def->mem.hard_limit) < 0) + goto cleanup; - if (def->mem.soft_limit) { - rc = virCgroupSetMemorySoftLimit(cgroup, def->mem.soft_limit); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to set memory soft limit for domain %s"), - def->name); - goto cleanup; - } - } + if (def->mem.soft_limit && + virCgroupSetMemorySoftLimit(cgroup, def->mem.soft_limit) < 0) + goto cleanup; - if (def->mem.swap_hard_limit) { - rc = virCgroupSetMemSwapHardLimit(cgroup, def->mem.swap_hard_limit); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to set swap hard limit for domain %s"), - def->name); - goto cleanup; - } - } + if (def->mem.swap_hard_limit && + virCgroupSetMemSwapHardLimit(cgroup, def->mem.swap_hard_limit) < 0) + goto cleanup; ret = 0; cleanup: @@ -306,32 +250,20 @@ cleanup: int virLXCCgroupGetMeminfo(virLXCMeminfoPtr meminfo) { - int ret = -1, rc; + int ret = -1; virCgroupPtr cgroup; if (virCgroupNewSelf(&cgroup) < 0) return -1; - rc = virLXCCgroupGetMemStat(cgroup, meminfo); - if (rc < 0) { - virReportSystemError(-rc, "%s", - _("Unable to get memory cgroup stat info")); + if (virLXCCgroupGetMemStat(cgroup, meminfo) < 0) goto cleanup; - } - rc = virLXCCgroupGetMemTotal(cgroup, meminfo); - if (rc < 0) { - virReportSystemError(-rc, "%s", - _("Unable to get memory cgroup total")); + if (virLXCCgroupGetMemTotal(cgroup, meminfo) < 0) goto cleanup; - } - rc = virLXCCgroupGetMemUsage(cgroup, meminfo); - if (rc < 0) { - virReportSystemError(-rc, "%s", - _("Unable to get memory cgroup stat usage")); + if (virLXCCgroupGetMemUsage(cgroup, meminfo) < 0) goto cleanup; - } virLXCCgroupGetMemSwapTotal(cgroup, meminfo); virLXCCgroupGetMemSwapUsage(cgroup, meminfo); @@ -360,17 +292,11 @@ virLXCSetupHostUsbDeviceCgroup(virUSBDevicePtr dev ATTRIBUTE_UNUSED, void *opaque) { virCgroupPtr cgroup = opaque; - int rc; VIR_DEBUG("Process path '%s' for USB device", path); - rc = virCgroupAllowDevicePath(cgroup, path, - VIR_CGROUP_DEVICE_RW); - if (rc < 0) { - virReportSystemError(-rc, - _("Unable to allow device %s"), - path); + if (virCgroupAllowDevicePath(cgroup, path, + VIR_CGROUP_DEVICE_RW) < 0) return -1; - } return 0; } @@ -382,17 +308,11 @@ virLXCTeardownHostUsbDeviceCgroup(virUSBDevicePtr dev ATTRIBUTE_UNUSED, void *opaque) { virCgroupPtr cgroup = opaque; - int rc; VIR_DEBUG("Process path '%s' for USB device", path); - rc = virCgroupDenyDevicePath(cgroup, path, - VIR_CGROUP_DEVICE_RW); - if (rc < 0) { - virReportSystemError(-rc, - _("Unable to deny device %s"), - path); + if (virCgroupDenyDevicePath(cgroup, path, + VIR_CGROUP_DEVICE_RW) < 0) return -1; - } return 0; } @@ -402,7 +322,6 @@ static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def, virCgroupPtr cgroup) { int ret = -1; - int rc; size_t i; static virLXCCgroupDevicePolicy devices[] = { {'c', LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_NULL}, @@ -415,62 +334,42 @@ static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def, {'c', LXC_DEV_MAJ_FUSE, LXC_DEV_MIN_FUSE}, {0, 0, 0}}; - rc = virCgroupDenyAllDevices(cgroup); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to deny devices for domain %s"), - def->name); + if (virCgroupDenyAllDevices(cgroup) < 0) goto cleanup; - } for (i = 0; devices[i].type != 0; i++) { virLXCCgroupDevicePolicyPtr dev = &devices[i]; - rc = virCgroupAllowDevice(cgroup, - dev->type, - dev->major, - dev->minor, - VIR_CGROUP_DEVICE_RWM); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to allow device %c:%d:%d for domain %s"), - dev->type, dev->major, dev->minor, def->name); + if (virCgroupAllowDevice(cgroup, + dev->type, + dev->major, + dev->minor, + VIR_CGROUP_DEVICE_RWM) < 0) goto cleanup; - } } for (i = 0; i < def->ndisks; i++) { if (def->disks[i]->type != VIR_DOMAIN_DISK_TYPE_BLOCK) continue; - rc = virCgroupAllowDevicePath(cgroup, - def->disks[i]->src, - (def->disks[i]->readonly ? - VIR_CGROUP_DEVICE_READ : - VIR_CGROUP_DEVICE_RW) | - VIR_CGROUP_DEVICE_MKNOD); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to allow device %s for domain %s"), - def->disks[i]->src, def->name); + if (virCgroupAllowDevicePath(cgroup, + def->disks[i]->src, + (def->disks[i]->readonly ? + VIR_CGROUP_DEVICE_READ : + VIR_CGROUP_DEVICE_RW) | + VIR_CGROUP_DEVICE_MKNOD) < 0) goto cleanup; - } } for (i = 0; i < def->nfss; i++) { if (def->fss[i]->type != VIR_DOMAIN_FS_TYPE_BLOCK) continue; - rc = virCgroupAllowDevicePath(cgroup, - def->fss[i]->src, - def->fss[i]->readonly ? - VIR_CGROUP_DEVICE_READ : - VIR_CGROUP_DEVICE_RW); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to allow device %s for domain %s"), - def->fss[i]->src, def->name); + if (virCgroupAllowDevicePath(cgroup, + def->fss[i]->src, + def->fss[i]->readonly ? + VIR_CGROUP_DEVICE_READ : + VIR_CGROUP_DEVICE_RW) < 0) goto cleanup; - } } for (i = 0; i < def->nhostdevs; i++) { @@ -520,14 +419,9 @@ static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def, } } - rc = virCgroupAllowDeviceMajor(cgroup, 'c', LXC_DEV_MAJ_PTY, - VIR_CGROUP_DEVICE_RWM); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to allow PTY devices for domain %s"), - def->name); + if (virCgroupAllowDeviceMajor(cgroup, 'c', LXC_DEV_MAJ_PTY, + VIR_CGROUP_DEVICE_RWM) < 0) goto cleanup; - } ret = 0; cleanup: @@ -600,18 +494,12 @@ virCgroupPtr virLXCCgroupJoin(virDomainDefPtr def) { virCgroupPtr cgroup = NULL; int ret = -1; - int rc; if (!(cgroup = virLXCCgroupCreate(def, true))) return NULL; - rc = virCgroupAddTask(cgroup, getpid()); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to add task %d to cgroup for domain %s"), - getpid(), def->name); + if (virCgroupAddTask(cgroup, getpid()) < 0) goto cleanup; - } ret = 0; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index e005d8d..0fa98df 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -562,7 +562,7 @@ static int lxcDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) { virDomainObjPtr vm; - int ret = -1, rc; + int ret = -1; virLXCDomainObjPrivatePtr priv; if (!(vm = lxcDomObjFromDomain(dom))) @@ -584,14 +584,12 @@ static int lxcDomainGetInfo(virDomainPtr dom, "%s", _("Cannot read cputime for domain")); goto cleanup; } - if ((rc = virCgroupGetMemoryUsage(priv->cgroup, &(info->memory))) < 0) { - virReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("Cannot read memory usage for domain")); - if (rc == -ENOENT) { - /* Don't fail if we can't read memory usage due to a lack of - * kernel support */ + if (virCgroupGetMemoryUsage(priv->cgroup, &(info->memory)) < 0) { + /* Don't fail if we can't read memory usage due to a lack of + * kernel support */ + if (virLastErrorIsSystemErrno(ENOENT)) info->memory = 0; - } else + else goto cleanup; } } @@ -746,7 +744,6 @@ lxcDomainSetMemoryParameters(virDomainPtr dom, size_t i; virDomainObjPtr vm = NULL; int ret = -1; - int rc; virLXCDomainObjPrivatePtr priv; virCheckFlags(0, -1); @@ -773,26 +770,14 @@ lxcDomainSetMemoryParameters(virDomainPtr dom, virTypedParameterPtr param = &params[i]; if (STREQ(param->field, VIR_DOMAIN_MEMORY_HARD_LIMIT)) { - rc = virCgroupSetMemoryHardLimit(priv->cgroup, params[i].value.ul); - if (rc != 0) { - virReportSystemError(-rc, "%s", - _("unable to set memory hard_limit tunable")); + if (virCgroupSetMemoryHardLimit(priv->cgroup, params[i].value.ul) < 0) ret = -1; - } } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_SOFT_LIMIT)) { - rc = virCgroupSetMemorySoftLimit(priv->cgroup, params[i].value.ul); - if (rc != 0) { - virReportSystemError(-rc, "%s", - _("unable to set memory soft_limit tunable")); + if (virCgroupSetMemorySoftLimit(priv->cgroup, params[i].value.ul) < 0) ret = -1; - } } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT)) { - rc = virCgroupSetMemSwapHardLimit(priv->cgroup, params[i].value.ul); - if (rc != 0) { - virReportSystemError(-rc, "%s", - _("unable to set swap_hard_limit tunable")); + if (virCgroupSetMemSwapHardLimit(priv->cgroup, params[i].value.ul) < 0) ret = -1; - } } } @@ -812,7 +797,6 @@ lxcDomainGetMemoryParameters(virDomainPtr dom, virDomainObjPtr vm = NULL; unsigned long long val; int ret = -1; - int rc; virLXCDomainObjPrivatePtr priv; virCheckFlags(0, -1); @@ -838,34 +822,22 @@ lxcDomainGetMemoryParameters(virDomainPtr dom, switch (i) { case 0: /* fill memory hard limit here */ - rc = virCgroupGetMemoryHardLimit(priv->cgroup, &val); - if (rc != 0) { - virReportSystemError(-rc, "%s", - _("unable to get memory hard limit")); + if (virCgroupGetMemoryHardLimit(priv->cgroup, &val) < 0) goto cleanup; - } if (virTypedParameterAssign(param, VIR_DOMAIN_MEMORY_HARD_LIMIT, VIR_TYPED_PARAM_ULLONG, val) < 0) goto cleanup; break; case 1: /* fill memory soft limit here */ - rc = virCgroupGetMemorySoftLimit(priv->cgroup, &val); - if (rc != 0) { - virReportSystemError(-rc, "%s", - _("unable to get memory soft limit")); + if (virCgroupGetMemorySoftLimit(priv->cgroup, &val) < 0) goto cleanup; - } if (virTypedParameterAssign(param, VIR_DOMAIN_MEMORY_SOFT_LIMIT, VIR_TYPED_PARAM_ULLONG, val) < 0) goto cleanup; break; case 2: /* fill swap hard limit here */ - rc = virCgroupGetMemSwapHardLimit(priv->cgroup, &val); - if (rc != 0) { - virReportSystemError(-rc, "%s", - _("unable to get swap hard limit")); + if (virCgroupGetMemSwapHardLimit(priv->cgroup, &val) < 0) goto cleanup; - } if (virTypedParameterAssign(param, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT, VIR_TYPED_PARAM_ULLONG, val) < 0) @@ -1676,21 +1648,11 @@ static int lxcGetVcpuBWLive(virCgroupPtr cgroup, unsigned long long *period, long long *quota) { - int rc; - - rc = virCgroupGetCpuCfsPeriod(cgroup, period); - if (rc < 0) { - virReportSystemError(-rc, "%s", - _("unable to get cpu bandwidth period tunable")); + if (virCgroupGetCpuCfsPeriod(cgroup, period) < 0) return -1; - } - rc = virCgroupGetCpuCfsQuota(cgroup, quota); - if (rc < 0) { - virReportSystemError(-rc, "%s", - _("unable to get cpu bandwidth tunable")); + if (virCgroupGetCpuCfsQuota(cgroup, quota) < 0) return -1; - } return 0; } @@ -1699,7 +1661,6 @@ lxcGetVcpuBWLive(virCgroupPtr cgroup, unsigned long long *period, static int lxcSetVcpuBWLive(virCgroupPtr cgroup, unsigned long long period, long long quota) { - int rc; unsigned long long old_period; if (period == 0 && quota == 0) @@ -1707,39 +1668,23 @@ static int lxcSetVcpuBWLive(virCgroupPtr cgroup, unsigned long long period, if (period) { /* get old period, and we can rollback if set quota failed */ - rc = virCgroupGetCpuCfsPeriod(cgroup, &old_period); - if (rc < 0) { - virReportSystemError(-rc, - "%s", _("Unable to get cpu bandwidth period")); + if (virCgroupGetCpuCfsPeriod(cgroup, &old_period) < 0) return -1; - } - rc = virCgroupSetCpuCfsPeriod(cgroup, period); - if (rc < 0) { - virReportSystemError(-rc, - "%s", _("Unable to set cpu bandwidth period")); + if (virCgroupSetCpuCfsPeriod(cgroup, period) < 0) return -1; - } } if (quota) { - rc = virCgroupSetCpuCfsQuota(cgroup, quota); - if (rc < 0) { - virReportSystemError(-rc, - "%s", _("Unable to set cpu bandwidth quota")); - goto cleanup; - } + if (virCgroupSetCpuCfsQuota(cgroup, quota) < 0) + goto error; } return 0; -cleanup: - if (period) { - rc = virCgroupSetCpuCfsPeriod(cgroup, old_period); - if (rc < 0) - virReportSystemError(-rc, "%s", - _("Unable to rollback cpu bandwidth period")); - } +error: + if (period) + virCgroupSetCpuCfsPeriod(cgroup, old_period); return -1; } @@ -1808,12 +1753,8 @@ lxcDomainSetSchedulerParametersFlags(virDomainPtr dom, if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_CPU_SHARES)) { if (flags & VIR_DOMAIN_AFFECT_LIVE) { - rc = virCgroupSetCpuShares(priv->cgroup, params[i].value.ul); - if (rc != 0) { - virReportSystemError(-rc, "%s", - _("unable to set cpu shares tunable")); + if (virCgroupSetCpuShares(priv->cgroup, params[i].value.ul) < 0) goto cleanup; - } vm->def->cputune.shares = params[i].value.ul; } @@ -1942,12 +1883,8 @@ lxcDomainGetSchedulerParametersFlags(virDomainPtr dom, goto cleanup; } - rc = virCgroupGetCpuShares(priv->cgroup, &shares); - if (rc != 0) { - virReportSystemError(-rc, "%s", - _("unable to get cpu shares tunable")); + if (virCgroupGetCpuShares(priv->cgroup, &shares) < 0) goto cleanup; - } if (*nparams > 1 && cpu_bw_status) { rc = lxcGetVcpuBWLive(priv->cgroup, &period, &quota); @@ -2047,20 +1984,14 @@ lxcDomainSetBlkioParameters(virDomainPtr dom, virTypedParameterPtr param = &params[i]; if (STREQ(param->field, VIR_DOMAIN_BLKIO_WEIGHT)) { - int rc; - if (params[i].value.ui > 1000 || params[i].value.ui < 100) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("out of blkio weight range.")); goto cleanup; } - rc = virCgroupSetBlkioWeight(priv->cgroup, params[i].value.ui); - if (rc != 0) { - virReportSystemError(-rc, "%s", - _("unable to set blkio weight tunable")); + if (virCgroupSetBlkioWeight(priv->cgroup, params[i].value.ui) < 0) goto cleanup; - } } } } @@ -2110,7 +2041,6 @@ lxcDomainGetBlkioParameters(virDomainPtr dom, virDomainDefPtr persistentDef = NULL; unsigned int val; int ret = -1; - int rc; virLXCDomainObjPrivatePtr priv; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | @@ -2151,12 +2081,8 @@ lxcDomainGetBlkioParameters(virDomainPtr dom, switch (i) { case 0: /* fill blkio weight here */ - rc = virCgroupGetBlkioWeight(priv->cgroup, &val); - if (rc != 0) { - virReportSystemError(-rc, "%s", - _("unable to get blkio weight")); + if (virCgroupGetBlkioWeight(priv->cgroup, &val) < 0) goto cleanup; - } if (virTypedParameterAssign(param, VIR_DOMAIN_BLKIO_WEIGHT, VIR_TYPED_PARAM_UINT, val) < 0) goto cleanup; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 2df80bc..eed396f 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -54,25 +54,23 @@ qemuSetupDiskPathAllow(virDomainDiskDefPtr disk, { virDomainObjPtr vm = opaque; qemuDomainObjPrivatePtr priv = vm->privateData; - int rc; + int ret; VIR_DEBUG("Process path %s for disk", path); - rc = virCgroupAllowDevicePath(priv->cgroup, path, + ret = virCgroupAllowDevicePath(priv->cgroup, path, (disk->readonly ? VIR_CGROUP_DEVICE_READ : VIR_CGROUP_DEVICE_RW)); virDomainAuditCgroupPath(vm, priv->cgroup, "allow", path, - disk->readonly ? "r" : "rw", rc); - if (rc < 0) { - if (rc == -EACCES) { /* Get this for root squash NFS */ - VIR_DEBUG("Ignoring EACCES for %s", path); - } else { - virReportSystemError(-rc, - _("Unable to allow access for disk path %s"), - path); - return -1; - } + disk->readonly ? "r" : "rw", ret == 0); + + /* Get this for root squash NFS */ + if (ret < 0 && + virLastErrorIsSystemErrno(EACCES)) { + VIR_DEBUG("Ignoring EACCES for %s", path); + virResetLastError(); + ret = 0; } - return 0; + return ret; } @@ -98,23 +96,21 @@ qemuTeardownDiskPathDeny(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, { virDomainObjPtr vm = opaque; qemuDomainObjPrivatePtr priv = vm->privateData; - int rc; + int ret; VIR_DEBUG("Process path %s for disk", path); - rc = virCgroupDenyDevicePath(priv->cgroup, path, - VIR_CGROUP_DEVICE_RWM); - virDomainAuditCgroupPath(vm, priv->cgroup, "deny", path, "rwm", rc); - if (rc < 0) { - if (rc == -EACCES) { /* Get this for root squash NFS */ - VIR_DEBUG("Ignoring EACCES for %s", path); - } else { - virReportSystemError(-rc, - _("Unable to deny access for disk path %s"), - path); - return -1; - } + ret = virCgroupDenyDevicePath(priv->cgroup, path, + VIR_CGROUP_DEVICE_RWM); + virDomainAuditCgroupPath(vm, priv->cgroup, "deny", path, "rwm", ret == 0); + + /* Get this for root squash NFS */ + if (ret < 0 && + virLastErrorIsSystemErrno(EACCES)) { + VIR_DEBUG("Ignoring EACCES for %s", path); + virResetLastError(); + ret = 0; } - return 0; + return ret; } @@ -135,31 +131,25 @@ qemuTeardownDiskCgroup(virDomainObjPtr vm, } static int -qemuSetupChrSourceCgroup(virDomainDefPtr def, +qemuSetupChrSourceCgroup(virDomainDefPtr def ATTRIBUTE_UNUSED, virDomainChrSourceDefPtr dev, void *opaque) { virDomainObjPtr vm = opaque; qemuDomainObjPrivatePtr priv = vm->privateData; - int rc; + int ret; if (dev->type != VIR_DOMAIN_CHR_TYPE_DEV) return 0; VIR_DEBUG("Process path '%s' for device", dev->data.file.path); - rc = virCgroupAllowDevicePath(priv->cgroup, dev->data.file.path, - VIR_CGROUP_DEVICE_RW); + ret = virCgroupAllowDevicePath(priv->cgroup, dev->data.file.path, + VIR_CGROUP_DEVICE_RW); virDomainAuditCgroupPath(vm, priv->cgroup, "allow", - dev->data.file.path, "rw", rc); - if (rc < 0) { - virReportSystemError(-rc, - _("Unable to allow device %s for %s"), - dev->data.file.path, def->name); - return -1; - } + dev->data.file.path, "rw", ret == 0); - return 0; + return ret; } static int @@ -176,18 +166,18 @@ qemuSetupTPMCgroup(virDomainDefPtr def, virDomainTPMDefPtr dev, void *opaque) { - int rc = 0; + int ret = 0; switch (dev->type) { case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: - rc = qemuSetupChrSourceCgroup(def, &dev->data.passthrough.source, - opaque); + ret = qemuSetupChrSourceCgroup(def, &dev->data.passthrough.source, + opaque); break; case VIR_DOMAIN_TPM_TYPE_LAST: break; } - return rc; + return ret; } @@ -198,20 +188,14 @@ qemuSetupHostUsbDeviceCgroup(virUSBDevicePtr dev ATTRIBUTE_UNUSED, { virDomainObjPtr vm = opaque; qemuDomainObjPrivatePtr priv = vm->privateData; - int rc; + int ret; VIR_DEBUG("Process path '%s' for USB device", path); - rc = virCgroupAllowDevicePath(priv->cgroup, path, + ret = virCgroupAllowDevicePath(priv->cgroup, path, VIR_CGROUP_DEVICE_RW); - virDomainAuditCgroupPath(vm, priv->cgroup, "allow", path, "rw", rc); - if (rc < 0) { - virReportSystemError(-rc, - _("Unable to allow device %s"), - path); - return -1; - } + virDomainAuditCgroupPath(vm, priv->cgroup, "allow", path, "rw", ret == 0); - return 0; + return ret; } static int @@ -221,25 +205,19 @@ qemuSetupHostScsiDeviceCgroup(virSCSIDevicePtr dev ATTRIBUTE_UNUSED, { virDomainObjPtr vm = opaque; qemuDomainObjPrivatePtr priv = vm->privateData; - int rc; + int ret; VIR_DEBUG("Process path '%s' for SCSI device", path); - rc = virCgroupAllowDevicePath(priv->cgroup, path, - virSCSIDeviceGetReadonly(dev) ? - VIR_CGROUP_DEVICE_READ : - VIR_CGROUP_DEVICE_RW); + ret = virCgroupAllowDevicePath(priv->cgroup, path, + virSCSIDeviceGetReadonly(dev) ? + VIR_CGROUP_DEVICE_READ : + VIR_CGROUP_DEVICE_RW); virDomainAuditCgroupPath(vm, priv->cgroup, "allow", path, - virSCSIDeviceGetReadonly(dev) ? "r" : "rw", rc); - if (rc < 0) { - virReportSystemError(-rc, - _("Unable to allow device %s"), - path); - return -1; - } + virSCSIDeviceGetReadonly(dev) ? "r" : "rw", ret == 0); - return 0; + return ret; } int @@ -267,7 +245,7 @@ qemuSetupHostdevCGroup(virDomainObjPtr vm, case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: if (dev->source.subsys.u.pci.backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { - int rc; + int rv; pci = virPCIDeviceNew(dev->source.subsys.u.pci.addr.domain, dev->source.subsys.u.pci.addr.bus, @@ -280,17 +258,12 @@ qemuSetupHostdevCGroup(virDomainObjPtr vm, goto cleanup; VIR_DEBUG("Cgroup allow %s for PCI device assignment", path); - rc = virCgroupAllowDevicePath(priv->cgroup, path, + rv = virCgroupAllowDevicePath(priv->cgroup, path, VIR_CGROUP_DEVICE_RW); virDomainAuditCgroupPath(vm, priv->cgroup, - "allow", path, "rw", rc); - if (rc < 0) { - virReportSystemError(-rc, - _("Unable to allow access " - "for device path %s"), - path); + "allow", path, "rw", rv == 0); + if (rv < 0) goto cleanup; - } } break; @@ -366,7 +339,7 @@ qemuTeardownHostdevCgroup(virDomainObjPtr vm, case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: if (dev->source.subsys.u.pci.backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { - int rc; + int rv; pci = virPCIDeviceNew(dev->source.subsys.u.pci.addr.domain, dev->source.subsys.u.pci.addr.bus, @@ -379,17 +352,12 @@ qemuTeardownHostdevCgroup(virDomainObjPtr vm, goto cleanup; VIR_DEBUG("Cgroup deny %s for PCI device assignment", path); - rc = virCgroupDenyDevicePath(priv->cgroup, path, + rv = virCgroupDenyDevicePath(priv->cgroup, path, VIR_CGROUP_DEVICE_RWM); virDomainAuditCgroupPath(vm, priv->cgroup, - "deny", path, "rwm", rc); - if (rc < 0) { - virReportSystemError(-rc, - _("Unable to deny access " - "for device path %s"), - path); + "deny", path, "rwm", rv == 0); + if (rv < 0) goto cleanup; - } } break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: @@ -411,7 +379,6 @@ static int qemuSetupBlkioCgroup(virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; - int rc = -1; size_t i; if (!virCgroupHasController(priv->cgroup, @@ -425,30 +392,18 @@ qemuSetupBlkioCgroup(virDomainObjPtr vm) } } - if (vm->def->blkio.weight != 0) { - rc = virCgroupSetBlkioWeight(priv->cgroup, vm->def->blkio.weight); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to set io weight for domain %s"), - vm->def->name); - return -1; - } - } + if (vm->def->blkio.weight != 0 && + virCgroupSetBlkioWeight(priv->cgroup, vm->def->blkio.weight) < 0) + return -1; if (vm->def->blkio.ndevices) { for (i = 0; i < vm->def->blkio.ndevices; i++) { virBlkioDeviceWeightPtr dw = &vm->def->blkio.devices[i]; if (!dw->weight) continue; - rc = virCgroupSetBlkioDeviceWeight(priv->cgroup, dw->path, - dw->weight); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to set io device weight " - "for domain %s"), - vm->def->name); + if (virCgroupSetBlkioDeviceWeight(priv->cgroup, dw->path, + dw->weight) < 0) return -1; - } } } @@ -460,7 +415,6 @@ static int qemuSetupMemoryCgroup(virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; - int rc; if (!virCgroupHasController(priv->cgroup,VIR_CGROUP_CONTROLLER_MEMORY)) { if (vm->def->mem.hard_limit != 0 || @@ -474,33 +428,17 @@ qemuSetupMemoryCgroup(virDomainObjPtr vm) } } - rc = virCgroupSetMemoryHardLimit(priv->cgroup, - qemuDomainMemoryLimit(vm->def)); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to set memory hard limit for domain %s"), - vm->def->name); + if (virCgroupSetMemoryHardLimit(priv->cgroup, + qemuDomainMemoryLimit(vm->def)) < 0) return -1; - } - if (vm->def->mem.soft_limit != 0) { - rc = virCgroupSetMemorySoftLimit(priv->cgroup, vm->def->mem.soft_limit); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to set memory soft limit for domain %s"), - vm->def->name); - return -1; - } - } - if (vm->def->mem.swap_hard_limit != 0) { - rc = virCgroupSetMemSwapHardLimit(priv->cgroup, vm->def->mem.swap_hard_limit); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to set swap hard limit for domain %s"), - vm->def->name); - return -1; - } - } + if (vm->def->mem.soft_limit != 0 && + virCgroupSetMemorySoftLimit(priv->cgroup, vm->def->mem.soft_limit) < 0) + return -1; + + if (vm->def->mem.swap_hard_limit != 0 && + virCgroupSetMemSwapHardLimit(priv->cgroup, vm->def->mem.swap_hard_limit) < 0) + return -1; return 0; } @@ -513,23 +451,22 @@ qemuSetupDevicesCgroup(virQEMUDriverPtr driver, qemuDomainObjPrivatePtr priv = vm->privateData; virQEMUDriverConfigPtr cfg = NULL; const char *const *deviceACL = NULL; - int rc = -1; + int rv = -1; int ret = -1; size_t i; if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) return 0; - rc = virCgroupDenyAllDevices(priv->cgroup); - virDomainAuditCgroup(vm, priv->cgroup, "deny", "all", rc == 0); - if (rc != 0) { - if (rc == -EPERM) { + rv = virCgroupDenyAllDevices(priv->cgroup); + virDomainAuditCgroup(vm, priv->cgroup, "deny", "all", rv == 0); + if (rv < 0) { + if (virLastErrorIsSystemErrno(EPERM)) { + virResetLastError(); VIR_WARN("Group devices ACL is not accessible, disabling whitelisting"); return 0; } - virReportSystemError(-rc, - _("Unable to deny all devices for %s"), vm->def->name); goto cleanup; } @@ -538,15 +475,12 @@ qemuSetupDevicesCgroup(virQEMUDriverPtr driver, goto cleanup; } - rc = virCgroupAllowDeviceMajor(priv->cgroup, 'c', DEVICE_PTY_MAJOR, + rv = virCgroupAllowDeviceMajor(priv->cgroup, 'c', DEVICE_PTY_MAJOR, VIR_CGROUP_DEVICE_RW); virDomainAuditCgroupMajor(vm, priv->cgroup, "allow", DEVICE_PTY_MAJOR, - "pty", "rw", rc == 0); - if (rc != 0) { - virReportSystemError(-rc, "%s", - _("unable to allow /dev/pts/ devices")); + "pty", "rw", rv == 0); + if (rv < 0) goto cleanup; - } cfg = virQEMUDriverGetConfig(driver); deviceACL = cfg->cgroupDeviceACL ? @@ -558,15 +492,12 @@ qemuSetupDevicesCgroup(virQEMUDriverPtr driver, ((vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && cfg->vncAllowHostAudio) || (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL)))) { - rc = virCgroupAllowDeviceMajor(priv->cgroup, 'c', DEVICE_SND_MAJOR, + rv = virCgroupAllowDeviceMajor(priv->cgroup, 'c', DEVICE_SND_MAJOR, VIR_CGROUP_DEVICE_RW); virDomainAuditCgroupMajor(vm, priv->cgroup, "allow", DEVICE_SND_MAJOR, - "sound", "rw", rc == 0); - if (rc != 0) { - virReportSystemError(-rc, "%s", - _("unable to allow /dev/snd/ devices")); + "sound", "rw", rv == 0); + if (rv < 0) goto cleanup; - } } for (i = 0; deviceACL[i] != NULL; i++) { @@ -576,16 +507,12 @@ qemuSetupDevicesCgroup(virQEMUDriverPtr driver, continue; } - rc = virCgroupAllowDevicePath(priv->cgroup, deviceACL[i], + rv = virCgroupAllowDevicePath(priv->cgroup, deviceACL[i], VIR_CGROUP_DEVICE_RW); - virDomainAuditCgroupPath(vm, priv->cgroup, "allow", deviceACL[i], "rw", rc); - if (rc < 0 && - rc != -ENOENT) { - virReportSystemError(-rc, - _("unable to allow device %s"), - deviceACL[i]); + virDomainAuditCgroupPath(vm, priv->cgroup, "allow", deviceACL[i], "rw", rv == 0); + if (rv < 0 && + !virLastErrorIsSystemErrno(ENOENT)) goto cleanup; - } } if (virDomainChrDefForeach(vm->def, @@ -620,7 +547,6 @@ qemuSetupCpusetCgroup(virDomainObjPtr vm, qemuDomainObjPrivatePtr priv = vm->privateData; char *mem_mask = NULL; char *cpu_mask = NULL; - int rc; int ret = -1; if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) @@ -643,14 +569,8 @@ qemuSetupCpusetCgroup(virDomainObjPtr vm, goto cleanup; } - rc = virCgroupSetCpusetMems(priv->cgroup, mem_mask); - - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to set cpuset.mems for domain %s"), - vm->def->name); + if (virCgroupSetCpusetMems(priv->cgroup, mem_mask) < 0) goto cleanup; - } } if (vm->def->cpumask || @@ -672,12 +592,8 @@ qemuSetupCpusetCgroup(virDomainObjPtr vm, goto cleanup; } - if ((rc = virCgroupSetCpusetCpus(priv->cgroup, cpu_mask)) != 0) { - virReportSystemError(-rc, - _("Unable to set cpuset.cpus for domain %s"), - vm->def->name); + if (virCgroupSetCpusetCpus(priv->cgroup, cpu_mask) < 0) goto cleanup; - } } ret = 0; @@ -692,7 +608,6 @@ static int qemuSetupCpuCgroup(virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; - int rc = -1; if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { if (vm->def->cputune.shares) { @@ -704,15 +619,9 @@ qemuSetupCpuCgroup(virDomainObjPtr vm) } } - if (vm->def->cputune.shares) { - rc = virCgroupSetCpuShares(priv->cgroup, vm->def->cputune.shares); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to set io cpu shares for domain %s"), - vm->def->name); - return -1; - } - } + if (vm->def->cputune.shares && + virCgroupSetCpuShares(priv->cgroup, vm->def->cputune.shares) < 0) + return -1; return 0; } @@ -723,7 +632,7 @@ qemuInitCgroup(virQEMUDriverPtr driver, virDomainObjPtr vm, bool startup) { - int rc = -1; + int ret = -1; qemuDomainObjPrivatePtr priv = vm->privateData; virCgroupPtr parent = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); @@ -795,11 +704,11 @@ qemuInitCgroup(virQEMUDriverPtr driver, } done: - rc = 0; + ret = 0; cleanup: virCgroupFree(&parent); virObjectUnref(cfg); - return rc; + return ret; } @@ -847,7 +756,6 @@ qemuSetupCgroupVcpuBW(virCgroupPtr cgroup, unsigned long long period, long long quota) { - int rc; unsigned long long old_period; if (period == 0 && quota == 0) @@ -855,39 +763,22 @@ qemuSetupCgroupVcpuBW(virCgroupPtr cgroup, if (period) { /* get old period, and we can rollback if set quota failed */ - rc = virCgroupGetCpuCfsPeriod(cgroup, &old_period); - if (rc < 0) { - virReportSystemError(-rc, - "%s", _("Unable to get cpu bandwidth period")); + if (virCgroupGetCpuCfsPeriod(cgroup, &old_period) < 0) return -1; - } - rc = virCgroupSetCpuCfsPeriod(cgroup, period); - if (rc < 0) { - virReportSystemError(-rc, - "%s", _("Unable to set cpu bandwidth period")); + if (virCgroupSetCpuCfsPeriod(cgroup, period) < 0) return -1; - } } - if (quota) { - rc = virCgroupSetCpuCfsQuota(cgroup, quota); - if (rc < 0) { - virReportSystemError(-rc, - "%s", _("Unable to set cpu bandwidth quota")); - goto cleanup; - } - } + if (quota && + virCgroupSetCpuCfsQuota(cgroup, quota) < 0) + goto error; return 0; -cleanup: - if (period) { - rc = virCgroupSetCpuCfsPeriod(cgroup, old_period); - if (rc < 0) - virReportSystemError(-rc, "%s", - _("Unable to rollback cpu bandwidth period")); - } +error: + if (period) + ignore_value(virCgroupSetCpuCfsPeriod(cgroup, old_period)); return -1; } @@ -913,28 +804,23 @@ int qemuSetupCgroupEmulatorPin(virCgroupPtr cgroup, virBitmapPtr cpumask) { - int rc = 0; + int ret = -1; char *new_cpus = NULL; new_cpus = virBitmapFormat(cpumask); if (!new_cpus) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to convert cpu mask")); - rc = -1; goto cleanup; } - rc = virCgroupSetCpusetCpus(cgroup, new_cpus); - if (rc < 0) { - virReportSystemError(-rc, - "%s", - _("Unable to set cpuset.cpus")); + if (virCgroupSetCpusetCpus(cgroup, new_cpus) < 0) goto cleanup; - } + ret = 0; cleanup: VIR_FREE(new_cpus); - return rc; + return ret; } int @@ -943,7 +829,6 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm) virCgroupPtr cgroup_vcpu = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; virDomainDefPtr def = vm->def; - int rc; size_t i, j; unsigned long long period = vm->def->cputune.period; long long quota = vm->def->cputune.quota; @@ -975,13 +860,8 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm) goto cleanup; /* move the thread for vcpu to sub dir */ - rc = virCgroupAddTask(cgroup_vcpu, priv->vcpupids[i]); - if (rc < 0) { - virReportSystemError(-rc, - _("unable to add vcpu %zu task %d to cgroup"), - i, priv->vcpupids[i]); + if (virCgroupAddTask(cgroup_vcpu, priv->vcpupids[i]) < 0) goto cleanup; - } if (period || quota) { if (qemuSetupCgroupVcpuBW(cgroup_vcpu, period, quota) < 0) @@ -1032,7 +912,6 @@ qemuSetupCgroupForEmulator(virQEMUDriverPtr driver, qemuDomainObjPrivatePtr priv = vm->privateData; unsigned long long period = vm->def->cputune.emulator_period; long long quota = vm->def->cputune.emulator_quota; - int rc = -1; if ((period || quota) && !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { @@ -1047,14 +926,8 @@ qemuSetupCgroupForEmulator(virQEMUDriverPtr driver, if (virCgroupNewEmulator(priv->cgroup, true, &cgroup_emulator) < 0) goto cleanup; - rc = virCgroupMoveTask(priv->cgroup, cgroup_emulator); - if (rc < 0) { - virReportSystemError(-rc, - _("Unable to move tasks from domain cgroup to " - "emulator cgroup for %s"), - vm->def->name); + if (virCgroupMoveTask(priv->cgroup, cgroup_emulator) < 0) goto cleanup; - } if (def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) { if (!(cpumap = qemuPrepareCpumap(driver, nodemask))) @@ -1067,20 +940,16 @@ qemuSetupCgroupForEmulator(virQEMUDriverPtr driver, } if (cpumask) { - if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) { - rc = qemuSetupCgroupEmulatorPin(cgroup_emulator, cpumask); - if (rc < 0) - goto cleanup; - } - cpumask = NULL; /* sanity */ + if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET) && + qemuSetupCgroupEmulatorPin(cgroup_emulator, cpumask) < 0) + goto cleanup; } if (period || quota) { - if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { - if ((rc = qemuSetupCgroupVcpuBW(cgroup_emulator, period, - quota)) < 0) - goto cleanup; - } + if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU) && + qemuSetupCgroupVcpuBW(cgroup_emulator, period, + quota) < 0) + goto cleanup; } virCgroupFree(&cgroup_emulator); @@ -1095,7 +964,7 @@ cleanup: virCgroupFree(&cgroup_emulator); } - return rc; + return -1; } int @@ -1113,18 +982,12 @@ int qemuAddToCgroup(virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; - int rc; if (priv->cgroup == NULL) return 0; /* Not supported, so claim success */ - rc = virCgroupAddTask(priv->cgroup, getpid()); - if (rc != 0) { - virReportSystemError(-rc, - _("unable to add domain %s task %d to cgroup"), - vm->def->name, getpid()); + if (virCgroupAddTask(priv->cgroup, getpid()) < 0) return -1; - } return 0; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e88267c..9bbc5d6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7749,7 +7749,6 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, ret = 0; if (flags & VIR_DOMAIN_AFFECT_LIVE) { for (i = 0; i < nparams; i++) { - int rc; virTypedParameterPtr param = &params[i]; if (STREQ(param->field, VIR_DOMAIN_BLKIO_WEIGHT)) { @@ -7760,12 +7759,8 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, continue; } - rc = virCgroupSetBlkioWeight(priv->cgroup, params[i].value.ui); - if (rc != 0) { - virReportSystemError(-rc, "%s", - _("unable to set blkio weight tunable")); + if (virCgroupSetBlkioWeight(priv->cgroup, params[i].value.ui) < 0) ret = -1; - } } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { size_t ndevices; virBlkioDeviceWeightPtr devices = NULL; @@ -7778,14 +7773,10 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, continue; } for (j = 0; j < ndevices; j++) { - rc = virCgroupSetBlkioDeviceWeight(priv->cgroup, - devices[j].path, - devices[j].weight); - if (rc < 0) { - virReportSystemError(-rc, - _("Unable to set io device weight " - "for path %s"), - devices[j].path); + if (virCgroupSetBlkioDeviceWeight(priv->cgroup, + devices[j].path, + devices[j].weight) < 0) { + ret = -1; break; } } @@ -7860,7 +7851,6 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, virDomainDefPtr persistentDef = NULL; unsigned int val; int ret = -1; - int rc; virCapsPtr caps = NULL; qemuDomainObjPrivatePtr priv; @@ -7910,12 +7900,8 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, switch (i) { case 0: /* fill blkio weight here */ - rc = virCgroupGetBlkioWeight(priv->cgroup, &val); - if (rc != 0) { - virReportSystemError(-rc, "%s", - _("unable to get blkio weight")); + if (virCgroupGetBlkioWeight(priv->cgroup, &val) < 0) goto cleanup; - } if (virTypedParameterAssign(param, VIR_DOMAIN_BLKIO_WEIGHT, VIR_TYPED_PARAM_UINT, val) < 0) goto cleanup; @@ -8040,8 +8026,8 @@ qemuDomainSetMemoryParameters(virDomainPtr dom, bool set_hard_limit = false; bool set_soft_limit = false; virQEMUDriverConfigPtr cfg = NULL; - int ret = -1; int rc; + int ret = -1; virCapsPtr caps = NULL; qemuDomainObjPrivatePtr priv; @@ -8173,7 +8159,6 @@ qemuDomainGetMemoryParameters(virDomainPtr dom, virDomainObjPtr vm = NULL; virDomainDefPtr persistentDef = NULL; int ret = -1; - int rc; virCapsPtr caps = NULL; qemuDomainObjPrivatePtr priv; @@ -8257,12 +8242,8 @@ qemuDomainGetMemoryParameters(virDomainPtr dom, switch (i) { case 0: /* fill memory hard limit here */ - rc = virCgroupGetMemoryHardLimit(priv->cgroup, &val); - if (rc != 0) { - virReportSystemError(-rc, "%s", - _("unable to get memory hard limit")); + if (virCgroupGetMemoryHardLimit(priv->cgroup, &val) < 0) goto cleanup; - } if (virTypedParameterAssign(param, VIR_DOMAIN_MEMORY_HARD_LIMIT, VIR_TYPED_PARAM_ULLONG, val) < 0) @@ -8270,12 +8251,8 @@ qemuDomainGetMemoryParameters(virDomainPtr dom, break; case 1: /* fill memory soft limit here */ - rc = virCgroupGetMemorySoftLimit(priv->cgroup, &val); - if (rc != 0) { - virReportSystemError(-rc, "%s", - _("unable to get memory soft limit")); + if (virCgroupGetMemorySoftLimit(priv->cgroup, &val) < 0) goto cleanup; - } if (virTypedParameterAssign(param, VIR_DOMAIN_MEMORY_SOFT_LIMIT, VIR_TYPED_PARAM_ULLONG, val) < 0) @@ -8283,13 +8260,9 @@ qemuDomainGetMemoryParameters(virDomainPtr dom, break; case 2: /* fill swap hard limit here */ - rc = virCgroupGetMemSwapHardLimit(priv->cgroup, &val); - if (rc != 0) { - if (rc != -ENOENT) { - virReportSystemError(-rc, "%s", - _("unable to get swap hard limit")); + if (virCgroupGetMemSwapHardLimit(priv->cgroup, &val) < 0) { + if (!virLastErrorIsSystemErrno(ENOENT)) goto cleanup; - } val = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; } if (virTypedParameterAssign(param, @@ -8382,7 +8355,6 @@ qemuDomainSetNumaParameters(virDomainPtr dom, persistentDef->numatune.memory.mode = params[i].value.i; } } else if (STREQ(param->field, VIR_DOMAIN_NUMA_NODESET)) { - int rc; virBitmapPtr nodeset = NULL; char *nodeset_str = NULL; @@ -8415,9 +8387,7 @@ qemuDomainSetNumaParameters(virDomainPtr dom, continue; } - if ((rc = virCgroupSetCpusetMems(priv->cgroup, nodeset_str)) != 0) { - virReportSystemError(-rc, "%s", - _("unable to set numa tunable")); + if (virCgroupSetCpusetMems(priv->cgroup, nodeset_str) < 0) { virBitmapFree(nodeset); VIR_FREE(nodeset_str); ret = -1; @@ -8474,7 +8444,6 @@ qemuDomainGetNumaParameters(virDomainPtr dom, virDomainDefPtr persistentDef = NULL; char *nodeset = NULL; int ret = -1; - int rc; virCapsPtr caps = NULL; qemuDomainObjPrivatePtr priv; @@ -8536,12 +8505,8 @@ qemuDomainGetNumaParameters(virDomainPtr dom, if (!nodeset && VIR_STRDUP(nodeset, "") < 0) goto cleanup; } else { - rc = virCgroupGetCpusetMems(priv->cgroup, &nodeset); - if (rc != 0) { - virReportSystemError(-rc, "%s", - _("unable to get numa nodeset")); + if (virCgroupGetCpusetMems(priv->cgroup, &nodeset) < 0) goto cleanup; - } } if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_NODESET, VIR_TYPED_PARAM_STRING, nodeset) < 0) @@ -8712,11 +8677,8 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_CPU_SHARES)) { if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if ((rc = virCgroupSetCpuShares(priv->cgroup, value_ul))) { - virReportSystemError(-rc, "%s", - _("unable to set cpu shares tunable")); + if (virCgroupSetCpuShares(priv->cgroup, value_ul) < 0) goto cleanup; - } vm->def->cputune.shares = value_ul; } @@ -8823,21 +8785,11 @@ static int qemuGetVcpuBWLive(virCgroupPtr cgroup, unsigned long long *period, long long *quota) { - int rc; - - rc = virCgroupGetCpuCfsPeriod(cgroup, period); - if (rc < 0) { - virReportSystemError(-rc, "%s", - _("unable to get cpu bandwidth period tunable")); + if (virCgroupGetCpuCfsPeriod(cgroup, period) < 0) return -1; - } - rc = virCgroupGetCpuCfsQuota(cgroup, quota); - if (rc < 0) { - virReportSystemError(-rc, "%s", - _("unable to get cpu bandwidth tunable")); + if (virCgroupGetCpuCfsQuota(cgroup, quota) < 0) return -1; - } return 0; } @@ -8979,12 +8931,8 @@ qemuDomainGetSchedulerParametersFlags(virDomainPtr dom, goto cleanup; } - rc = virCgroupGetCpuShares(priv->cgroup, &shares); - if (rc != 0) { - virReportSystemError(-rc, "%s", - _("unable to get cpu shares tunable")); + if (virCgroupGetCpuShares(priv->cgroup, &shares) < 0) goto cleanup; - } if (*nparams > 1 && cpu_bw_status) { rc = qemuGetVcpusBWLive(vm, &period, &quota); diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index d285aa1..3dedfe8 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -4522,8 +4522,8 @@ qemuMigrationToFile(virQEMUDriverPtr driver, virDomainObjPtr vm, enum qemuDomainAsyncJob asyncJob) { qemuDomainObjPrivatePtr priv = vm->privateData; - int ret = -1; int rc; + int ret = -1; bool restoreLabel = false; virCommandPtr cmd = NULL; int pipeFD[2] = { -1, -1 }; @@ -4557,15 +4557,12 @@ qemuMigrationToFile(virQEMUDriverPtr driver, virDomainObjPtr vm, * that botches pclose. */ if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) { - rc = virCgroupAllowDevicePath(priv->cgroup, path, - VIR_CGROUP_DEVICE_RW); - virDomainAuditCgroupPath(vm, priv->cgroup, "allow", path, "rw", rc); - if (rc == 1) { + int rv = virCgroupAllowDevicePath(priv->cgroup, path, + VIR_CGROUP_DEVICE_RW); + virDomainAuditCgroupPath(vm, priv->cgroup, "allow", path, "rw", rv == 0); + if (rv == 1) { /* path was not a device, no further need for cgroup */ - } else if (rc < 0) { - virReportSystemError(-rc, - _("Unable to allow device %s for %s"), - path, vm->def->name); + } else if (rv < 0) { goto cleanup; } } @@ -4664,12 +4661,9 @@ cleanup: if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) { - rc = virCgroupDenyDevicePath(priv->cgroup, path, - VIR_CGROUP_DEVICE_RWM); - virDomainAuditCgroupPath(vm, priv->cgroup, "deny", path, "rwm", rc); - if (rc < 0) - VIR_WARN("Unable to deny device %s for %s %d", - path, vm->def->name, rc); + int rv = virCgroupDenyDevicePath(priv->cgroup, path, + VIR_CGROUP_DEVICE_RWM); + virDomainAuditCgroupPath(vm, priv->cgroup, "deny", path, "rwm", rv == 0); } return ret; } 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 :|