On 07/18/13 14:39, Martin Kletzander wrote:
On 07/18/2013 12:49 PM, Peter Krempa wrote:
From: Osier Yang <jyang@redhat.com>
When either "cpuset" of <vcpu> is specified, or the "placement" of <vcpu> is "auto", only setting the cpuset.mems might cause the guest starting to fail. E.g. ("placement" of both <vcpu> and <numatune> is "auto"):
[...]
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index fbe7e6f..0ccbff9 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c [...] @@ -652,9 +654,37 @@ qemuSetupCpusetCgroup(virDomainObjPtr vm, } }
+ if (vm->def->cpumask || + (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO)) { + + if (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) { + virBitmapPtr cpumap = virCapabilitiesGetCpusForNodemask(caps, nodemask);
You don't check for error after this line and then ...
+ cpu_mask = virBitmapFormat(cpumap); + virBitmapFree(cpumap); + } else { + cpu_mask = virBitmapFormat(vm->def->cpumask); + } + + if (!cpu_mask) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to convert memory nodemask"));
... rewrite it here in case it fails. With an error about some fancy "memory nodemask" of yours (copy-paste error) ;-)
ACK with these two nits fixed.
I noticed that the last hunk of the patch would actually leak the caps in case of cgroups fail. I will be pushing this with the following patch squashed in: diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 9d2f757..4bebd31 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -836,15 +836,15 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, virCapsPtr caps = NULL; int ret = -1; - if (!(caps = virQEMUDriverGetCapabilities(driver, false))) - goto cleanup; - if (qemuInitCgroup(driver, vm, true) < 0) return -1; if (!priv->cgroup) return 0; + if (!(caps = virQEMUDriverGetCapabilities(driver, false))) + goto cleanup; + if (qemuSetupDevicesCgroup(driver, vm) < 0) goto cleanup;
Martin
Peter