[libvirt] [PATCH 0/3] REWORK/REPOST: Refactor some qemuDomainHotplugVcpus code
Originally these were in the middle of the IOThreads Setting code that needs some rework. So rather than needing to have to continually merge changes in this space, I separated them out. These are (hopefully) pure code motion and will be used in some manner by changes that will come this release cycle for IOThread Add/Del Support. IOThreads's posting: http://www.redhat.com/archives/libvir-list/2015-March/msg01014.html and specifically in order: http://www.redhat.com/archives/libvir-list/2015-March/msg01017.html http://www.redhat.com/archives/libvir-list/2015-March/msg01018.html http://www.redhat.com/archives/libvir-list/2015-March/msg01019.html I did have to make some changes to the last one to account for other recent changes in the code John Ferlan (3): qemu: qemuDomainHotplugVcpus - separate out the add cgroup qemu: qemuDomainHotplugVcpus - separate out the del cgroup and pin qemu: qemuDomainHotplugVcpus - separate out pin adjustment code src/qemu/qemu_driver.c | 201 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 130 insertions(+), 71 deletions(-) -- 2.1.0
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 | 69 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 48 insertions(+), 21 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6132674..fa880b7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4635,9 +4635,49 @@ static void qemuProcessEventHandler(void *data, void *opaque) VIR_FREE(processEvent); } -static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, - virDomainObjPtr vm, - unsigned int nvcpus) +typedef int cgroupNewFunc(virCgroupPtr domain, + int id, + bool create, + virCgroupPtr *group); + +static virCgroupPtr +qemuDomainHotplugAddCgroup(virCgroupPtr cgroup, + cgroupNewFunc func, + int index, + char *mem_mask, + pid_t pid) +{ + virCgroupPtr new_cgroup = NULL; + int rv = -1; + + /* Create cgroup */ + if (func(cgroup, 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,25 +4766,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 (virCgroupNewVcpu(priv->cgroup, i, true, &cgroup_vcpu) < 0) + cgroup_vcpu = qemuDomainHotplugAddCgroup(priv->cgroup, + virCgroupNewVcpu, + 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 Thu, Apr 02, 2015 at 12:24:40PM -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 | 69 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 48 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6132674..fa880b7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4635,9 +4635,49 @@ static void qemuProcessEventHandler(void *data, void *opaque) VIR_FREE(processEvent); }
-static int qemuDomainHotplugVcpus(virQEMUDriverPtr driver, - virDomainObjPtr vm, - unsigned int nvcpus) +typedef int cgroupNewFunc(virCgroupPtr domain, + int id, + bool create, + virCgroupPtr *group); +
We have three virCgroupNew* functions that only differ by the parameters of the virAsprintf call. Would it make sense to turn them into one like this: virCgroupNewThread(virCgroupThreadName name, int id, ...) which would take the name of the thread from the enum, instead of the function name? 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 | 40 ++++++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fa880b7..5dc1ad4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4675,6 +4675,30 @@ qemuDomainHotplugAddCgroup(virCgroupPtr cgroup, } static int +qemuDomainHotplugDelCgroupPin(virCgroupPtr cgroup, + cgroupNewFunc func, + int index, + virDomainPinDefPtr **pindef_list, + size_t *npin) +{ + virCgroupPtr new_cgroup = NULL; + + if (cgroup) { + if (func(cgroup, 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) @@ -4827,19 +4851,11 @@ qemuDomainHotplugVcpus(virQEMUDriverPtr driver, } } else { for (i = oldvcpus - 1; i >= nvcpus; i--) { - if (priv->cgroup) { - if (virCgroupNewVcpu(priv->cgroup, i, false, &cgroup_vcpu) < 0) - goto cleanup; - - /* Remove cgroup for the offlined vcpu */ - virCgroupRemove(cgroup_vcpu); - virCgroupFree(&cgroup_vcpu); - } + if (qemuDomainHotplugDelCgroupPin(priv->cgroup, virCgroupNewVcpu, i, + &vm->def->cputune.vcpupin, + &vm->def->cputune.nvcpupin) < 0) + goto cleanup; - /* Free vcpupin setting */ - virDomainPinDel(&vm->def->cputune.vcpupin, - &vm->def->cputune.nvcpupin, - i); } } -- 2.1.0
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 5dc1ad4..f047403 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4675,6 +4675,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, cgroupNewFunc func, int index, @@ -4800,48 +4850,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
participants (2)
-
John Ferlan -
Ján Tomko