[libvirt] [PATCH] qemu: Pin the emulator when only cpuset is specified

[Osier Yang]

On 2012年10月19日 11:45, Osier Yang wrote:
> On 2012年10月17日 23:38, Martin Kletzander wrote:
>> According to our recent changes (clarifications), we should be pinning
>> qemu's emulator processes using the<vcpu> 'cpuset' attribute in case
>> there is no<emulatorpin> specified. This however doesn't work
>> entirely as expected and this patch should resolve all the remaining
>> issues.
>> ---
>> src/qemu/qemu_cgroup.c | 25 ++++++++++++++++---------
>> src/qemu/qemu_cgroup.h | 4 ++--
>> src/qemu/qemu_driver.c | 3 ++-
>> src/qemu/qemu_process.c | 16 ++++++++--------
>> 4 files changed, 28 insertions(+), 20 deletions(-)
>>
>> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
>> index 166f9b9..6b94686 100644
>> --- a/src/qemu/qemu_cgroup.c
>> +++ b/src/qemu/qemu_cgroup.c
>> @@ -501,7 +501,7 @@ int qemuSetupCgroupVcpuPin(virCgroupPtr cgroup,
>>
>> for (i = 0; i< nvcpupin; i++) {
>> if (vcpuid == vcpupin[i]->vcpuid) {
>> - return qemuSetupCgroupEmulatorPin(cgroup, vcpupin[i]);
>> + return qemuSetupCgroupEmulatorPin(cgroup, vcpupin[i]->cpumask);
>> }
>> }
>>
>> @@ -509,12 +509,12 @@ int qemuSetupCgroupVcpuPin(virCgroupPtr cgroup,
>> }
>>
>> int qemuSetupCgroupEmulatorPin(virCgroupPtr cgroup,
>> - virDomainVcpuPinDefPtr vcpupin)
>> + virBitmapPtr cpumask)
>> {
>> int rc = 0;
>> char *new_cpus = NULL;
>>
>> - new_cpus = virBitmapFormat(vcpupin->cpumask);
>> + new_cpus = virBitmapFormat(cpumask);
>> if (!new_cpus) {
>> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> _("failed to convert cpu mask"));
>> @@ -643,6 +643,7 @@ cleanup:
>> int qemuSetupCgroupForEmulator(struct qemud_driver *driver,
>> virDomainObjPtr vm)
>> {
>> + virBitmapPtr cpumask = NULL;
>> virCgroupPtr cgroup = NULL;
>> virCgroupPtr cgroup_emulator = NULL;
>> virDomainDefPtr def = vm->def;
>> @@ -690,12 +691,18 @@ int qemuSetupCgroupForEmulator(struct
>> qemud_driver *driver,
>> }
>> }
>>
>> - if (def->cputune.emulatorpin&&
>> - qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUSET)) {
>> - rc = qemuSetupCgroupEmulatorPin(cgroup_emulator,
>> - def->cputune.emulatorpin);
>> - if (rc< 0)
>> - goto cleanup;
>> + if (def->cputune.emulatorpin)
>> + cpumask = def->cputune.emulatorpin->cpumask;
>> + else if (def->cpumask)
>> + cpumask = def->cpumask;
>> +
>> + if (cpumask) {
>> + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUSET)) {
>> + rc = qemuSetupCgroupEmulatorPin(cgroup_emulator, cpumask);
>> + if (rc< 0)
>> + goto cleanup;
>> + }
>> + cpumask = NULL; /* sanity */
>> }
>>
>> if (period || quota) {
>> diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h
>> index b7b0211..362080a 100644
>> --- a/src/qemu/qemu_cgroup.h
>> +++ b/src/qemu/qemu_cgroup.h
>> @@ -1,7 +1,7 @@
>> /*
>> * qemu_cgroup.h: QEMU cgroup management
>> *
>> - * Copyright (C) 2006-2007, 2009-2011 Red Hat, Inc.
>> + * Copyright (C) 2006-2007, 2009-2012 Red Hat, Inc.
>> * Copyright (C) 2006 Daniel P. Berrange
>> *
>> * This library is free software; you can redistribute it and/or
>> @@ -57,7 +57,7 @@ int qemuSetupCgroupVcpuPin(virCgroupPtr cgroup,
>> virDomainVcpuPinDefPtr *vcpupin,
>> int nvcpupin,
>> int vcpuid);
>> -int qemuSetupCgroupEmulatorPin(virCgroupPtr cgroup,
>> virDomainVcpuPinDefPtr vcpupin);
>> +int qemuSetupCgroupEmulatorPin(virCgroupPtr cgroup, virBitmapPtr
>> cpumask);
>> int qemuSetupCgroupForVcpu(struct qemud_driver *driver,
>> virDomainObjPtr vm);
>> int qemuSetupCgroupForEmulator(struct qemud_driver *driver,
>> virDomainObjPtr vm);
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index fa37bfd..adfbfa6 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -4245,7 +4245,8 @@ qemudDomainPinEmulator(virDomainPtr dom,
>> if (virCgroupForDomain(driver->cgroup, vm->def->name,
>> &cgroup_dom, 0) == 0) {
>> if (virCgroupForEmulator(cgroup_dom,&cgroup_emulator, 0) == 0) {
>> - if (qemuSetupCgroupEmulatorPin(cgroup_emulator, newVcpuPin[0])< 0) {
>> + if (qemuSetupCgroupEmulatorPin(cgroup_emulator,
>> + newVcpuPin[0]->cpumask)< 0) {
>> virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>> _("failed to set cpuset.cpus in cgroup"
>> " for emulator threads"));
>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>> index a94e9c4..e08ec67 100644
>> --- a/src/qemu/qemu_process.c
>> +++ b/src/qemu/qemu_process.c
>> @@ -2024,11 +2024,12 @@ cleanup:
>> return ret;
>> }
>>
>> -/* Set CPU affinities for emulator threads if emulatorpin xml
>> provided. */
>> +/* Set CPU affinities for emulator threads. */
>> static int
>> qemuProcessSetEmulatorAffinites(virConnectPtr conn,
>> virDomainObjPtr vm)
>> {
>> + virBitmapPtr cpumask;
>> virDomainDefPtr def = vm->def;
>> virNodeInfo nodeinfo;
>> int ret = -1;
>> @@ -2036,15 +2037,14 @@ qemuProcessSetEmulatorAffinites(virConnectPtr
>> conn,
>> if (virNodeGetInfo(conn,&nodeinfo) != 0)
>> return -1;
>>
>> - if (!def->cputune.emulatorpin)
>> - return 0;
>> -
>> - if (virProcessInfoSetAffinity(vm->pid,
>> - def->cputune.emulatorpin->cpumask)< 0) {
>> + if (def->cputune.emulatorpin)
>> + cpumask = def->cputune.emulatorpin->cpumask;
>> + else if (def->cpumask)
>> + cpumask = def->cpumask;
>> + else
>> goto cleanup;
>> - }
>>
>> - ret = 0;
>> + ret = virProcessInfoSetAffinity(vm->pid, cpumask);
>> cleanup:
>> return ret;
>> }
>
> This patch actually duplicates the affinity setting on domain
> process with sched_setaffinity.
>
> Assume "cpuset" of <vcpu> is specified, and no "<emulatorpin>".
>
> qemuProcessInitCpuAffinity is excuted between the fork
> and exec, with this patch, qemuProcessSetEmulatorAffinites
> will set the affinity again using sched_setaffinity.
>
> Cgroup setting is fine though, as there is no previous setting
> with cgroup.
>
> I'm wondering why the domain process is not pinned to
> pCPUs specified "cpuset" of "<vcpu>" by qemuProcessInitCpuAffinity,
> when <emulatorpin> is not specified.
>
> Regards,
> Osier
>

Just realized this patch actually introduced regression,

Assumes XML like:

<vcpu placement="auto">10</vcpu>

Which expects the domain process is pinned to the nodeset
returned from numad, however, the qemuProcessSetEmulatorAffinites
will override the previous pinning with all available pCPUs.

Because:

 >> + if (def->cputune.emulatorpin)
 >> + cpumask = def->cputune.emulatorpin->cpumask;
 >> + else if (def->cpumask)
 >> + cpumask = def->cpumask;

Regards,
Osier