On Fri, May 17, 2013 at 07:59:35PM +0800, Osier Yang wrote:
--- src/qemu/qemu_cgroup.c | 63 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 38 insertions(+), 25 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 8f84ef9..8a2cc9d 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -671,6 +671,35 @@ cleanup: return ret; }
+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; +} +
+ blank line
int qemuInitCgroup(virQEMUDriverPtr driver, virDomainObjPtr vm, bool startup) @@ -784,46 +813,30 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, virDomainObjPtr vm, virBitmapPtr nodemask) { - int rc = -1; qemuDomainObjPrivatePtr priv = vm->privateData;
if (qemuInitCgroup(driver, vm, true) < 0) return -1;
if (!priv->cgroup) - goto done; + return 0;
if (qemuSetupDevicesCgroup(driver, vm) < 0) - goto cleanup; + return -1;
if (qemuSetupBlkioCgroup(vm) < 0) - goto cleanup; + return -1;
if (qemuSetupMemoryCgroup(vm) < 0) - goto cleanup; - - if (vm->def->cputune.shares != 0) { - if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { - 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); - goto cleanup; - } - } else { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("CPU tuning is not available on this host")); - } - } + return -1;
if (qemuSetupCpusetCgroup(vm, nodemask) < 0) - goto cleanup; + return -1;
-done: - rc = 0; -cleanup: - return rc == 0 ? 0 : -1; + if (qemuSetupCpuCgroup(vm) < 0) + return -1; + + return 0; }
I don't think you should be replacing all the 'goto cleanup' lines with 'return -1' here. It is good practice to have a cleanup block, even if not currently used, since historically we've found that we need to add such blocks in more often than not. 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 :|