[libvirt] [PATCH v2 0/5] Refactor cgroup and qemuDomainHotplugVcpus

v1 here: http://www.redhat.com/archives/libvir-list/2015-April/msg00126.html John Ferlan (5): cgroup: Introduce virCgroupNewThread cgroup: Use virCgroupNewThread qemu: qemuDomainHotplugVcpus - separate out the add cgroup qemu: qemuDomainHotplugVcpus - separate out the del cgroup and pin qemu: qemuDomainHotplugVcpus - separate out pin adjustment code src/libvirt_private.syms | 4 +- src/qemu/qemu_cgroup.c | 22 +++-- src/qemu/qemu_driver.c | 233 ++++++++++++++++++++++++++++++----------------- src/util/vircgroup.c | 144 +++++++---------------------- src/util/vircgroup.h | 30 +++--- 5 files changed, 208 insertions(+), 225 deletions(-) -- 2.1.0

Create a new common API to replace the virCgroupNew{Vcpu|Emulator|IOThread} API's using an emum to generate the cgroup name Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libvirt_private.syms | 1 + src/util/vircgroup.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/vircgroup.h | 15 ++++++++++ 3 files changed, 90 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9f82926..0800cb6 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1186,6 +1186,7 @@ virCgroupNewIOThread; virCgroupNewMachine; virCgroupNewPartition; virCgroupNewSelf; +virCgroupNewThread; virCgroupNewVcpu; virCgroupPathOfController; virCgroupRemove; diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index d42f433..7fec0cc 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1422,6 +1422,67 @@ virCgroupNewDomainPartition(virCgroupPtr partition, /** + * virCgroupNewThread: + * + * @domain: group for the domain + * @name: enum to generate the name for the new thread + * @id: id of the vcpu or iothread + * @create: true to create if not already existing + * @group: Pointer to returned virCgroupPtr + * + * Returns 0 on success, or -1 on error + */ +int +virCgroupNewThread(virCgroupPtr domain, + virCgroupThreadName nameval, + int id, + bool create, + virCgroupPtr *group) +{ + int ret = -1; + char *name = NULL; + int controllers; + + switch (nameval) { + case VIR_CGROUP_VCPU_NAME: + if (virAsprintf(&name, "vcpu%d", id) < 0) + goto cleanup; + break; + case VIR_CGROUP_EMULATOR_NAME: + if (VIR_STRDUP(name, "emulator") < 0) + goto cleanup; + break; + case VIR_CGROUP_IOTHREAD_NAME: + if (virAsprintf(&name, "iothread%d", id) < 0) + goto cleanup; + break; + case VIR_CGROUP_NAME_LAST: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected name value %d"), nameval); + goto cleanup; + } + + controllers = ((1 << VIR_CGROUP_CONTROLLER_CPU) | + (1 << VIR_CGROUP_CONTROLLER_CPUACCT) | + (1 << VIR_CGROUP_CONTROLLER_CPUSET)); + + if (virCgroupNew(-1, name, domain, controllers, group) < 0) + goto cleanup; + + if (virCgroupMakeGroup(domain, *group, create, VIR_CGROUP_NONE) < 0) { + virCgroupRemove(*group); + virCgroupFree(group); + goto cleanup; + } + + ret = 0; + cleanup: + VIR_FREE(name); + return ret; +} + + +/** * virCgroupNewVcpu: * * @domain: group for the domain @@ -4066,6 +4127,19 @@ virCgroupNewDomainPartition(virCgroupPtr partition ATTRIBUTE_UNUSED, int +virCgroupNewThread(virCgroupPtr domain ATTRIBUTE_UNUSED, + virCgroupThreadName nameval ATTRIBUTE_UNUSED, + int id ATTRIBUTE_UNUSED, + bool create ATTRIBUTE_UNUSED, + virCgroupPtr *group ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENXIO, "%s", + _("Control groups not supported on this platform")); + return -1; +} + + +int virCgroupNewVcpu(virCgroupPtr domain ATTRIBUTE_UNUSED, int vcpuid ATTRIBUTE_UNUSED, bool create ATTRIBUTE_UNUSED, diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index eee15ca..a756b03 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -52,6 +52,14 @@ VIR_ENUM_DECL(virCgroupController); * Make sure we will not overflow */ verify(VIR_CGROUP_CONTROLLER_LAST < 8 * sizeof(int)); +typedef enum { + VIR_CGROUP_VCPU_NAME = 0, + VIR_CGROUP_EMULATOR_NAME, + VIR_CGROUP_IOTHREAD_NAME, + + VIR_CGROUP_NAME_LAST +} virCgroupThreadName; + bool virCgroupAvailable(void); int virCgroupNewPartition(const char *path, @@ -70,6 +78,13 @@ int virCgroupNewDomainPartition(virCgroupPtr partition, virCgroupPtr *group) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5); +int virCgroupNewThread(virCgroupPtr domain, + virCgroupThreadName nameval, + int id, + bool create, + virCgroupPtr *group) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(5); + int virCgroupNewVcpu(virCgroupPtr domain, int vcpuid, bool create, -- 2.1.0

On Tue, Apr 07, 2015 at 09:23:13AM -0400, John Ferlan wrote:
Create a new common API to replace the virCgroupNew{Vcpu|Emulator|IOThread} API's using an emum to generate the cgroup name
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libvirt_private.syms | 1 + src/util/vircgroup.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/vircgroup.h | 15 ++++++++++ 3 files changed, 90 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9f82926..0800cb6 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1186,6 +1186,7 @@ virCgroupNewIOThread; virCgroupNewMachine; virCgroupNewPartition; virCgroupNewSelf; +virCgroupNewThread; virCgroupNewVcpu; virCgroupPathOfController; virCgroupRemove; diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index d42f433..7fec0cc 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1422,6 +1422,67 @@ virCgroupNewDomainPartition(virCgroupPtr partition,
/** + * virCgroupNewThread: + * + * @domain: group for the domain + * @name: enum to generate the name for the new thread + * @id: id of the vcpu or iothread + * @create: true to create if not already existing + * @group: Pointer to returned virCgroupPtr + * + * Returns 0 on success, or -1 on error + */ +int +virCgroupNewThread(virCgroupPtr domain, + virCgroupThreadName nameval, + int id, + bool create, + virCgroupPtr *group) +{ + int ret = -1; + char *name = NULL; + int controllers; + + switch (nameval) { + case VIR_CGROUP_VCPU_NAME: + if (virAsprintf(&name, "vcpu%d", id) < 0) + goto cleanup; + break; + case VIR_CGROUP_EMULATOR_NAME: + if (VIR_STRDUP(name, "emulator") < 0) + goto cleanup; + break; + case VIR_CGROUP_IOTHREAD_NAME: + if (virAsprintf(&name, "iothread%d", id) < 0) + goto cleanup; + break; + case VIR_CGROUP_NAME_LAST: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected name value %d"), nameval); + goto cleanup; + } + + controllers = ((1 << VIR_CGROUP_CONTROLLER_CPU) | + (1 << VIR_CGROUP_CONTROLLER_CPUACCT) | + (1 << VIR_CGROUP_CONTROLLER_CPUSET)); + + if (virCgroupNew(-1, name, domain, controllers, group) < 0) + goto cleanup; + + if (virCgroupMakeGroup(domain, *group, create, VIR_CGROUP_NONE) < 0) { + virCgroupRemove(*group); + virCgroupFree(group); + goto cleanup; + } + + ret = 0; + cleanup: + VIR_FREE(name); + return ret; +} + + +/** * virCgroupNewVcpu: * * @domain: group for the domain @@ -4066,6 +4127,19 @@ virCgroupNewDomainPartition(virCgroupPtr partition ATTRIBUTE_UNUSED,
int +virCgroupNewThread(virCgroupPtr domain ATTRIBUTE_UNUSED, + virCgroupThreadName nameval ATTRIBUTE_UNUSED, + int id ATTRIBUTE_UNUSED, + bool create ATTRIBUTE_UNUSED, + virCgroupPtr *group ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENXIO, "%s", + _("Control groups not supported on this platform")); + return -1; +} + + +int virCgroupNewVcpu(virCgroupPtr domain ATTRIBUTE_UNUSED, int vcpuid ATTRIBUTE_UNUSED, bool create ATTRIBUTE_UNUSED, diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index eee15ca..a756b03 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -52,6 +52,14 @@ VIR_ENUM_DECL(virCgroupController); * Make sure we will not overflow */ verify(VIR_CGROUP_CONTROLLER_LAST < 8 * sizeof(int));
+typedef enum { + VIR_CGROUP_VCPU_NAME = 0, + VIR_CGROUP_EMULATOR_NAME, + VIR_CGROUP_IOTHREAD_NAME, + + VIR_CGROUP_NAME_LAST +} virCgroupThreadName; +
The enum name does not match the prefix of its values and even the prefix is not the same. ACK if you change it to satisfy both conditions, e.g.: typedef enum { VIR_CGROUP_THREAD_VCPU VIR_CGROUP_THREAD_EMULATOR VIR_CGROUP_THREAD_IOTHREAD VIR_CGROUP_THREAD_LAST } virCgroupThread; Jan

Replace the virCgroupNew{Vcpu|Emulator|IOThread} calls with the common virCgroupNewThread API Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libvirt_private.syms | 3 - src/qemu/qemu_cgroup.c | 22 ++++--- src/qemu/qemu_driver.c | 41 +++++++----- src/util/vircgroup.c | 160 +---------------------------------------------- src/util/vircgroup.h | 17 ----- 5 files changed, 42 insertions(+), 201 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0800cb6..12614cb 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1180,14 +1180,11 @@ virCgroupMoveTask; virCgroupNewDetect; virCgroupNewDetectMachine; virCgroupNewDomainPartition; -virCgroupNewEmulator; virCgroupNewIgnoreError; -virCgroupNewIOThread; virCgroupNewMachine; virCgroupNewPartition; virCgroupNewSelf; virCgroupNewThread; -virCgroupNewVcpu; virCgroupPathOfController; virCgroupRemove; virCgroupRemoveRecursively; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 50546a1..f287d24 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -629,7 +629,8 @@ qemuSetupCpusetMems(virDomainObjPtr vm) goto cleanup; if (mem_mask) - if (virCgroupNewEmulator(priv->cgroup, false, &cgroup_temp) < 0 || + if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_EMULATOR_NAME, 0, + false, &cgroup_temp) < 0 || virCgroupSetCpusetMems(cgroup_temp, mem_mask) < 0) goto cleanup; @@ -790,7 +791,8 @@ qemuRestoreCgroupState(virDomainObjPtr vm) goto error; for (i = 0; i < priv->nvcpupids; i++) { - if (virCgroupNewVcpu(priv->cgroup, i, false, &cgroup_temp) < 0 || + if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_VCPU_NAME, i, + false, &cgroup_temp) < 0 || virCgroupSetCpusetMemoryMigrate(cgroup_temp, true) < 0 || virCgroupGetCpusetMems(cgroup_temp, &nodeset) < 0 || virCgroupSetCpusetMems(cgroup_temp, nodeset) < 0) @@ -800,7 +802,8 @@ qemuRestoreCgroupState(virDomainObjPtr vm) } for (i = 0; i < priv->niothreadpids; i++) { - if (virCgroupNewIOThread(priv->cgroup, i + 1, false, &cgroup_temp) < 0 || + if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_IOTHREAD_NAME, i + 1, + false, &cgroup_temp) < 0 || virCgroupSetCpusetMemoryMigrate(cgroup_temp, true) < 0 || virCgroupGetCpusetMems(cgroup_temp, &nodeset) < 0 || virCgroupSetCpusetMems(cgroup_temp, nodeset) < 0) @@ -809,7 +812,8 @@ qemuRestoreCgroupState(virDomainObjPtr vm) virCgroupFree(&cgroup_temp); } - if (virCgroupNewEmulator(priv->cgroup, false, &cgroup_temp) < 0 || + if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_EMULATOR_NAME, 0, + false, &cgroup_temp) < 0 || virCgroupSetCpusetMemoryMigrate(cgroup_temp, true) < 0 || virCgroupGetCpusetMems(cgroup_temp, &nodeset) < 0 || virCgroupSetCpusetMems(cgroup_temp, nodeset) < 0) @@ -1010,7 +1014,8 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm) goto cleanup; for (i = 0; i < priv->nvcpupids; i++) { - if (virCgroupNewVcpu(priv->cgroup, i, true, &cgroup_vcpu) < 0) + if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_VCPU_NAME, i, + true, &cgroup_vcpu) < 0) goto cleanup; /* move the thread for vcpu to sub dir */ @@ -1096,7 +1101,8 @@ qemuSetupCgroupForEmulator(virDomainObjPtr vm) if (priv->cgroup == NULL) return 0; /* Not supported, so claim success */ - if (virCgroupNewEmulator(priv->cgroup, true, &cgroup_emulator) < 0) + if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_EMULATOR_NAME, 0, + true, &cgroup_emulator) < 0) goto cleanup; if (virCgroupMoveTask(priv->cgroup, cgroup_emulator) < 0) @@ -1183,8 +1189,8 @@ qemuSetupCgroupForIOThreads(virDomainObjPtr vm) /* IOThreads are numbered 1..n, although the array is 0..n-1, * so we will account for that here */ - if (virCgroupNewIOThread(priv->cgroup, i + 1, true, - &cgroup_iothread) < 0) + if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_IOTHREAD_NAME, i + 1, + true, &cgroup_iothread) < 0) goto cleanup; /* move the thread for iothread to sub dir */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6132674..be54b7e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4728,7 +4728,8 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, if (priv->cgroup) { int rv = -1; /* Create cgroup for the onlined vcpu */ - if (virCgroupNewVcpu(priv->cgroup, i, true, &cgroup_vcpu) < 0) + if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_VCPU_NAME, i, + true, &cgroup_vcpu) < 0) goto cleanup; if (mem_mask && @@ -4801,7 +4802,8 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, } else { for (i = oldvcpus - 1; i >= nvcpus; i--) { if (priv->cgroup) { - if (virCgroupNewVcpu(priv->cgroup, i, false, &cgroup_vcpu) < 0) + if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_VCPU_NAME, i, + false, &cgroup_vcpu) < 0) goto cleanup; /* Remove cgroup for the offlined vcpu */ @@ -4894,7 +4896,8 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, if (flags & VIR_DOMAIN_AFFECT_LIVE && !(flags & VIR_DOMAIN_VCPU_GUEST) && virNumaIsAvailable()) { - if (virCgroupNewEmulator(priv->cgroup, false, &cgroup_temp) < 0) + if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_EMULATOR_NAME, 0, + false, &cgroup_temp) < 0) goto endjob; if (!(all_nodes = virNumaGetHostNodeset())) @@ -5136,7 +5139,8 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, /* Configure the corresponding cpuset cgroup before set affinity. */ if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) { - if (virCgroupNewVcpu(priv->cgroup, vcpu, false, &cgroup_vcpu) < 0) + if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_VCPU_NAME, vcpu, + false, &cgroup_vcpu) < 0) goto endjob; if (qemuSetupCgroupCpusetCpus(cgroup_vcpu, pcpumap) < 0) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -5419,7 +5423,8 @@ qemuDomainPinEmulator(virDomainPtr dom, /* * Configure the corresponding cpuset cgroup. */ - if (virCgroupNewEmulator(priv->cgroup, false, &cgroup_emulator) < 0) + if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_EMULATOR_NAME, + 0, false, &cgroup_emulator) < 0) goto endjob; if (qemuSetupCgroupCpusetCpus(cgroup_emulator, newVcpuPin[0]->cpumask) < 0) { @@ -6014,8 +6019,8 @@ qemuDomainPinIOThread(virDomainPtr dom, /* Configure the corresponding cpuset cgroup before set affinity. */ if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) { - if (virCgroupNewIOThread(priv->cgroup, iothread_id, - false, &cgroup_iothread) < 0) + if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_IOTHREAD_NAME, + iothread_id, false, &cgroup_iothread) < 0) goto endjob; if (qemuSetupCgroupCpusetCpus(cgroup_iothread, pcpumap) < 0) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -9956,21 +9961,23 @@ qemuDomainSetNumaParamsLive(virDomainObjPtr vm, if (!(nodeset_str = virBitmapFormat(nodeset))) goto cleanup; - if (virCgroupNewEmulator(priv->cgroup, false, &cgroup_temp) < 0 || + if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_EMULATOR_NAME, 0, + false, &cgroup_temp) < 0 || virCgroupSetCpusetMems(cgroup_temp, nodeset_str) < 0) goto cleanup; virCgroupFree(&cgroup_temp); for (i = 0; i < priv->nvcpupids; i++) { - if (virCgroupNewVcpu(priv->cgroup, i, false, &cgroup_temp) < 0 || + if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_VCPU_NAME, i, + false, &cgroup_temp) < 0 || virCgroupSetCpusetMems(cgroup_temp, nodeset_str) < 0) goto cleanup; virCgroupFree(&cgroup_temp); } for (i = 0; i < priv->niothreadpids; i++) { - if (virCgroupNewIOThread(priv->cgroup, i + 1, false, - &cgroup_temp) < 0 || + if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_IOTHREAD_NAME, i + 1, + false, &cgroup_temp) < 0 || virCgroupSetCpusetMems(cgroup_temp, nodeset_str) < 0) goto cleanup; virCgroupFree(&cgroup_temp); @@ -10237,7 +10244,8 @@ qemuSetVcpusBWLive(virDomainObjPtr vm, virCgroupPtr cgroup, */ if (priv->nvcpupids != 0 && priv->vcpupids[0] != vm->pid) { for (i = 0; i < priv->nvcpupids; i++) { - if (virCgroupNewVcpu(cgroup, i, false, &cgroup_vcpu) < 0) + if (virCgroupNewThread(cgroup, VIR_CGROUP_VCPU_NAME, i, + false, &cgroup_vcpu) < 0) goto cleanup; if (qemuSetupCgroupVcpuBW(cgroup_vcpu, period, quota) < 0) @@ -10267,7 +10275,8 @@ qemuSetEmulatorBandwidthLive(virDomainObjPtr vm, virCgroupPtr cgroup, if (priv->nvcpupids == 0 || priv->vcpupids[0] == vm->pid) return 0; - if (virCgroupNewEmulator(cgroup, false, &cgroup_emulator) < 0) + if (virCgroupNewThread(cgroup, VIR_CGROUP_EMULATOR_NAME, 0, + false, &cgroup_emulator) < 0) goto cleanup; if (qemuSetupCgroupVcpuBW(cgroup_emulator, period, quota) < 0) @@ -10562,7 +10571,8 @@ qemuGetVcpusBWLive(virDomainObjPtr vm, } /* get period and quota for vcpu0 */ - if (virCgroupNewVcpu(priv->cgroup, 0, false, &cgroup_vcpu) < 0) + if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_VCPU_NAME, 0, + false, &cgroup_vcpu) < 0) goto cleanup; rc = qemuGetVcpuBWLive(cgroup_vcpu, period, quota); @@ -10595,7 +10605,8 @@ qemuGetEmulatorBandwidthLive(virDomainObjPtr vm, virCgroupPtr cgroup, } /* get period and quota for emulator */ - if (virCgroupNewEmulator(cgroup, false, &cgroup_emulator) < 0) + if (virCgroupNewThread(cgroup, VIR_CGROUP_EMULATOR_NAME, 0, + false, &cgroup_emulator) < 0) goto cleanup; rc = qemuGetVcpuBWLive(cgroup_emulator, period, quota); diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 7fec0cc..f1665c8 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1482,128 +1482,6 @@ virCgroupNewThread(virCgroupPtr domain, } -/** - * virCgroupNewVcpu: - * - * @domain: group for the domain - * @vcpuid: id of the vcpu - * @create: true to create if not already existing - * @group: Pointer to returned virCgroupPtr - * - * Returns 0 on success, or -1 on error - */ -int -virCgroupNewVcpu(virCgroupPtr domain, - int vcpuid, - bool create, - virCgroupPtr *group) -{ - int ret = -1; - char *name = NULL; - int controllers; - - if (virAsprintf(&name, "vcpu%d", vcpuid) < 0) - goto cleanup; - - controllers = ((1 << VIR_CGROUP_CONTROLLER_CPU) | - (1 << VIR_CGROUP_CONTROLLER_CPUACCT) | - (1 << VIR_CGROUP_CONTROLLER_CPUSET)); - - if (virCgroupNew(-1, name, domain, controllers, group) < 0) - goto cleanup; - - if (virCgroupMakeGroup(domain, *group, create, VIR_CGROUP_NONE) < 0) { - virCgroupRemove(*group); - virCgroupFree(group); - goto cleanup; - } - - ret = 0; - cleanup: - VIR_FREE(name); - return ret; -} - - -/** - * virCgroupNewEmulator: - * - * @domain: group for the domain - * @create: true to create if not already existing - * @group: Pointer to returned virCgroupPtr - * - * Returns: 0 on success or -1 on error - */ -int -virCgroupNewEmulator(virCgroupPtr domain, - bool create, - virCgroupPtr *group) -{ - int ret = -1; - int controllers; - - controllers = ((1 << VIR_CGROUP_CONTROLLER_CPU) | - (1 << VIR_CGROUP_CONTROLLER_CPUACCT) | - (1 << VIR_CGROUP_CONTROLLER_CPUSET)); - - if (virCgroupNew(-1, "emulator", domain, controllers, group) < 0) - goto cleanup; - - if (virCgroupMakeGroup(domain, *group, create, VIR_CGROUP_NONE) < 0) { - virCgroupRemove(*group); - virCgroupFree(group); - goto cleanup; - } - - ret = 0; - cleanup: - return ret; -} - - -/** - * virCgroupNewIOThread: - * - * @domain: group for the domain - * @iothreadid: id of the iothread - * @create: true to create if not already existing - * @group: Pointer to returned virCgroupPtr - * - * Returns 0 on success, or -1 on error - */ -int -virCgroupNewIOThread(virCgroupPtr domain, - int iothreadid, - bool create, - virCgroupPtr *group) -{ - int ret = -1; - char *name = NULL; - int controllers; - - if (virAsprintf(&name, "iothread%d", iothreadid) < 0) - goto cleanup; - - controllers = ((1 << VIR_CGROUP_CONTROLLER_CPU) | - (1 << VIR_CGROUP_CONTROLLER_CPUACCT) | - (1 << VIR_CGROUP_CONTROLLER_CPUSET)); - - if (virCgroupNew(-1, name, domain, controllers, group) < 0) - goto cleanup; - - if (virCgroupMakeGroup(domain, *group, create, VIR_CGROUP_NONE) < 0) { - virCgroupRemove(*group); - virCgroupFree(group); - goto cleanup; - } - - ret = 0; - cleanup: - VIR_FREE(name); - return ret; -} - - int virCgroupNewDetect(pid_t pid, int controllers, @@ -3106,7 +2984,8 @@ virCgroupGetPercpuVcpuSum(virCgroupPtr group, unsigned long long tmp; ssize_t j; - if (virCgroupNewVcpu(group, i, false, &group_vcpu) < 0) + if (virCgroupNewThread(group, VIR_CGROUP_VCPU_NAME, i, + false, &group_vcpu) < 0) goto cleanup; if (virCgroupGetCpuacctPercpuUsage(group_vcpu, &buf) < 0) @@ -4140,41 +4019,6 @@ virCgroupNewThread(virCgroupPtr domain ATTRIBUTE_UNUSED, int -virCgroupNewVcpu(virCgroupPtr domain ATTRIBUTE_UNUSED, - int vcpuid ATTRIBUTE_UNUSED, - bool create ATTRIBUTE_UNUSED, - virCgroupPtr *group ATTRIBUTE_UNUSED) -{ - virReportSystemError(ENXIO, "%s", - _("Control groups not supported on this platform")); - return -1; -} - - -int -virCgroupNewEmulator(virCgroupPtr domain ATTRIBUTE_UNUSED, - bool create ATTRIBUTE_UNUSED, - virCgroupPtr *group ATTRIBUTE_UNUSED) -{ - virReportSystemError(ENXIO, "%s", - _("Control groups not supported on this platform")); - return -1; -} - - -int -virCgroupNewIOThread(virCgroupPtr domain ATTRIBUTE_UNUSED, - int iothreadid ATTRIBUTE_UNUSED, - bool create ATTRIBUTE_UNUSED, - virCgroupPtr *group ATTRIBUTE_UNUSED) -{ - virReportSystemError(ENXIO, "%s", - _("Control groups not supported on this platform")); - return -1; -} - - -int virCgroupNewDetect(pid_t pid ATTRIBUTE_UNUSED, int controllers ATTRIBUTE_UNUSED, virCgroupPtr *group ATTRIBUTE_UNUSED) diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index a756b03..cf90e0b 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -85,23 +85,6 @@ int virCgroupNewThread(virCgroupPtr domain, virCgroupPtr *group) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(5); -int virCgroupNewVcpu(virCgroupPtr domain, - int vcpuid, - bool create, - virCgroupPtr *group) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4); - -int virCgroupNewEmulator(virCgroupPtr domain, - bool create, - virCgroupPtr *group) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3); - -int virCgroupNewIOThread(virCgroupPtr domain, - int iothreadid, - bool create, - virCgroupPtr *group) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4); - int virCgroupNewDetect(pid_t pid, int controllers, virCgroupPtr *group); -- 2.1.0

On Tue, Apr 07, 2015 at 09:23:14AM -0400, John Ferlan wrote:
Replace the virCgroupNew{Vcpu|Emulator|IOThread} calls with the common virCgroupNewThread API
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libvirt_private.syms | 3 - src/qemu/qemu_cgroup.c | 22 ++++--- src/qemu/qemu_driver.c | 41 +++++++----- src/util/vircgroup.c | 160 +---------------------------------------------- src/util/vircgroup.h | 17 ----- 5 files changed, 42 insertions(+), 201 deletions(-)
ACK (with the enum values changed to match patch 1) Jan

Future IOThread setting patches would copy the code anyway, so create and generalize the add the vcpu to a cgroup into its own API. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 65 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 43 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index be54b7e..1114ce9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4635,9 +4635,44 @@ static void qemuProcessEventHandler(void *data, void *opaque) VIR_FREE(processEvent); } -static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, - virDomainObjPtr vm, - unsigned int nvcpus) +static virCgroupPtr +qemuDomainHotplugAddCgroup(virCgroupPtr cgroup, + virCgroupThreadName nameval, + int index, + char *mem_mask, + pid_t pid) +{ + virCgroupPtr new_cgroup = NULL; + int rv = -1; + + /* Create cgroup */ + if (virCgroupNewThread(cgroup, nameval, index, true, &new_cgroup) < 0) + return NULL; + + if (mem_mask && virCgroupSetCpusetMems(new_cgroup, mem_mask) < 0) + goto error; + + /* Add pid/thread to the cgroup */ + rv = virCgroupAddTask(new_cgroup, pid); + if (rv < 0) { + virReportSystemError(-rv, + _("unable to add id %d task %d to cgroup"), + index, pid); + virCgroupRemove(new_cgroup); + goto error; + } + + return new_cgroup; + + error: + virCgroupFree(&new_cgroup); + return NULL; +} + +static int +qemuDomainHotplugVcpus(virQEMUDriverPtr driver, + virDomainObjPtr vm, + unsigned int nvcpus) { qemuDomainObjPrivatePtr priv = vm->privateData; size_t i; @@ -4726,26 +4761,12 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, if (nvcpus > oldvcpus) { for (i = oldvcpus; i < nvcpus; i++) { if (priv->cgroup) { - int rv = -1; - /* Create cgroup for the onlined vcpu */ - if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_VCPU_NAME, i, - true, &cgroup_vcpu) < 0) + cgroup_vcpu = qemuDomainHotplugAddCgroup(priv->cgroup, + VIR_CGROUP_VCPU_NAME, + i, mem_mask, + cpupids[i]); + if (!cgroup_vcpu) goto cleanup; - - if (mem_mask && - virCgroupSetCpusetMems(cgroup_vcpu, mem_mask) < 0) - goto cleanup; - - /* Add vcpu thread to the cgroup */ - rv = virCgroupAddTask(cgroup_vcpu, cpupids[i]); - if (rv < 0) { - virReportSystemError(-rv, - _("unable to add vcpu %zu task %d to cgroup"), - i, cpupids[i]); - virCgroupRemove(cgroup_vcpu); - goto cleanup; - } - } /* Inherit def->cpuset */ -- 2.1.0

On Tue, Apr 07, 2015 at 09:23:15AM -0400, John Ferlan wrote:
Future IOThread setting patches would copy the code anyway, so create and generalize the add the vcpu to a cgroup into its own API.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 65 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 43 insertions(+), 22 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index be54b7e..1114ce9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4635,9 +4635,44 @@ static void qemuProcessEventHandler(void *data, void *opaque) VIR_FREE(processEvent); }
-static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, - virDomainObjPtr vm, - unsigned int nvcpus) +static virCgroupPtr +qemuDomainHotplugAddCgroup(virCgroupPtr cgroup,
qemuDomainAddCgroupForThread would be better IMHO.
+ virCgroupThreadName nameval, + int index, + char *mem_mask, + pid_t pid) +{ + virCgroupPtr new_cgroup = NULL; + int rv = -1; + + /* Create cgroup */ + if (virCgroupNewThread(cgroup, nameval, index, true, &new_cgroup) < 0) + return NULL; + + if (mem_mask && virCgroupSetCpusetMems(new_cgroup, mem_mask) < 0) + goto error; + + /* Add pid/thread to the cgroup */ + rv = virCgroupAddTask(new_cgroup, pid); + if (rv < 0) { + virReportSystemError(-rv, + _("unable to add id %d task %d to cgroup"), + index, pid); + virCgroupRemove(new_cgroup); + goto error; + } + + return new_cgroup; + + error: + virCgroupFree(&new_cgroup); + return NULL; +} + +static int +qemuDomainHotplugVcpus(virQEMUDriverPtr driver, + virDomainObjPtr vm, + unsigned int nvcpus) { qemuDomainObjPrivatePtr priv = vm->privateData; size_t i; @@ -4726,26 +4761,12 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, if (nvcpus > oldvcpus) { for (i = oldvcpus; i < nvcpus; i++) { if (priv->cgroup) { - int rv = -1; - /* Create cgroup for the onlined vcpu */ - if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_VCPU_NAME, i, - true, &cgroup_vcpu) < 0) + cgroup_vcpu = qemuDomainHotplugAddCgroup(priv->cgroup, + VIR_CGROUP_VCPU_NAME, + i, mem_mask, + cpupids[i]); + if (!cgroup_vcpu) goto cleanup; - - if (mem_mask && - virCgroupSetCpusetMems(cgroup_vcpu, mem_mask) < 0) - goto cleanup; - - /* Add vcpu thread to the cgroup */ - rv = virCgroupAddTask(cgroup_vcpu, cpupids[i]); - if (rv < 0) { - virReportSystemError(-rv, - _("unable to add vcpu %zu task %d to cgroup"), - i, cpupids[i]); - virCgroupRemove(cgroup_vcpu); - goto cleanup; - } - }
/* Inherit def->cpuset */
ACK Jan

Future IOThread setting patches would copy the code anyway, so create and generalize a delete cgroup and pindef for the vcpu into its own API. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 43 +++++++++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1114ce9..cec2758 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4670,6 +4670,30 @@ qemuDomainHotplugAddCgroup(virCgroupPtr cgroup, } static int +qemuDomainHotplugDelCgroupPin(virCgroupPtr cgroup, + virCgroupThreadName nameval, + int index, + virDomainPinDefPtr **pindef_list, + size_t *npin) +{ + virCgroupPtr new_cgroup = NULL; + + if (cgroup) { + if (virCgroupNewThread(cgroup, nameval, index, false, &new_cgroup) < 0) + return -1; + + /* Remove the offlined cgroup */ + virCgroupRemove(new_cgroup); + virCgroupFree(&new_cgroup); + } + + /* Free pindef setting */ + virDomainPinDel(pindef_list, npin, index); + + return 0; +} + +static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, virDomainObjPtr vm, unsigned int nvcpus) @@ -4822,20 +4846,11 @@ qemuDomainHotplugVcpus(virQEMUDriverPtr driver, } } else { for (i = oldvcpus - 1; i >= nvcpus; i--) { - if (priv->cgroup) { - if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_VCPU_NAME, i, - false, &cgroup_vcpu) < 0) - goto cleanup; - - /* Remove cgroup for the offlined vcpu */ - virCgroupRemove(cgroup_vcpu); - virCgroupFree(&cgroup_vcpu); - } - - /* Free vcpupin setting */ - virDomainPinDel(&vm->def->cputune.vcpupin, - &vm->def->cputune.nvcpupin, - i); + if (qemuDomainHotplugDelCgroupPin(priv->cgroup, + VIR_CGROUP_VCPU_NAME, i, + &vm->def->cputune.vcpupin, + &vm->def->cputune.nvcpupin) < 0) + goto cleanup; } } -- 2.1.0

On Tue, Apr 07, 2015 at 09:23:16AM -0400, John Ferlan wrote:
Future IOThread setting patches would copy the code anyway, so create and generalize a delete cgroup and pindef for the vcpu into its own API.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 43 +++++++++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1114ce9..cec2758 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4670,6 +4670,30 @@ qemuDomainHotplugAddCgroup(virCgroupPtr cgroup, }
static int +qemuDomainHotplugDelCgroupPin(virCgroupPtr cgroup,
qemuDomainDelCgroupForThread
+ virCgroupThreadName nameval, + int index,
+ virDomainPinDefPtr **pindef_list, + size_t *npin)
I think the pin deletion should be separate from this function. These arguments would not be needed then. ACK with those two changes. Jan

Future IOThread setting patches would copy the code anyway, so create and generalize the adding of pindef for the vcpu into its own API Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 92 +++++++++++++++++++++++++++++--------------------- 1 file changed, 54 insertions(+), 38 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index cec2758..1601ba3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4670,6 +4670,56 @@ qemuDomainHotplugAddCgroup(virCgroupPtr cgroup, } static int +qemuDomainHotplugAddPin(virBitmapPtr cpumask, + int index, + pid_t pid, + virDomainPinDefPtr **pindef_list, + size_t *npin, + virCgroupPtr cgroup) +{ + int ret = -1; + virDomainPinDefPtr pindef = NULL; + + /* vm->def->cputune.* arrays can't be NULL if + * vm->def->cpumask is not NULL. + */ + if (VIR_ALLOC(pindef) < 0) + goto cleanup; + + if (!(pindef->cpumask = virBitmapNewCopy(cpumask))) { + VIR_FREE(pindef); + goto cleanup; + } + pindef->id = index; + if (VIR_APPEND_ELEMENT_COPY(*pindef_list, *npin, pindef) < 0) { + virBitmapFree(pindef->cpumask); + VIR_FREE(pindef); + goto cleanup; + } + + if (cgroup) { + if (qemuSetupCgroupCpusetCpus(cgroup, pindef->cpumask) < 0) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("failed to set cpuset.cpus in cgroup for id %d"), + index); + goto cleanup; + } + } else { + if (virProcessSetAffinity(pid, pindef->cpumask) < 0) { + virReportError(VIR_ERR_SYSTEM_ERROR, + _("failed to set cpu affinity for id %d"), + index); + goto cleanup; + } + } + + ret = 0; + + cleanup: + return ret; +} + +static int qemuDomainHotplugDelCgroupPin(virCgroupPtr cgroup, virCgroupThreadName nameval, int index, @@ -4795,48 +4845,14 @@ qemuDomainHotplugVcpus(virQEMUDriverPtr driver, /* Inherit def->cpuset */ if (vm->def->cpumask) { - /* vm->def->cputune.vcpupin can't be NULL if - * vm->def->cpumask is not NULL. - */ - virDomainPinDefPtr vcpupin = NULL; - - if (VIR_ALLOC(vcpupin) < 0) - goto cleanup; - - if (!(vcpupin->cpumask = virBitmapNewCopy(vm->def->cpumask))) { - VIR_FREE(vcpupin); - goto cleanup; - } - vcpupin->id = i; - if (VIR_APPEND_ELEMENT_COPY(vm->def->cputune.vcpupin, - vm->def->cputune.nvcpupin, vcpupin) < 0) { - virBitmapFree(vcpupin->cpumask); - VIR_FREE(vcpupin); + if (qemuDomainHotplugAddPin(vm->def->cpumask, i, cpupids[i], + &vm->def->cputune.vcpupin, + &vm->def->cputune.nvcpupin, + cgroup_vcpu) < 0) { ret = -1; goto cleanup; } - - if (cgroup_vcpu) { - if (qemuSetupCgroupCpusetCpus(cgroup_vcpu, - vcpupin->cpumask) < 0) { - virReportError(VIR_ERR_OPERATION_INVALID, - _("failed to set cpuset.cpus in cgroup" - " for vcpu %zu"), i); - ret = -1; - goto cleanup; - } - } else { - if (virProcessSetAffinity(cpupids[i], - vcpupin->cpumask) < 0) { - virReportError(VIR_ERR_SYSTEM_ERROR, - _("failed to set cpu affinity for vcpu %zu"), - i); - ret = -1; - goto cleanup; - } - } } - virCgroupFree(&cgroup_vcpu); if (qemuProcessSetSchedParams(i, cpupids[i], -- 2.1.0

On Tue, Apr 07, 2015 at 09:23:17AM -0400, John Ferlan wrote:
Future IOThread setting patches would copy the code anyway, so create and generalize the adding of pindef for the vcpu into its own API
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 92 +++++++++++++++++++++++++++++--------------------- 1 file changed, 54 insertions(+), 38 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index cec2758..1601ba3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4670,6 +4670,56 @@ qemuDomainHotplugAddCgroup(virCgroupPtr cgroup, }
static int +qemuDomainHotplugAddPin(virBitmapPtr cpumask, + int index, + pid_t pid, + virDomainPinDefPtr **pindef_list, + size_t *npin, + virCgroupPtr cgroup) +{ + int ret = -1; + virDomainPinDefPtr pindef = NULL; + + /* vm->def->cputune.* arrays can't be NULL if + * vm->def->cpumask is not NULL. + */ + if (VIR_ALLOC(pindef) < 0) + goto cleanup; + + if (!(pindef->cpumask = virBitmapNewCopy(cpumask))) { + VIR_FREE(pindef); + goto cleanup; + } + pindef->id = index; + if (VIR_APPEND_ELEMENT_COPY(*pindef_list, *npin, pindef) < 0) { + virBitmapFree(pindef->cpumask); + VIR_FREE(pindef); + goto cleanup; + }
I'd rather split this function into two - one that adds the pin definition and another that pins the thread Jan.

I'd rather split this function into two - one that adds the pin definition and another that pins the thread
Jan.
I've resent 5/5 again - other changes are made, but I'll push as a series... Not always very creative with the function names... John

Future IOThread setting patches would copy the code anyway, so create and generalize the adding of pindef for the vcpu and the pinning of the thread into their own APIs. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 103 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 67 insertions(+), 36 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7810dee..4165e9e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4670,6 +4670,67 @@ qemuDomainAddCgroupForThread(virCgroupPtr cgroup, } static int +qemuDomainHotplugAddPin(virBitmapPtr cpumask, + int index, + virDomainPinDefPtr **pindef_list, + size_t *npin) +{ + int ret = -1; + virDomainPinDefPtr pindef = NULL; + + /* vm->def->cputune.* arrays can't be NULL if + * vm->def->cpumask is not NULL. + */ + if (VIR_ALLOC(pindef) < 0) + goto cleanup; + + if (!(pindef->cpumask = virBitmapNewCopy(cpumask))) { + VIR_FREE(pindef); + goto cleanup; + } + pindef->id = index; + if (VIR_APPEND_ELEMENT_COPY(*pindef_list, *npin, pindef) < 0) { + virBitmapFree(pindef->cpumask); + VIR_FREE(pindef); + goto cleanup; + } + ret = 0; + + cleanup: + return ret; +} + +static int +qemuDomainHotplugPinThread(virBitmapPtr cpumask, + int index, + pid_t pid, + virCgroupPtr cgroup) +{ + int ret = -1; + + if (cgroup) { + if (qemuSetupCgroupCpusetCpus(cgroup, cpumask) < 0) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("failed to set cpuset.cpus in cgroup for id %d"), + index); + goto cleanup; + } + } else { + if (virProcessSetAffinity(pid, cpumask) < 0) { + virReportError(VIR_ERR_SYSTEM_ERROR, + _("failed to set cpu affinity for id %d"), + index); + goto cleanup; + } + } + + ret = 0; + + cleanup: + return ret; +} + +static int qemuDomainDelCgroupForThread(virCgroupPtr cgroup, virCgroupThreadName nameval, int index) @@ -4791,48 +4852,18 @@ qemuDomainHotplugVcpus(virQEMUDriverPtr driver, /* Inherit def->cpuset */ if (vm->def->cpumask) { - /* vm->def->cputune.vcpupin can't be NULL if - * vm->def->cpumask is not NULL. - */ - virDomainPinDefPtr vcpupin = NULL; - - if (VIR_ALLOC(vcpupin) < 0) - goto cleanup; - - if (!(vcpupin->cpumask = virBitmapNewCopy(vm->def->cpumask))) { - VIR_FREE(vcpupin); + if (qemuDomainHotplugAddPin(vm->def->cpumask, i, + &vm->def->cputune.vcpupin, + &vm->def->cputune.nvcpupin) < 0) { + ret = -1; goto cleanup; } - vcpupin->id = i; - if (VIR_APPEND_ELEMENT_COPY(vm->def->cputune.vcpupin, - vm->def->cputune.nvcpupin, vcpupin) < 0) { - virBitmapFree(vcpupin->cpumask); - VIR_FREE(vcpupin); + if (qemuDomainHotplugPinThread(vm->def->cpumask, i, cpupids[i], + cgroup_vcpu) < 0) { ret = -1; goto cleanup; } - - if (cgroup_vcpu) { - if (qemuSetupCgroupCpusetCpus(cgroup_vcpu, - vcpupin->cpumask) < 0) { - virReportError(VIR_ERR_OPERATION_INVALID, - _("failed to set cpuset.cpus in cgroup" - " for vcpu %zu"), i); - ret = -1; - goto cleanup; - } - } else { - if (virProcessSetAffinity(cpupids[i], - vcpupin->cpumask) < 0) { - virReportError(VIR_ERR_SYSTEM_ERROR, - _("failed to set cpu affinity for vcpu %zu"), - i); - ret = -1; - goto cleanup; - } - } } - virCgroupFree(&cgroup_vcpu); if (qemuProcessSetSchedParams(i, cpupids[i], -- 2.1.0

On Wed, Apr 08, 2015 at 09:50:43AM -0400, John Ferlan wrote:
Future IOThread setting patches would copy the code anyway, so create and generalize the adding of pindef for the vcpu and the pinning of the thread into their own APIs.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 103 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 67 insertions(+), 36 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7810dee..4165e9e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4670,6 +4670,67 @@ qemuDomainAddCgroupForThread(virCgroupPtr cgroup, }
static int +qemuDomainHotplugAddPin(virBitmapPtr cpumask, + int index, + virDomainPinDefPtr **pindef_list, + size_t *npin) +{ + int ret = -1; + virDomainPinDefPtr pindef = NULL; + + /* vm->def->cputune.* arrays can't be NULL if + * vm->def->cpumask is not NULL. + */
ACK with this comment removed. Jan
participants (2)
-
John Ferlan
-
Ján Tomko