[libvirt] [PATCH 3/3] Convert remainder of cgroups code to report errors

Daniel P. Berrange berrange at redhat.com
Fri Jul 19 17:31:22 UTC 2013


On Fri, Jul 19, 2013 at 09:51:03AM -0600, Eric Blake wrote:
> On 07/19/2013 08:54 AM, Daniel P. Berrange wrote:
> 
> >>>         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.
> >>
> >> Fixed those.
> > 
> > This time with the diff attached.
> 
> > +++ b/src/qemu/qemu_cgroup.c
> 
> Still incomplete - you fixed virCgroupAddTask callers, but didn't audit
> for others.  At least in this file, we have:

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 :|




More information about the libvir-list mailing list