Re: [libvirt] [PATCH 10/34] qemu: Don't use priv->ncpus to iterate cgroup setting
On 01/14/2016 11:26 AM, Peter Krempa wrote:
Iterate over all cpus skipping inactive ones.
---
src/qemu/qemu_driver.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
Patch 7 introduces virDomainDefGetVcpumap (or GetOnlineVcpuMap) - why
not use that instead and then iterate the set bits. This works, but the
less places that check for vcpu->online perhaps the better. Perhaps also
a way to reduce the decision points of using ->maxvcpus or the
virDomainDefGetVcpusMax call...
Also, I'm going to stop here and pick it up again Monday - figured I'd
at least start and get part way through to reduce the pile for next
week. I also ran the whole series through a Coverity build and found a
couple of issues in patch 20 and 29 (which I'll note now, but review
that code more in depth later).
Other than the issue I noted in patch 4 which I think needs to be
addressed (although looking back it might be able to use the
GetOnlineVcpuMap too), consider this as an ACK for 1-3, 5-10 with any
nits noted cleaned up.
John
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2300c7e..3102fc2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10005,7 +10005,12 @@ qemuDomainSetNumaParamsLive(virDomainObjPtr vm,
goto cleanup;
virCgroupFree(&cgroup_temp);
- for (i = 0; i < priv->nvcpupids; i++) {
+ for (i = 0; i < virDomainDefGetVcpusMax(vm->def); i++) {
+ virDomainVcpuInfoPtr vcpu = virDomainDefGetVcpu(vm->def, i);
+
+ if (!vcpu->online)
+ continue;
+
if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_VCPU, i,
false, &cgroup_temp) < 0 ||
virCgroupSetCpusetMems(cgroup_temp, nodeset_str) < 0)
@@ -10238,7 +10243,6 @@ qemuSetVcpusBWLive(virDomainObjPtr vm, virCgroupPtr cgroup,
unsigned long long period, long long quota)
{
size_t i;
- qemuDomainObjPrivatePtr priv = vm->privateData;
virCgroupPtr cgroup_vcpu = NULL;
if (period == 0 && quota == 0)
@@ -10247,7 +10251,12 @@ qemuSetVcpusBWLive(virDomainObjPtr vm, virCgroupPtr cgroup,
if (!qemuDomainHasVcpuPids(vm))
return 0;
- for (i = 0; i < priv->nvcpupids; i++) {
+ for (i = 0; i < virDomainDefGetVcpusMax(vm->def); i++) {
+ virDomainVcpuInfoPtr vcpu = virDomainDefGetVcpu(vm->def, i);
+
+ if (!vcpu->online)
+ continue;
+
if (virCgroupNewThread(cgroup, VIR_CGROUP_THREAD_VCPU, i,
false, &cgroup_vcpu) < 0)
goto cleanup;