[libvirt] [PATCHv4 2/3] qemu: Set cpuset.cpus for domain process

[Peter Krempa]

On 07/18/13 14:39, Martin Kletzander wrote:
> On 07/18/2013 12:49 PM, Peter Krempa wrote:
>> From: Osier Yang <jyang at 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