Re: [libvirt] [PATCH 1/3] qemu: Abstract code for the cpu controller setting into a helper

On 05/24/2013 11:08 AM, Osier Yang wrote: > --- > src/qemu/qemu_cgroup.c | 55 +++++++++++++++++++++++++++++++++----------------- > 1 file changed, 36 insertions(+), 19 deletions(-) > > diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c > index bb1b235..cf46993 100644 > --- a/src/qemu/qemu_cgroup.c > +++ b/src/qemu/qemu_cgroup.c > @@ -676,6 +676,36 @@ cleanup: > } > > > +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) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("CPU tuning is not available on this host")); > + return -1; > + } else { > + return 0; > + } > + } > + > + 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; > + } > + } > + > + return 0; > +} > + > + I think this would be a bit more readable: if (!vm->def->cputune.shares) return 0; if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { virReportError(...); return -1; } rc = virCgroupSetCpuShares(priv->cgroup, vm->def->cputune.shares); if (rc != 0) { ... but that's a matter of opinion. ACK either way.