[libvirt] [PATCH 0/3] qemu: vcpu: Split up qemuDomainSetVcpusFlags more
As requested in the review for the vcpu hotplug series, split the API code even more. Peter Krempa (3): qemu: setvcpus: Extract setting of maximum vcpu count qemu: driver: Extract setting of live vcpu count qemu: driver: Split out regular vcpu hotplug code into a function src/qemu/qemu_driver.c | 246 +++++++++++++++++++++++++++++-------------------- 1 file changed, 144 insertions(+), 102 deletions(-) -- 2.9.2
Setting of the maximum vcpu count is slightly semantically different thus split it into a self-contained func. --- src/qemu/qemu_driver.c | 67 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 44 insertions(+), 23 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3708146..b81baa5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4756,6 +4756,42 @@ qemuDomainSetVcpusAgent(virDomainObjPtr vm, static int +qemuDomainSetVcpusMax(virQEMUDriverPtr driver, + virDomainDefPtr def, + virDomainDefPtr persistentDef, + unsigned int nvcpus) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + int ret = -1; + + if (def) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("maximum vcpu count of a live domain can't be modified")); + goto cleanup; + } + + if (virDomainNumaGetCPUCountTotal(persistentDef->numa) > nvcpus) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("Number of CPUs in <numa> exceeds the desired " + "maximum vcpu count")); + goto cleanup; + } + + if (virDomainDefSetVcpusMax(persistentDef, nvcpus, driver->xmlopt) < 0) + goto cleanup; + + if (virDomainSaveConfig(cfg->configDir, driver->caps, persistentDef) < 0) + goto cleanup; + + ret = 0; + + cleanup: + virObjectUnref(cfg); + return ret; +} + + +static int qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, unsigned int flags) { @@ -4799,14 +4835,12 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0) goto endjob; - if (def) { - if (flags & VIR_DOMAIN_VCPU_MAXIMUM) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("maximum vcpu count of a live domain can't be " - "modified")); - goto endjob; - } + if (flags & VIR_DOMAIN_VCPU_MAXIMUM) { + ret = qemuDomainSetVcpusMax(driver, def, persistentDef, nvcpus); + goto endjob; + } + if (def) { if (nvcpus > virDomainDefGetVcpusMax(def)) { virReportError(VIR_ERR_INVALID_ARG, _("requested vcpus is greater than max allowable" @@ -4817,8 +4851,7 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, } if (persistentDef) { - if (!(flags & VIR_DOMAIN_VCPU_MAXIMUM) && - nvcpus > virDomainDefGetVcpusMax(persistentDef)) { + if (nvcpus > virDomainDefGetVcpusMax(persistentDef)) { virReportError(VIR_ERR_INVALID_ARG, _("requested vcpus is greater than max allowable" " vcpus for the persistent domain: %u > %u"), @@ -4862,20 +4895,8 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, } if (persistentDef) { - if (flags & VIR_DOMAIN_VCPU_MAXIMUM) { - if (virDomainNumaGetCPUCountTotal(persistentDef->numa) > nvcpus) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("Number of CPUs in <numa> exceeds the desired " - "maximum vcpu count")); - goto endjob; - } - - if (virDomainDefSetVcpusMax(persistentDef, nvcpus, driver->xmlopt) < 0) - goto endjob; - } else { - if (virDomainDefSetVcpus(persistentDef, nvcpus) < 0) - goto endjob; - } + if (virDomainDefSetVcpus(persistentDef, nvcpus) < 0) + goto endjob; if (virDomainSaveConfig(cfg->configDir, driver->caps, persistentDef) < 0) -- 2.9.2
The live code does ugly things. Contain it in a separate function. --- src/qemu/qemu_driver.c | 121 +++++++++++++++++++++++++++---------------------- 1 file changed, 68 insertions(+), 53 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b81baa5..95a7838 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4792,6 +4792,72 @@ qemuDomainSetVcpusMax(virQEMUDriverPtr driver, static int +qemuDomainSetVcpusLive(virQEMUDriverPtr driver, + virQEMUDriverConfigPtr cfg, + virDomainObjPtr vm, + unsigned int nvcpus) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + size_t i; + virCgroupPtr cgroup_temp = NULL; + char *mem_mask = NULL; + char *all_nodes_str = NULL; + virBitmapPtr all_nodes = NULL; + virErrorPtr err = NULL; + int ret = -1; + + if (virNumaIsAvailable() && + virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) { + if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0, + false, &cgroup_temp) < 0) + goto cleanup; + + if (!(all_nodes = virNumaGetHostNodeset())) + goto cleanup; + + if (!(all_nodes_str = virBitmapFormat(all_nodes))) + goto cleanup; + + if (virCgroupGetCpusetMems(cgroup_temp, &mem_mask) < 0 || + virCgroupSetCpusetMems(cgroup_temp, all_nodes_str) < 0) + goto cleanup; + } + + if (nvcpus > virDomainDefGetVcpus(vm->def)) { + for (i = virDomainDefGetVcpus(vm->def); i < nvcpus; i++) { + if (qemuDomainHotplugAddVcpu(driver, vm, i) < 0) + goto cleanup; + } + } else { + for (i = virDomainDefGetVcpus(vm->def) - 1; i >= nvcpus; i--) { + if (qemuDomainHotplugDelVcpu(driver, vm, i) < 0) + goto cleanup; + } + } + + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) + goto cleanup; + + ret = 0; + + cleanup: + if (mem_mask) { + err = virSaveLastError(); + virCgroupSetCpusetMems(cgroup_temp, mem_mask); + virSetError(err); + virFreeError(err); + VIR_FREE(mem_mask); + } + + VIR_FREE(all_nodes_str); + virBitmapFree(all_nodes); + virCgroupFree(&cgroup_temp); + + return ret; +} + + +static int qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, unsigned int flags) { @@ -4801,13 +4867,6 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, virDomainDefPtr persistentDef; int ret = -1; virQEMUDriverConfigPtr cfg = NULL; - qemuDomainObjPrivatePtr priv; - size_t i; - virCgroupPtr cgroup_temp = NULL; - char *mem_mask = NULL; - char *all_nodes_str = NULL; - virBitmapPtr all_nodes = NULL; - virErrorPtr err = NULL; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG | @@ -4822,8 +4881,6 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, if (virDomainSetVcpusFlagsEnsureACL(dom->conn, vm->def, flags) < 0) goto cleanup; - priv = vm->privateData; - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; @@ -4860,39 +4917,8 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, } } - if (def && virNumaIsAvailable() && - virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) { - if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0, - false, &cgroup_temp) < 0) - goto endjob; - - if (!(all_nodes = virNumaGetHostNodeset())) - goto endjob; - - if (!(all_nodes_str = virBitmapFormat(all_nodes))) - goto endjob; - - if (virCgroupGetCpusetMems(cgroup_temp, &mem_mask) < 0 || - virCgroupSetCpusetMems(cgroup_temp, all_nodes_str) < 0) - goto endjob; - } - - if (def) { - if (nvcpus > virDomainDefGetVcpus(def)) { - for (i = virDomainDefGetVcpus(def); i < nvcpus; i++) { - if (qemuDomainHotplugAddVcpu(driver, vm, i) < 0) - goto endjob; - } - } else { - for (i = virDomainDefGetVcpus(def) - 1; i >= nvcpus; i--) { - if (qemuDomainHotplugDelVcpu(driver, vm, i) < 0) - goto endjob; - } - } - - if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) - goto endjob; - } + if (def && qemuDomainSetVcpusLive(driver, cfg, vm, nvcpus) < 0) + goto endjob; if (persistentDef) { if (virDomainDefSetVcpus(persistentDef, nvcpus) < 0) @@ -4906,21 +4932,10 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, ret = 0; endjob: - if (mem_mask) { - err = virSaveLastError(); - virCgroupSetCpusetMems(cgroup_temp, mem_mask); - virSetError(err); - virFreeError(err); - } - qemuDomainObjEndJob(driver, vm); cleanup: virDomainObjEndAPI(&vm); - VIR_FREE(mem_mask); - VIR_FREE(all_nodes_str); - virBitmapFree(all_nodes); - virCgroupFree(&cgroup_temp); virObjectUnref(cfg); return ret; } -- 2.9.2
All other modes of qemuDomainSetVcpusFlags have helpers so finish the work by splitting the regular code into a new function. This patch also touches up the coding (spacing) style. --- src/qemu/qemu_driver.c | 100 ++++++++++++++++++++++++++----------------------- 1 file changed, 53 insertions(+), 47 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 95a7838..8ee1194 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4858,7 +4858,53 @@ qemuDomainSetVcpusLive(virQEMUDriverPtr driver, static int -qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, +qemuDomainSetVcpusInternal(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDefPtr def, + virDomainDefPtr persistentDef, + unsigned int nvcpus) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + int ret = -1; + + if (def && nvcpus > virDomainDefGetVcpusMax(def)) { + virReportError(VIR_ERR_INVALID_ARG, + _("requested vcpus is greater than max allowable" + " vcpus for the live domain: %u > %u"), + nvcpus, virDomainDefGetVcpusMax(def)); + goto cleanup; + } + + if (persistentDef && nvcpus > virDomainDefGetVcpusMax(persistentDef)) { + virReportError(VIR_ERR_INVALID_ARG, + _("requested vcpus is greater than max allowable" + " vcpus for the persistent domain: %u > %u"), + nvcpus, virDomainDefGetVcpusMax(persistentDef)); + goto cleanup; + } + + if (def && qemuDomainSetVcpusLive(driver, cfg, vm, nvcpus) < 0) + goto cleanup; + + if (persistentDef) { + if (virDomainDefSetVcpus(persistentDef, nvcpus) < 0) + goto cleanup; + + if (virDomainSaveConfig(cfg->configDir, driver->caps, persistentDef) < 0) + goto cleanup; + } + + ret = 0; + + cleanup: + virObjectUnref(cfg); + return ret; +} + + +static int +qemuDomainSetVcpusFlags(virDomainPtr dom, + unsigned int nvcpus, unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; @@ -4866,7 +4912,6 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, virDomainDefPtr def; virDomainDefPtr persistentDef; int ret = -1; - virQEMUDriverConfigPtr cfg = NULL; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG | @@ -4876,70 +4921,31 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - cfg = virQEMUDriverGetConfig(driver); - if (virDomainSetVcpusFlagsEnsureACL(dom->conn, vm->def, flags) < 0) goto cleanup; if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; - if (flags & VIR_DOMAIN_VCPU_GUEST) { - ret = qemuDomainSetVcpusAgent(vm, nvcpus); - goto endjob; - } - if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0) goto endjob; - if (flags & VIR_DOMAIN_VCPU_MAXIMUM) { + if (flags & VIR_DOMAIN_VCPU_GUEST) + ret = qemuDomainSetVcpusAgent(vm, nvcpus); + else if (flags & VIR_DOMAIN_VCPU_MAXIMUM) ret = qemuDomainSetVcpusMax(driver, def, persistentDef, nvcpus); - goto endjob; - } - - if (def) { - if (nvcpus > virDomainDefGetVcpusMax(def)) { - virReportError(VIR_ERR_INVALID_ARG, - _("requested vcpus is greater than max allowable" - " vcpus for the live domain: %u > %u"), - nvcpus, virDomainDefGetVcpusMax(def)); - goto endjob; - } - } - - if (persistentDef) { - if (nvcpus > virDomainDefGetVcpusMax(persistentDef)) { - virReportError(VIR_ERR_INVALID_ARG, - _("requested vcpus is greater than max allowable" - " vcpus for the persistent domain: %u > %u"), - nvcpus, virDomainDefGetVcpusMax(persistentDef)); - goto endjob; - } - } - - if (def && qemuDomainSetVcpusLive(driver, cfg, vm, nvcpus) < 0) - goto endjob; - - if (persistentDef) { - if (virDomainDefSetVcpus(persistentDef, nvcpus) < 0) - goto endjob; - - if (virDomainSaveConfig(cfg->configDir, driver->caps, - persistentDef) < 0) - goto endjob; - } - - ret = 0; + else + ret = qemuDomainSetVcpusInternal(driver, vm, def, persistentDef, nvcpus); endjob: qemuDomainObjEndJob(driver, vm); cleanup: virDomainObjEndAPI(&vm); - virObjectUnref(cfg); return ret; } + static int qemuDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) { -- 2.9.2
participants (2)
-
Pavel Hrdina -
Peter Krempa