From: Daniel P. Berrange Date: Mon, 8 Jul 2013 10:08:46 +0000 (+0100) Subject: Convert remainder of cgroups code to report errors X-Git-Url: http://xenbits.xensource.com/gitweb?a=commitdiff_plain;h=0d7f45ae;p=libvirt.git Convert remainder of cgroups code to report errors Convert the remaining methods in vircgroup.c to report errors instead of returning errno values. Signed-off-by: Daniel P. Berrange --- diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 3cec8e2d77..025720dff7 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 e005d8d537..21cf2e39e1 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,15 +584,15 @@ 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)) { + virResetLastError(); info->memory = 0; - } else + } else { goto cleanup; + } } } @@ -746,7 +746,6 @@ lxcDomainSetMemoryParameters(virDomainPtr dom, size_t i; virDomainObjPtr vm = NULL; int ret = -1; - int rc; virLXCDomainObjPrivatePtr priv; virCheckFlags(0, -1); @@ -773,26 +772,14 @@ lxcDomainSetMemoryParameters(virDomainPtr dom, virTypedParameterPtr param = ¶ms[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 +799,6 @@ lxcDomainGetMemoryParameters(virDomainPtr dom, virDomainObjPtr vm = NULL; unsigned long long val; int ret = -1; - int rc; virLXCDomainObjPrivatePtr priv; virCheckFlags(0, -1); @@ -838,34 +824,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 +1650,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 +1663,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,38 +1670,28 @@ 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: +error: if (period) { - rc = virCgroupSetCpuCfsPeriod(cgroup, old_period); - if (rc < 0) - virReportSystemError(-rc, "%s", - _("Unable to rollback cpu bandwidth period")); + virErrorPtr saved = virSaveLastError(); + virCgroupSetCpuCfsPeriod(cgroup, old_period); + if (saved) { + virSetError(saved); + virFreeError(saved); + } } return -1; @@ -1808,12 +1761,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 +1891,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, "a); @@ -2047,20 +1992,14 @@ lxcDomainSetBlkioParameters(virDomainPtr dom, virTypedParameterPtr param = ¶ms[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 +2049,6 @@ lxcDomainGetBlkioParameters(virDomainPtr dom, virDomainDefPtr persistentDef = NULL; unsigned int val; int ret = -1; - int rc; virLXCDomainObjPrivatePtr priv; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | @@ -2151,12 +2089,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 2df80bc14e..02d2770af0 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, - (disk->readonly ? VIR_CGROUP_DEVICE_READ - : VIR_CGROUP_DEVICE_RW)); + 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, - 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; - } + ret = virCgroupAllowDevicePath(priv->cgroup, path, + VIR_CGROUP_DEVICE_RW); + 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,38 +763,27 @@ 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: +error: if (period) { - rc = virCgroupSetCpuCfsPeriod(cgroup, old_period); - if (rc < 0) - virReportSystemError(-rc, "%s", - _("Unable to rollback cpu bandwidth period")); + virErrorPtr saved = virSaveLastError(); + ignore_value(virCgroupSetCpuCfsPeriod(cgroup, old_period)); + if (saved) { + virSetError(saved); + virFreeError(saved); + } } return -1; @@ -913,28 +810,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 +835,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 +866,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 +918,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 +932,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 +946,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 +970,7 @@ cleanup: virCgroupFree(&cgroup_emulator); } - return rc; + return -1; } int @@ -1113,18 +988,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 5292863577..0af76a521d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7755,7 +7755,6 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, ret = 0; if (flags & VIR_DOMAIN_AFFECT_LIVE) { for (i = 0; i < nparams; i++) { - int rc; virTypedParameterPtr param = ¶ms[i]; if (STREQ(param->field, VIR_DOMAIN_BLKIO_WEIGHT)) { @@ -7766,12 +7765,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; @@ -7784,14 +7779,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; } } @@ -7866,7 +7857,6 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, virDomainDefPtr persistentDef = NULL; unsigned int val; int ret = -1; - int rc; virCapsPtr caps = NULL; qemuDomainObjPrivatePtr priv; @@ -7916,12 +7906,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; @@ -8046,8 +8032,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; @@ -8179,7 +8165,6 @@ qemuDomainGetMemoryParameters(virDomainPtr dom, virDomainObjPtr vm = NULL; virDomainDefPtr persistentDef = NULL; int ret = -1; - int rc; virCapsPtr caps = NULL; qemuDomainObjPrivatePtr priv; @@ -8263,12 +8248,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) @@ -8276,12 +8257,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) @@ -8289,13 +8266,10 @@ 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) && + !virLastErrorIsSystemErrno(EOPNOTSUPP)) goto cleanup; - } val = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; } if (virTypedParameterAssign(param, @@ -8388,7 +8362,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; @@ -8421,9 +8394,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; @@ -8480,7 +8451,6 @@ qemuDomainGetNumaParameters(virDomainPtr dom, virDomainDefPtr persistentDef = NULL; char *nodeset = NULL; int ret = -1; - int rc; virCapsPtr caps = NULL; qemuDomainObjPrivatePtr priv; @@ -8542,12 +8512,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) @@ -8718,11 +8684,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; } @@ -8829,21 +8792,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; } @@ -8985,12 +8938,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, "a); diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index d285aa1422..3dedfe8da3 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; } diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 124da787b2..525161181f 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -496,20 +496,31 @@ int virCgroupPathOfController(virCgroupPtr group, } } } - if (controller == -1) - return -ENOSYS; + if (controller == -1) { + virReportSystemError(ENOSYS, "%s", + _("No controllers are mounted")); + return -1; + } - if (group->controllers[controller].mountPoint == NULL) - return -ENOENT; + if (group->controllers[controller].mountPoint == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Controller '%s' is not mounted"), + virCgroupControllerTypeToString(controller)); + return -1; + } - if (group->controllers[controller].placement == NULL) - return -ENOENT; + if (group->controllers[controller].placement == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Controller '%s' is not enabled for group"), + virCgroupControllerTypeToString(controller)); + return -1; + } if (virAsprintf(path, "%s%s/%s", group->controllers[controller].mountPoint, group->controllers[controller].placement, - key ? key : "") == -1) - return -ENOMEM; + key ? key : "") < 0) + return -1; return 0; } @@ -520,25 +531,24 @@ static int virCgroupSetValueStr(virCgroupPtr group, const char *key, const char *value) { - int rc = 0; + int ret = -1; char *keypath = NULL; - rc = virCgroupPathOfController(group, controller, key, &keypath); - if (rc != 0) - return rc; + if (virCgroupPathOfController(group, controller, key, &keypath) < 0) + return -1; VIR_DEBUG("Set value '%s' to '%s'", keypath, value); - rc = virFileWriteStr(keypath, value, 0); - if (rc < 0) { - rc = -errno; - VIR_DEBUG("Failed to write value '%s': %m", value); - } else { - rc = 0; + if (virFileWriteStr(keypath, value, 0) < 0) { + virReportSystemError(errno, + _("Unable to write to '%s'"), keypath); + goto cleanup; } - VIR_FREE(keypath); + ret = 0; - return rc; +cleanup: + VIR_FREE(keypath); + return ret; } static int virCgroupGetValueStr(virCgroupPtr group, @@ -546,34 +556,31 @@ static int virCgroupGetValueStr(virCgroupPtr group, const char *key, char **value) { - int rc; char *keypath = NULL; + int ret = -1, rc; *value = NULL; - rc = virCgroupPathOfController(group, controller, key, &keypath); - if (rc != 0) { - VIR_DEBUG("No path of %s, %s", group->path, key); - return rc; - } + if (virCgroupPathOfController(group, controller, key, &keypath) < 0) + return -1; VIR_DEBUG("Get value %s", keypath); - rc = virFileReadAll(keypath, 1024*1024, value); - if (rc < 0) { - rc = -errno; - VIR_DEBUG("Failed to read %s: %m\n", keypath); - } else { - /* Terminated with '\n' has sometimes harmful effects to the caller */ - if (rc > 0 && (*value)[rc - 1] == '\n') - (*value)[rc - 1] = '\0'; - - rc = 0; + if ((rc = virFileReadAll(keypath, 1024*1024, value)) < 0) { + virReportSystemError(errno, + _("Unable to read from '%s'"), keypath); + goto cleanup; } - VIR_FREE(keypath); + /* Terminated with '\n' has sometimes harmful effects to the caller */ + if (rc > 0 && (*value)[rc - 1] == '\n') + (*value)[rc - 1] = '\0'; - return rc; + ret = 0; + +cleanup: + VIR_FREE(keypath); + return ret; } static int virCgroupSetValueU64(virCgroupPtr group, @@ -582,16 +589,16 @@ static int virCgroupSetValueU64(virCgroupPtr group, unsigned long long int value) { char *strval = NULL; - int rc; + int ret; - if (virAsprintf(&strval, "%llu", value) == -1) - return -ENOMEM; + if (virAsprintf(&strval, "%llu", value) < 0) + return -1; - rc = virCgroupSetValueStr(group, controller, key, strval); + ret = virCgroupSetValueStr(group, controller, key, strval); VIR_FREE(strval); - return rc; + return ret; } @@ -602,16 +609,16 @@ static int virCgroupSetValueI64(virCgroupPtr group, long long int value) { char *strval = NULL; - int rc; + int ret; - if (virAsprintf(&strval, "%lld", value) == -1) - return -ENOMEM; + if (virAsprintf(&strval, "%lld", value) < 0) + return -1; - rc = virCgroupSetValueStr(group, controller, key, strval); + ret = virCgroupSetValueStr(group, controller, key, strval); VIR_FREE(strval); - return rc; + return ret; } static int virCgroupGetValueI64(virCgroupPtr group, @@ -620,18 +627,23 @@ static int virCgroupGetValueI64(virCgroupPtr group, long long int *value) { char *strval = NULL; - int rc = 0; + int ret = -1; - rc = virCgroupGetValueStr(group, controller, key, &strval); - if (rc != 0) - goto out; + if (virCgroupGetValueStr(group, controller, key, &strval) < 0) + goto cleanup; - if (virStrToLong_ll(strval, NULL, 10, value) < 0) - rc = -EINVAL; -out: - VIR_FREE(strval); + if (virStrToLong_ll(strval, NULL, 10, value) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse '%s' as an integer"), + strval); + goto cleanup; + } - return rc; + ret = 0; + +cleanup: + VIR_FREE(strval); + return ret; } static int virCgroupGetValueU64(virCgroupPtr group, @@ -640,18 +652,23 @@ static int virCgroupGetValueU64(virCgroupPtr group, unsigned long long int *value) { char *strval = NULL; - int rc = 0; + int ret = -1; - rc = virCgroupGetValueStr(group, controller, key, &strval); - if (rc != 0) - goto out; + if (virCgroupGetValueStr(group, controller, key, &strval) < 0) + goto cleanup; - if (virStrToLong_ull(strval, NULL, 10, value) < 0) - rc = -EINVAL; -out: - VIR_FREE(strval); + if (virStrToLong_ull(strval, NULL, 10, value) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse '%s' as an integer"), + strval); + goto cleanup; + } - return rc; + ret = 0; + +cleanup: + VIR_FREE(strval); + return ret; } @@ -659,7 +676,6 @@ out: static int virCgroupCpuSetInherit(virCgroupPtr parent, virCgroupPtr group) { size_t i; - int rc = 0; const char *inherit_values[] = { "cpuset.cpus", "cpuset.mems", @@ -669,29 +685,22 @@ static int virCgroupCpuSetInherit(virCgroupPtr parent, virCgroupPtr group) for (i = 0; i < ARRAY_CARDINALITY(inherit_values); i++) { char *value; - rc = virCgroupGetValueStr(parent, - VIR_CGROUP_CONTROLLER_CPUSET, - inherit_values[i], - &value); - if (rc != 0) { - virReportSystemError(-rc, - _("Failed to get '%s'"), inherit_values[i]); + if (virCgroupGetValueStr(parent, + VIR_CGROUP_CONTROLLER_CPUSET, + inherit_values[i], + &value) < 0) return -1; - } VIR_DEBUG("Inherit %s = %s", inherit_values[i], value); - rc = virCgroupSetValueStr(group, - VIR_CGROUP_CONTROLLER_CPUSET, - inherit_values[i], - value); - VIR_FREE(value); - - if (rc != 0) { - virReportSystemError(-rc, - _("Failed to set '%s'"), inherit_values[i]); + if (virCgroupSetValueStr(group, + VIR_CGROUP_CONTROLLER_CPUSET, + inherit_values[i], + value) < 0) { + VIR_FREE(value); return -1; } + VIR_FREE(value); } return 0; @@ -699,33 +708,23 @@ static int virCgroupCpuSetInherit(virCgroupPtr parent, virCgroupPtr group) static int virCgroupSetMemoryUseHierarchy(virCgroupPtr group) { - int rc = 0; unsigned long long value; const char *filename = "memory.use_hierarchy"; - rc = virCgroupGetValueU64(group, - VIR_CGROUP_CONTROLLER_MEMORY, - filename, &value); - if (rc != 0) { - virReportSystemError(-rc, - _("Failed to get '%s'"), filename); + if (virCgroupGetValueU64(group, + VIR_CGROUP_CONTROLLER_MEMORY, + filename, &value) < 0) return -1; - } /* Setting twice causes error, so if already enabled, skip setting */ if (value == 1) return 0; VIR_DEBUG("Setting up %s/%s", group->path, filename); - rc = virCgroupSetValueU64(group, - VIR_CGROUP_CONTROLLER_MEMORY, - filename, 1); - - if (rc != 0) { - virReportSystemError(-rc, - _("Failed to set '%s'"), filename); + if (virCgroupSetValueU64(group, + VIR_CGROUP_CONTROLLER_MEMORY, + filename, 1) < 0) return -1; - } return 0; } @@ -749,12 +748,9 @@ static int virCgroupMakeGroup(virCgroupPtr parent, continue; } - if (virCgroupPathOfController(group, i, "", &path) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to find path of controller %s"), - virCgroupControllerTypeToString(i)); + if (virCgroupPathOfController(group, i, "", &path) < 0) return -1; - } + /* As of Feb 2011, clang can't see that the above function * call did not modify group. */ sa_assert(group->controllers[i].mountPoint); @@ -992,11 +988,11 @@ int virCgroupRemove(virCgroupPtr group) * @group: The cgroup to add a task to * @pid: The pid of the task to add * - * Returns: 0 on success + * Returns: 0 on success, -1 on error */ int virCgroupAddTask(virCgroupPtr group, pid_t pid) { - int rc = 0; + int ret = -1; size_t i; for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { @@ -1004,12 +1000,13 @@ int virCgroupAddTask(virCgroupPtr group, pid_t pid) if (!group->controllers[i].mountPoint) continue; - rc = virCgroupSetValueU64(group, i, "tasks", (unsigned long long)pid); - if (rc != 0) - break; + if (virCgroupSetValueU64(group, i, "tasks", (unsigned long long)pid) < 0) + goto cleanup; } - return rc; + ret = 0; +cleanup: + return ret; } /** @@ -1019,15 +1016,22 @@ int virCgroupAddTask(virCgroupPtr group, pid_t pid) * @pid: The pid of the task to add * @controller: The cgroup controller to be operated on * - * Returns: 0 on success or -errno on failure + * Returns: 0 on success or -1 on error */ int virCgroupAddTaskController(virCgroupPtr group, pid_t pid, int controller) { - if (controller < 0 || controller >= VIR_CGROUP_CONTROLLER_LAST) - return -EINVAL; + if (controller < 0 || controller >= VIR_CGROUP_CONTROLLER_LAST) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Controller %d out of range"), controller); + return -1; + } - if (!group->controllers[controller].mountPoint) - return -EINVAL; + if (!group->controllers[controller].mountPoint) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Controller '%s' not mounted"), + virCgroupControllerTypeToString(controller)); + return -1; + } return virCgroupSetValueU64(group, controller, "tasks", (unsigned long long)pid); @@ -1043,22 +1047,25 @@ static int virCgroupAddTaskStrController(virCgroupPtr group, int rc = 0; char *endp; - if (VIR_STRDUP_QUIET(str, pidstr) < 0) - return -ENOMEM; + if (VIR_STRDUP(str, pidstr) < 0) + return -1; cur = str; while (*cur != '\0') { - rc = virStrToLong_ull(cur, &endp, 10, &p); - if (rc != 0) + if (virStrToLong_ull(cur, &endp, 10, &p) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot parse '%s' as an integer"), cur); goto cleanup; + } - rc = virCgroupAddTaskController(group, p, controller); - /* A thread that exits between when we first read the source - * tasks and now is not fatal. */ - if (rc == -ESRCH) - rc = 0; - else if (rc != 0) - goto cleanup; + if (virCgroupAddTaskController(group, p, controller) < 0) { + /* A thread that exits between when we first read the source + * tasks and now is not fatal. */ + if (virLastErrorIsSystemErrno(ESRCH)) + virResetLastError(); + else + goto cleanup; + } next = strchr(cur, '\n'); if (next) { @@ -1081,11 +1088,11 @@ cleanup: * @dest_group: The destination where all tasks are added to * @controller: The cgroup controller to be operated on * - * Returns: 0 on success or -errno on failure + * Returns: 0 on success or -1 on failure */ int virCgroupMoveTask(virCgroupPtr src_group, virCgroupPtr dest_group) { - int rc = 0; + int ret = -1; char *content = NULL; size_t i; @@ -1100,21 +1107,21 @@ int virCgroupMoveTask(virCgroupPtr src_group, virCgroupPtr dest_group) * until content is empty. */ while (1) { VIR_FREE(content); - rc = virCgroupGetValueStr(src_group, i, "tasks", &content); - if (rc != 0) - return rc; + if (virCgroupGetValueStr(src_group, i, "tasks", &content) < 0) + return -1; + if (!*content) break; - rc = virCgroupAddTaskStrController(dest_group, content, i); - if (rc != 0) + if (virCgroupAddTaskStrController(dest_group, content, i) < 0) goto cleanup; } } + ret = 0; cleanup: VIR_FREE(content); - return rc; + return ret; } @@ -1628,12 +1635,16 @@ bool virCgroupNewIgnoreError(void) * @group: The cgroup to change io weight for * @weight: The Weight for this cgroup * - * Returns: 0 on success + * Returns: 0 on success, -1 on error */ int virCgroupSetBlkioWeight(virCgroupPtr group, unsigned int weight) { - if (weight > 1000 || weight < 100) - return -EINVAL; + if (weight > 1000 || weight < 100) { + virReportError(VIR_ERR_INVALID_ARG, + _("weight '%u' must be in range (100, 1000)"), + weight); + return -1; + } return virCgroupSetValueU64(group, VIR_CGROUP_CONTROLLER_BLKIO, @@ -1647,7 +1658,7 @@ int virCgroupSetBlkioWeight(virCgroupPtr group, unsigned int weight) * @group: The cgroup to get weight for * @Weight: Pointer to returned weight * - * Returns: 0 on success + * Returns: 0 on success, -1 on error */ int virCgroupGetBlkioWeight(virCgroupPtr group, unsigned int *weight) { @@ -1671,7 +1682,7 @@ int virCgroupGetBlkioWeight(virCgroupPtr group, unsigned int *weight) * device_weight is treated as a write-only parameter, so * there isn't a getter counterpart. * - * Returns: 0 on success, -errno on failure + * Returns: 0 on success, -1 on error */ #if defined(major) && defined(minor) int virCgroupSetBlkioDeviceWeight(virCgroupPtr group, @@ -1682,18 +1693,30 @@ int virCgroupSetBlkioDeviceWeight(virCgroupPtr group, struct stat sb; int ret; - if (weight && (weight > 1000 || weight < 100)) - return -EINVAL; + if (weight && (weight > 1000 || weight < 100)) { + virReportError(VIR_ERR_INVALID_ARG, + _("weight '%u' must be in range (100, 1000)"), + weight); + return -1; + } - if (stat(path, &sb) < 0) - return -errno; + if (stat(path, &sb) < 0) { + virReportSystemError(errno, + _("Path '%s' is not accessible"), + path); + return -1; + } - if (!S_ISBLK(sb.st_mode)) - return -EINVAL; + if (!S_ISBLK(sb.st_mode)) { + virReportSystemError(EINVAL, + _("Path '%s' must be a block device"), + path); + return -1; + } if (virAsprintf(&str, "%d:%d %d", major(sb.st_rdev), minor(sb.st_rdev), weight) < 0) - return -errno; + return -1; ret = virCgroupSetValueStr(group, VIR_CGROUP_CONTROLLER_BLKIO, @@ -1708,7 +1731,9 @@ virCgroupSetBlkioDeviceWeight(virCgroupPtr group ATTRIBUTE_UNUSED, const char *path ATTRIBUTE_UNUSED, unsigned int weight ATTRIBUTE_UNUSED) { - return -ENOSYS; + virReportSystemError(ENOSYS, "%s", + _("Control groups not supported on this platform")); + return -1; } #endif @@ -1724,9 +1749,14 @@ int virCgroupSetMemory(virCgroupPtr group, unsigned long long kb) { unsigned long long maxkb = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; - if (kb > maxkb) - return -EINVAL; - else if (kb == maxkb) + if (kb > maxkb) { + virReportError(VIR_ERR_INVALID_ARG, + _("Memory '%llu' must be less than %llu"), + kb, maxkb); + return -1; + } + + if (kb == maxkb) return virCgroupSetValueI64(group, VIR_CGROUP_CONTROLLER_MEMORY, "memory.limit_in_bytes", @@ -1803,9 +1833,14 @@ int virCgroupSetMemorySoftLimit(virCgroupPtr group, unsigned long long kb) { unsigned long long maxkb = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; - if (kb > maxkb) - return -EINVAL; - else if (kb == maxkb) + if (kb > maxkb) { + virReportError(VIR_ERR_INVALID_ARG, + _("Memory '%llu' must be less than %llu"), + kb, maxkb); + return -1; + } + + if (kb == maxkb) return virCgroupSetValueI64(group, VIR_CGROUP_CONTROLLER_MEMORY, "memory.soft_limit_in_bytes", @@ -1850,9 +1885,14 @@ int virCgroupSetMemSwapHardLimit(virCgroupPtr group, unsigned long long kb) { unsigned long long maxkb = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; - if (kb > maxkb) - return -EINVAL; - else if (kb == maxkb) + if (kb > maxkb) { + virReportError(VIR_ERR_INVALID_ARG, + _("Memory '%llu' must be less than %llu"), + kb, maxkb); + return -1; + } + + if (kb == maxkb) return virCgroupSetValueI64(group, VIR_CGROUP_CONTROLLER_MEMORY, "memory.memsw.limit_in_bytes", @@ -1997,25 +2037,26 @@ int virCgroupDenyAllDevices(virCgroupPtr group) int virCgroupAllowDevice(virCgroupPtr group, char type, int major, int minor, int perms) { - int rc; + int ret = -1; char *devstr = NULL; if (virAsprintf(&devstr, "%c %i:%i %s%s%s", type, major, minor, perms & VIR_CGROUP_DEVICE_READ ? "r" : "", perms & VIR_CGROUP_DEVICE_WRITE ? "w" : "", - perms & VIR_CGROUP_DEVICE_MKNOD ? "m" : "") == -1) { - rc = -ENOMEM; - goto out; - } + perms & VIR_CGROUP_DEVICE_MKNOD ? "m" : "") < 0) + goto cleanup; - rc = virCgroupSetValueStr(group, - VIR_CGROUP_CONTROLLER_DEVICES, - "devices.allow", - devstr); -out: - VIR_FREE(devstr); + if (virCgroupSetValueStr(group, + VIR_CGROUP_CONTROLLER_DEVICES, + "devices.allow", + devstr) < 0) + goto cleanup; - return rc; + ret = 0; + +cleanup: + VIR_FREE(devstr); + return ret; } /** @@ -2031,25 +2072,26 @@ out: int virCgroupAllowDeviceMajor(virCgroupPtr group, char type, int major, int perms) { - int rc; + int ret = -1; char *devstr = NULL; if (virAsprintf(&devstr, "%c %i:* %s%s%s", type, major, perms & VIR_CGROUP_DEVICE_READ ? "r" : "", perms & VIR_CGROUP_DEVICE_WRITE ? "w" : "", - perms & VIR_CGROUP_DEVICE_MKNOD ? "m" : "") == -1) { - rc = -ENOMEM; - goto out; - } + perms & VIR_CGROUP_DEVICE_MKNOD ? "m" : "") < 0) + goto cleanup; - rc = virCgroupSetValueStr(group, - VIR_CGROUP_CONTROLLER_DEVICES, - "devices.allow", - devstr); - out: - VIR_FREE(devstr); + if (virCgroupSetValueStr(group, + VIR_CGROUP_CONTROLLER_DEVICES, + "devices.allow", + devstr) < 0) + goto cleanup; - return rc; + ret = 0; + +cleanup: + VIR_FREE(devstr); + return ret; } /** @@ -2063,15 +2105,19 @@ int virCgroupAllowDeviceMajor(virCgroupPtr group, char type, int major, * adds that to the cgroup ACL * * Returns: 0 on success, 1 if path exists but is not a device, or - * negative errno value on failure + * -1 on error */ #if defined(major) && defined(minor) int virCgroupAllowDevicePath(virCgroupPtr group, const char *path, int perms) { struct stat sb; - if (stat(path, &sb) < 0) - return -errno; + if (stat(path, &sb) < 0) { + virReportSystemError(errno, + _("Path '%s' is not accessible"), + path); + return -1; + } if (!S_ISCHR(sb.st_mode) && !S_ISBLK(sb.st_mode)) return 1; @@ -2087,7 +2133,9 @@ int virCgroupAllowDevicePath(virCgroupPtr group ATTRIBUTE_UNUSED, const char *path ATTRIBUTE_UNUSED, int perms ATTRIBUTE_UNUSED) { - return -ENOSYS; + virReportSystemError(ENOSYS, "%s", + _("Control groups not supported on this platform")); + return -1; } #endif @@ -2106,25 +2154,26 @@ int virCgroupAllowDevicePath(virCgroupPtr group ATTRIBUTE_UNUSED, int virCgroupDenyDevice(virCgroupPtr group, char type, int major, int minor, int perms) { - int rc; + int ret = -1; char *devstr = NULL; if (virAsprintf(&devstr, "%c %i:%i %s%s%s", type, major, minor, perms & VIR_CGROUP_DEVICE_READ ? "r" : "", perms & VIR_CGROUP_DEVICE_WRITE ? "w" : "", - perms & VIR_CGROUP_DEVICE_MKNOD ? "m" : "") == -1) { - rc = -ENOMEM; - goto out; - } + perms & VIR_CGROUP_DEVICE_MKNOD ? "m" : "") < 0) + goto cleanup; - rc = virCgroupSetValueStr(group, - VIR_CGROUP_CONTROLLER_DEVICES, - "devices.deny", - devstr); -out: - VIR_FREE(devstr); + if (virCgroupSetValueStr(group, + VIR_CGROUP_CONTROLLER_DEVICES, + "devices.deny", + devstr) < 0) + goto cleanup; - return rc; + ret = 0; + +cleanup: + VIR_FREE(devstr); + return ret; } /** @@ -2140,25 +2189,26 @@ out: int virCgroupDenyDeviceMajor(virCgroupPtr group, char type, int major, int perms) { - int rc; + int ret = -1; char *devstr = NULL; if (virAsprintf(&devstr, "%c %i:* %s%s%s", type, major, perms & VIR_CGROUP_DEVICE_READ ? "r" : "", perms & VIR_CGROUP_DEVICE_WRITE ? "w" : "", - perms & VIR_CGROUP_DEVICE_MKNOD ? "m" : "") == -1) { - rc = -ENOMEM; - goto out; - } + perms & VIR_CGROUP_DEVICE_MKNOD ? "m" : "") < 0) + goto cleanup; - rc = virCgroupSetValueStr(group, - VIR_CGROUP_CONTROLLER_DEVICES, - "devices.deny", - devstr); - out: - VIR_FREE(devstr); + if (virCgroupSetValueStr(group, + VIR_CGROUP_CONTROLLER_DEVICES, + "devices.deny", + devstr) < 0) + goto cleanup; - return rc; + ret = 0; + +cleanup: + VIR_FREE(devstr); + return ret; } #if defined(major) && defined(minor) @@ -2166,8 +2216,12 @@ int virCgroupDenyDevicePath(virCgroupPtr group, const char *path, int perms) { struct stat sb; - if (stat(path, &sb) < 0) - return -errno; + if (stat(path, &sb) < 0) { + virReportSystemError(errno, + _("Path '%s' is not accessible"), + path); + return -1; + } if (!S_ISCHR(sb.st_mode) && !S_ISBLK(sb.st_mode)) return 1; @@ -2183,7 +2237,9 @@ int virCgroupDenyDevicePath(virCgroupPtr group ATTRIBUTE_UNUSED, const char *path ATTRIBUTE_UNUSED, int perms ATTRIBUTE_UNUSED) { - return -ENOSYS; + virReportSystemError(ENOSYS, "%s", + _("Control groups not supported on this platform")); + return -1; } #endif @@ -2214,8 +2270,12 @@ int virCgroupSetCpuCfsPeriod(virCgroupPtr group, unsigned long long cfs_period) /* The cfs_period shoule be greater or equal than 1ms, and less or equal * than 1s. */ - if (cfs_period < 1000 || cfs_period > 1000000) - return -EINVAL; + if (cfs_period < 1000 || cfs_period > 1000000) { + virReportError(VIR_ERR_INVALID_ARG, + _("cfs_period '%llu' must be in range (1000, 1000000)"), + cfs_period); + return -1; + } return virCgroupSetValueU64(group, VIR_CGROUP_CONTROLLER_CPU, @@ -2248,14 +2308,14 @@ int virCgroupGetCpuCfsPeriod(virCgroupPtr group, unsigned long long *cfs_period) */ int virCgroupSetCpuCfsQuota(virCgroupPtr group, long long cfs_quota) { - if (cfs_quota >= 0) { - /* The cfs_quota shoule be greater or equal than 1ms */ - if (cfs_quota < 1000) - return -EINVAL; - - /* check overflow */ - if (cfs_quota > ULLONG_MAX / 1000) - return -EINVAL; + /* The cfs_quota should be greater or equal than 1ms */ + if (cfs_quota >= 0 && + (cfs_quota < 1000 || + cfs_quota > ULLONG_MAX / 1000)) { + virReportError(VIR_ERR_INVALID_ARG, + _("cfs_quota '%lld' must be in range (1000, %llu)"), + cfs_quota, ULLONG_MAX / 1000); + return -1; } return virCgroupSetValueI64(group, @@ -2298,17 +2358,25 @@ int virCgroupGetCpuacctStat(virCgroupPtr group, unsigned long long *user, { char *str; char *p; - int ret; + int ret = -1; static double scale = -1.0; - if ((ret = virCgroupGetValueStr(group, VIR_CGROUP_CONTROLLER_CPUACCT, - "cpuacct.stat", &str)) < 0) - return ret; + if (virCgroupGetValueStr(group, VIR_CGROUP_CONTROLLER_CPUACCT, + "cpuacct.stat", &str) < 0) + return -1; + if (!(p = STRSKIP(str, "user ")) || - virStrToLong_ull(p, &p, 10, user) < 0 || - !(p = STRSKIP(p, "\nsystem ")) || + virStrToLong_ull(p, &p, 10, user) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot parse user stat '%s'"), + p); + goto cleanup; + } + if (!(p = STRSKIP(p, "\nsystem ")) || virStrToLong_ull(p, NULL, 10, sys) < 0) { - ret = -EINVAL; + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot parse sys stat '%s'"), + p); goto cleanup; } /* times reported are in system ticks (generally 100 Hz), but that @@ -2317,7 +2385,8 @@ int virCgroupGetCpuacctStat(virCgroupPtr group, unsigned long long *user, if (scale < 0) { long ticks_per_sec = sysconf(_SC_CLK_TCK); if (ticks_per_sec == -1) { - ret = -errno; + virReportSystemError(errno, "%s", + _("Cannot determine system clock HZ")); goto cleanup; } scale = 1000000000.0 / ticks_per_sec; @@ -2335,7 +2404,9 @@ int virCgroupGetCpuacctStat(virCgroupPtr group ATTRIBUTE_UNUSED, unsigned long long *user ATTRIBUTE_UNUSED, unsigned long long *sys ATTRIBUTE_UNUSED) { - return -ENOSYS; + virReportSystemError(ENOSYS, "%s", + _("Control groups not supported on this platform")); + return -1; } #endif @@ -2368,12 +2439,8 @@ static int virCgroupKillInternal(virCgroupPtr group, int signum, virHashTablePtr VIR_DEBUG("group=%p path=%s signum=%d pids=%p", group, group->path, signum, pids); - if (virCgroupPathOfController(group, -1, "tasks", &keypath) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("No tasks file for group path '%s'"), - group->path); + if (virCgroupPathOfController(group, -1, "tasks", &keypath) < 0) return -1; - } /* PIDs may be forking as we kill them, so loop * until there are no new PIDs found @@ -2481,12 +2548,8 @@ static int virCgroupKillRecursiveInternal(virCgroupPtr group, int signum, virHas struct dirent *ent; VIR_DEBUG("group=%p path=%s signum=%d pids=%p", group, group->path, signum, pids); - if (virCgroupPathOfController(group, -1, "", &keypath) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("No tasks file for group path '%s'"), - group->path); + if (virCgroupPathOfController(group, -1, "", &keypath) < 0) return -1; - } if ((rc = virCgroupKillInternal(group, signum, pids)) < 0) return -1; @@ -2619,8 +2682,9 @@ static char *virCgroupIdentifyRoot(virCgroupPtr group) return NULL; } - ignore_value(VIR_STRNDUP_QUIET(ret, group->controllers[i].mountPoint, - tmp - group->controllers[i].mountPoint)); + if (VIR_STRNDUP(ret, group->controllers[i].mountPoint, + tmp - group->controllers[i].mountPoint) < 0) + return NULL; return ret; } @@ -2686,7 +2750,7 @@ int virCgroupIsolateMount(virCgroupPtr group, const char *oldroot, virReportSystemError(errno, _("Failed to bind cgroup '%s' on '%s'"), src, group->controllers[i].mountPoint); - VIR_FREE(src); + VIR_FREE(src); goto cleanup; } @@ -2719,6 +2783,8 @@ int virCgroupIsolateMount(virCgroupPtr group ATTRIBUTE_UNUSED, const char *oldroot ATTRIBUTE_UNUSED, const char *mountopts ATTRIBUTE_UNUSED) { - return -ENOSYS; + virReportSystemError(ENOSYS, "%s", + _("Control groups not supported on this platform")); + return -1; } #endif /* __linux__ */