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

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

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