[libvirt] [PATCH v2 00/27] Prepare for specific vcpu hot(un)plug - part 1

This series is getting rather big. The target is to refactor the way libvirt stores info about vCPUs into a single structure (okay, two structures for the qemu driver. Part 1 is not yet completely there, well, not even halfway. Future work will involve fully allocating priv->vcpupids to the maxcpus size and moving around few other bits of data in cputune and other parts to the new structure. Yet another follow up work is then to add new APIs for vCPU hotplug, which will enable adding vCPUs sparsely (useful if you have NUMA). Since this refactor will result in tracking all vcpu-related data in one struct, the result will automagically fix a few bugs where we'd end up with invalid config after vcpu unplug or other operations. Version 2 does not contain already pushed patches and incorporates feedback from John's review. Since I've changed quite a few things I'm reposting this. Peter Krempa (27): conf: Replace writes to def->maxvcpus with accessor conf: Use local copy of maxvcpus in virDomainVcpuParse conf: Extract update of vcpu count if maxvcpus is decreased conf: Add helper to check whether domain has offline vCPUs conf: Replace read access to def->maxvcpus with accessor conf: Replace writes to def->vcpus with accessor conf: Move vcpu count check into helper conf: Replace read accesses to def->vcpus with accessor conf: Turn def->maxvcpus into size_t qemu: domain: Add helper to access vm->privateData->agent qemu: Extract vCPU onlining/offlining via agent into a separate function qemu: qemuDomainSetVcpusAgent: re-check agent before calling it the again qemu: Split up vCPU hotplug and hotunplug qemu: cpu hotplug: Fix error handling logic qemu: monitor: Remove weird return values from qemuMonitorSetCPU qemu: cpu hotplug: Move loops to qemuDomainSetVcpusFlags qemu: Refactor qemuDomainHotplugVcpus qemu: refactor qemuDomainHotunplugVcpus conf: turn def->vcpus into a structure conf: ABI: Split up and improve vcpu info ABI checking conf: Add helper to get pointer to a certain vCPU definition qemu: cgroup: Remove now unreachable check qemu: Drop checking vcpu threads in emulator bandwidth getter/setter qemu: Replace checking for vcpu<->pid mapping availability with a helper qemu: Add helper to retrieve vCPU pid qemu: driver: Refactor qemuDomainHelperGetVcpus qemu: cgroup: Don't use priv->ncpupids to iterate domain vCPUs src/bhyve/bhyve_command.c | 2 +- src/bhyve/bhyve_driver.c | 2 +- src/conf/domain_audit.c | 2 +- src/conf/domain_conf.c | 190 ++++++++++++--- src/conf/domain_conf.h | 20 +- src/hyperv/hyperv_driver.c | 10 +- src/libvirt_private.syms | 6 + src/libxl/libxl_conf.c | 6 +- src/libxl/libxl_driver.c | 40 ++-- src/lxc/lxc_controller.c | 2 +- src/lxc/lxc_driver.c | 2 +- src/lxc/lxc_native.c | 7 +- src/openvz/openvz_conf.c | 7 +- src/openvz/openvz_driver.c | 19 +- src/phyp/phyp_driver.c | 12 +- src/qemu/qemu_cgroup.c | 42 ++-- src/qemu/qemu_command.c | 29 ++- src/qemu/qemu_domain.c | 47 ++++ src/qemu/qemu_domain.h | 4 + src/qemu/qemu_driver.c | 542 ++++++++++++++++++++++--------------------- src/qemu/qemu_monitor.c | 3 + src/qemu/qemu_monitor_json.c | 8 - src/qemu/qemu_monitor_text.c | 23 +- src/qemu/qemu_process.c | 22 +- src/test/test_driver.c | 38 +-- src/uml/uml_driver.c | 2 +- src/vbox/vbox_common.c | 19 +- src/vmware/vmware_driver.c | 2 +- src/vmx/vmx.c | 38 +-- src/vz/vz_driver.c | 8 +- src/vz/vz_sdk.c | 13 +- src/xen/xm_internal.c | 19 +- src/xenapi/xenapi_driver.c | 7 +- src/xenapi/xenapi_utils.c | 6 +- src/xenconfig/xen_common.c | 16 +- src/xenconfig/xen_sxpr.c | 27 ++- 36 files changed, 767 insertions(+), 475 deletions(-) -- 2.6.2

To support further refactors replace all write access to def->maxvcpus with a accessor function. --- src/conf/domain_conf.c | 18 ++++++++++++++++-- src/conf/domain_conf.h | 2 ++ src/hyperv/hyperv_driver.c | 5 ++++- src/libvirt_private.syms | 1 + src/libxl/libxl_driver.c | 8 ++++++-- src/lxc/lxc_native.c | 4 +++- src/openvz/openvz_conf.c | 4 +++- src/openvz/openvz_driver.c | 5 ++++- src/phyp/phyp_driver.c | 4 +++- src/qemu/qemu_command.c | 9 +++++++-- src/qemu/qemu_driver.c | 4 +++- src/test/test_driver.c | 4 +++- src/vbox/vbox_common.c | 11 +++++++++-- src/vmx/vmx.c | 5 ++++- src/vz/vz_sdk.c | 4 +++- src/xen/xm_internal.c | 4 +++- src/xenapi/xenapi_driver.c | 4 +++- src/xenconfig/xen_common.c | 4 +++- src/xenconfig/xen_sxpr.c | 3 ++- 19 files changed, 82 insertions(+), 21 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7ec6954..02b5e6d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1430,6 +1430,16 @@ void virDomainLeaseDefFree(virDomainLeaseDefPtr def) } +int +virDomainDefSetVcpusMax(virDomainDefPtr def, + unsigned int maxvcpus) +{ + def->maxvcpus = maxvcpus; + + return 0; +} + + virDomainDiskDefPtr virDomainDiskDefNew(virDomainXMLOptionPtr xmlopt) { @@ -14691,18 +14701,22 @@ virDomainVcpuParse(virDomainDefPtr def, { int n; char *tmp = NULL; + unsigned int maxvcpus; int ret = -1; - if ((n = virXPathUInt("string(./vcpu[1])", ctxt, &def->maxvcpus)) < 0) { + if ((n = virXPathUInt("string(./vcpu[1])", ctxt, &maxvcpus)) < 0) { if (n == -2) { virReportError(VIR_ERR_XML_ERROR, "%s", _("maximum vcpus count must be an integer")); goto cleanup; } - def->maxvcpus = 1; + maxvcpus = 1; } + if (virDomainDefSetVcpusMax(def, maxvcpus) < 0) + goto cleanup; + if ((n = virXPathUInt("string(./vcpu[1]/@current)", ctxt, &def->vcpus)) < 0) { if (n == -2) { virReportError(VIR_ERR_XML_ERROR, "%s", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 038d65b..f65026b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2337,6 +2337,8 @@ struct _virDomainDef { xmlNodePtr metadata; }; +int virDomainDefSetVcpusMax(virDomainDefPtr def, unsigned int vcpus); + unsigned long long virDomainDefGetMemoryInitial(const virDomainDef *def); void virDomainDefSetMemoryTotal(virDomainDefPtr def, unsigned long long size); void virDomainDefSetMemoryInitial(virDomainDefPtr def, unsigned long long size); diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 72261df..1e8db03 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -873,8 +873,11 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) virDomainDefSetMemoryTotal(def, memorySettingData->data->Limit * 1024); /* megabyte to kilobyte */ def->mem.cur_balloon = memorySettingData->data->VirtualQuantity * 1024; /* megabyte to kilobyte */ + if (virDomainDefSetVcpusMax(def, + processorSettingData->data->VirtualQuantity) < 0) + goto cleanup; + def->vcpus = processorSettingData->data->VirtualQuantity; - def->maxvcpus = processorSettingData->data->VirtualQuantity; def->os.type = VIR_DOMAIN_OSTYPE_HVM; /* FIXME: devices section is totally missing */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 42528a4..b1bced7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -230,6 +230,7 @@ virDomainDefParseString; virDomainDefPostParse; virDomainDefSetMemoryInitial; virDomainDefSetMemoryTotal; +virDomainDefSetVcpusMax; virDomainDeleteConfig; virDomainDeviceAddressIsValid; virDomainDeviceAddressTypeToString; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 35d7fae..dbc2b78 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -552,8 +552,10 @@ libxlAddDom0(libxlDriverPrivatePtr driver) def = NULL; virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_BOOTED); + if (virDomainDefSetVcpusMax(vm->def, d_info.vcpu_max_id + 1)) + goto cleanup; + vm->def->vcpus = d_info.vcpu_online; - vm->def->maxvcpus = d_info.vcpu_max_id + 1; vm->def->mem.cur_balloon = d_info.current_memkb; virDomainDefSetMemoryTotal(vm->def, d_info.max_memkb); @@ -2184,7 +2186,9 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, switch (flags) { case VIR_DOMAIN_VCPU_MAXIMUM | VIR_DOMAIN_VCPU_CONFIG: - def->maxvcpus = nvcpus; + if (virDomainDefSetVcpusMax(def, nvcpus) < 0) + goto cleanup; + if (nvcpus < def->vcpus) def->vcpus = nvcpus; break; diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index 2f95597..1c65475 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -1019,7 +1019,9 @@ lxcParseConfigString(const char *config) /* Value not handled by the LXC driver, setting to * minimum required to make XML parsing pass */ - vmdef->maxvcpus = 1; + if (virDomainDefSetVcpusMax(vmdef, 1) < 0) + goto error; + vmdef->vcpus = 1; vmdef->nfss = 0; diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index c0f65c9..6629105 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -582,7 +582,9 @@ int openvzLoadDomains(struct openvz_driver *driver) if (ret == 0 || vcpus == 0) vcpus = openvzGetNodeCPUs(); - def->maxvcpus = vcpus; + if (virDomainDefSetVcpusMax(def, vcpus) < 0) + goto cleanup; + def->vcpus = vcpus; /* XXX load rest of VM config data .... */ diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index b8c0f50..307b607 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1368,7 +1368,10 @@ static int openvzDomainSetVcpusInternal(virDomainObjPtr vm, if (virRun(prog, NULL) < 0) return -1; - vm->def->maxvcpus = vm->def->vcpus = nvcpus; + if (virDomainDefSetVcpusMax(vm->def, nvcpus) < 0) + return -1; + + vm->def->vcpus = nvcpus; return 0; } diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 14264c0..b60e5ba 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -3295,7 +3295,9 @@ phypDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) goto err; } - def.maxvcpus = vcpus; + if (virDomainDefSetVcpusMax(&def, vcpus) < 0) + goto err; + def.vcpus = vcpus; return virDomainDefFormat(&def, diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a0947b2..30d7bdb 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -12640,7 +12640,11 @@ qemuParseCommandLineSmp(virDomainDefPtr dom, } } - dom->maxvcpus = maxcpus ? maxcpus : dom->vcpus; + if (maxcpus == 0) + maxcpus = dom->vcpus; + + if (virDomainDefSetVcpusMax(dom, maxcpus) < 0) + goto error; if (sockets && cores && threads) { virCPUDefPtr cpu; @@ -12754,7 +12758,8 @@ qemuParseCommandLine(virCapsPtr qemuCaps, def->id = -1; def->mem.cur_balloon = 64 * 1024; virDomainDefSetMemoryTotal(def, def->mem.cur_balloon); - def->maxvcpus = 1; + if (virDomainDefSetVcpusMax(def, 1) < 0) + goto error; def->vcpus = 1; def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_UTC; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ae1d8e7..5dc7243 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4979,7 +4979,9 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, } if (flags & VIR_DOMAIN_VCPU_MAXIMUM) { - persistentDef->maxvcpus = nvcpus; + if (virDomainDefSetVcpusMax(persistentDef, nvcpus) < 0) + goto endjob; + if (nvcpus < persistentDef->vcpus) persistentDef->vcpus = nvcpus; } else { diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 9ccd567..6bf41d7 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2376,7 +2376,9 @@ testDomainSetVcpusFlags(virDomainPtr domain, unsigned int nrCpus, if (persistentDef) { if (flags & VIR_DOMAIN_VCPU_MAXIMUM) { - persistentDef->maxvcpus = nrCpus; + if (virDomainDefSetVcpusMax(persistentDef, nrCpus) < 0) + goto cleanup; + if (nrCpus < persistentDef->vcpus) persistentDef->vcpus = nrCpus; } else { diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 3e6ed7a..1a0cc63 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -3901,7 +3901,10 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) virDomainDefSetMemoryTotal(def, memorySize * 1024); gVBoxAPI.UIMachine.GetCPUCount(machine, &CPUCount); - def->maxvcpus = def->vcpus = CPUCount; + if (virDomainDefSetVcpusMax(def, CPUCount) < 0) + goto cleanup; + + def->vcpus = CPUCount; /* Skip cpumasklen, cpumask, onReboot, onPoweroff, onCrash */ @@ -6055,7 +6058,11 @@ static char *vboxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, def->dom->os.type = VIR_DOMAIN_OSTYPE_HVM; def->dom->os.arch = virArchFromHost(); gVBoxAPI.UIMachine.GetCPUCount(machine, &CPUCount); - def->dom->maxvcpus = def->dom->vcpus = CPUCount; + if (virDomainDefSetVcpusMax(def->dom, CPUCount) < 0) + goto cleanup; + + def->dom->vcpus = CPUCount; + if (vboxSnapshotGetReadWriteDisks(def, snapshot) < 0) VIR_DEBUG("Could not get read write disks for snapshot"); diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 7c3c10a..fe9d407 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -1457,7 +1457,10 @@ virVMXParseConfig(virVMXContext *ctx, goto cleanup; } - def->maxvcpus = def->vcpus = numvcpus; + if (virDomainDefSetVcpusMax(def, numvcpus) < 0) + goto cleanup; + + def->vcpus = numvcpus; /* vmx:sched.cpu.affinity -> def:cpumask */ /* NOTE: maps to VirtualMachine:config.cpuAffinity.affinitySet */ diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 750133d..eb0d2e8 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1150,8 +1150,10 @@ prlsdkConvertCpuInfo(PRL_HANDLE sdkdom, if (cpuCount > hostcpus) cpuCount = hostcpus; + if (virDomainDefSetVcpusMax(def, cpuCount) < 0) + goto cleanup; + def->vcpus = cpuCount; - def->maxvcpus = cpuCount; pret = PrlVmCfg_GetCpuMask(sdkdom, NULL, &buflen); prlsdkCheckRetGoto(pret, cleanup); diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 75f98b1..d465576 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -704,7 +704,9 @@ xenXMDomainSetVcpusFlags(virConnectPtr conn, } if (flags & VIR_DOMAIN_VCPU_MAXIMUM) { - entry->def->maxvcpus = vcpus; + if (virDomainDefSetVcpusMax(entry->def, vcpus) < 0) + goto cleanup; + if (entry->def->vcpus > vcpus) entry->def->vcpus = vcpus; } else { diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index e503974..fa66bb1 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -1502,7 +1502,9 @@ xenapiDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) vcpus = xenapiDomainGetMaxVcpus(dom); - defPtr->maxvcpus = vcpus; + if (virDomainDefSetVcpusMax(defPtr, vcpus) < 0) + goto error; + defPtr->vcpus = vcpus; enum xen_on_normal_exit action; diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index 0890c73..eba5be2 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -502,7 +502,9 @@ xenParseCPUFeatures(virConfPtr conf, virDomainDefPtr def) MAX_VIRT_CPUS < count) return -1; - def->maxvcpus = count; + if (virDomainDefSetVcpusMax(def, count) < 0) + return -1; + if (xenConfigGetULong(conf, "vcpu_avail", &count, -1) < 0) return -1; diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c index 7fc9c9d..76c4051 100644 --- a/src/xenconfig/xen_sxpr.c +++ b/src/xenconfig/xen_sxpr.c @@ -1173,7 +1173,8 @@ xenParseSxpr(const struct sexpr *root, } } - def->maxvcpus = sexpr_int(root, "domain/vcpus"); + if (virDomainDefSetVcpusMax(def, sexpr_int(root, "domain/vcpus")) < 0) + goto error; def->vcpus = count_one_bits_l(sexpr_u64(root, "domain/vcpu_avail")); if (!def->vcpus || def->maxvcpus < def->vcpus) def->vcpus = def->maxvcpus; -- 2.6.2

Use the local variable rather than getting it all the time from the struct. This will simplify further refactors. --- src/conf/domain_conf.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 02b5e6d..6a77964 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14724,13 +14724,13 @@ virDomainVcpuParse(virDomainDefPtr def, goto cleanup; } - def->vcpus = def->maxvcpus; + def->vcpus = maxvcpus; } - if (def->maxvcpus < def->vcpus) { + if (maxvcpus < def->vcpus) { virReportError(VIR_ERR_INTERNAL_ERROR, _("maxvcpus must not be less than current vcpus " - "(%u < %u)"), def->maxvcpus, def->vcpus); + "(%u < %u)"), maxvcpus, def->vcpus); goto cleanup; } -- 2.6.2

The code can be unified into the new accessor rather than being scattered accross the drivers. --- src/conf/domain_conf.c | 3 +++ src/libxl/libxl_driver.c | 3 --- src/qemu/qemu_driver.c | 3 --- src/test/test_driver.c | 3 --- src/xen/xm_internal.c | 3 --- 5 files changed, 3 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6a77964..99a789b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1434,6 +1434,9 @@ int virDomainDefSetVcpusMax(virDomainDefPtr def, unsigned int maxvcpus) { + if (maxvcpus < def->vcpus) + def->vcpus = maxvcpus; + def->maxvcpus = maxvcpus; return 0; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index dbc2b78..17d7736 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2188,9 +2188,6 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, case VIR_DOMAIN_VCPU_MAXIMUM | VIR_DOMAIN_VCPU_CONFIG: if (virDomainDefSetVcpusMax(def, nvcpus) < 0) goto cleanup; - - if (nvcpus < def->vcpus) - def->vcpus = nvcpus; break; case VIR_DOMAIN_VCPU_CONFIG: diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5dc7243..6b59687 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4981,9 +4981,6 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, if (flags & VIR_DOMAIN_VCPU_MAXIMUM) { if (virDomainDefSetVcpusMax(persistentDef, nvcpus) < 0) goto endjob; - - if (nvcpus < persistentDef->vcpus) - persistentDef->vcpus = nvcpus; } else { persistentDef->vcpus = nvcpus; } diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 6bf41d7..f579e0e 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2378,9 +2378,6 @@ testDomainSetVcpusFlags(virDomainPtr domain, unsigned int nrCpus, if (flags & VIR_DOMAIN_VCPU_MAXIMUM) { if (virDomainDefSetVcpusMax(persistentDef, nrCpus) < 0) goto cleanup; - - if (nrCpus < persistentDef->vcpus) - persistentDef->vcpus = nrCpus; } else { persistentDef->vcpus = nrCpus; } diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index d465576..bf8306a 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -706,9 +706,6 @@ xenXMDomainSetVcpusFlags(virConnectPtr conn, if (flags & VIR_DOMAIN_VCPU_MAXIMUM) { if (virDomainDefSetVcpusMax(entry->def, vcpus) < 0) goto cleanup; - - if (entry->def->vcpus > vcpus) - entry->def->vcpus = vcpus; } else { entry->def->vcpus = vcpus; } -- 2.6.2

The new helper will simplify checking whether the domain config contains inactive vCPUs. --- src/conf/domain_conf.c | 9 ++++++++- src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + src/openvz/openvz_driver.c | 2 +- src/qemu/qemu_command.c | 4 ++-- src/vbox/vbox_common.c | 2 +- src/vmx/vmx.c | 2 +- src/vz/vz_sdk.c | 2 +- src/xenconfig/xen_common.c | 2 +- src/xenconfig/xen_sxpr.c | 4 ++-- 10 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 99a789b..8289acf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1443,6 +1443,13 @@ virDomainDefSetVcpusMax(virDomainDefPtr def, } +bool +virDomainDefHasVcpusOffline(const virDomainDef *def) +{ + return def->vcpus < def->maxvcpus; +} + + virDomainDiskDefPtr virDomainDiskDefNew(virDomainXMLOptionPtr xmlopt) { @@ -21821,7 +21828,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAsprintf(buf, " cpuset='%s'", cpumask); VIR_FREE(cpumask); } - if (def->vcpus != def->maxvcpus) + if (virDomainDefHasVcpusOffline(def)) virBufferAsprintf(buf, " current='%u'", def->vcpus); virBufferAsprintf(buf, ">%u</vcpu>\n", def->maxvcpus); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f65026b..7f43d26 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2338,6 +2338,7 @@ struct _virDomainDef { }; int virDomainDefSetVcpusMax(virDomainDefPtr def, unsigned int vcpus); +bool virDomainDefHasVcpusOffline(const virDomainDef *def); unsigned long long virDomainDefGetMemoryInitial(const virDomainDef *def); void virDomainDefSetMemoryTotal(virDomainDefPtr def, unsigned long long size); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b1bced7..4ccf04c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -219,6 +219,7 @@ virDomainDefGetMemoryInitial; virDomainDefGetSecurityLabelDef; virDomainDefHasDeviceAddress; virDomainDefHasMemoryHotplug; +virDomainDefHasVcpusOffline; virDomainDefMaybeAddController; virDomainDefMaybeAddInput; virDomainDefNeedsPlacementAdvice; diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 307b607..56569d1 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1030,7 +1030,7 @@ openvzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int fla if (openvzDomainSetNetworkConfig(conn, vm->def) < 0) goto cleanup; - if (vm->def->vcpus != vm->def->maxvcpus) { + if (virDomainDefHasVcpusOffline(vm->def)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("current vcpu count must equal maximum")); goto cleanup; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 30d7bdb..f3327c0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7866,7 +7866,7 @@ qemuBuildSmpArgStr(const virDomainDef *def, virBufferAsprintf(&buf, "%u", def->vcpus); if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SMP_TOPOLOGY)) { - if (def->vcpus != def->maxvcpus) + if (virDomainDefHasVcpusOffline(def)) virBufferAsprintf(&buf, ",maxcpus=%u", def->maxvcpus); /* sockets, cores, and threads are either all zero * or all non-zero, thus checking one of them is enough */ @@ -7879,7 +7879,7 @@ qemuBuildSmpArgStr(const virDomainDef *def, virBufferAsprintf(&buf, ",cores=%u", 1); virBufferAsprintf(&buf, ",threads=%u", 1); } - } else if (def->vcpus != def->maxvcpus) { + } else if (virDomainDefHasVcpusOffline(def)) { virBufferFreeAndReset(&buf); /* FIXME - consider hot-unplugging cpus after boot for older qemu */ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 1a0cc63..0a17aeb 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -1891,7 +1891,7 @@ vboxDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags def->mem.cur_balloon, (unsigned)rc); } - if (def->vcpus != def->maxvcpus) { + if (virDomainDefHasVcpusOffline(def)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("current vcpu count must equal maximum")); } diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index fe9d407..0813553 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -3175,7 +3175,7 @@ virVMXFormatConfig(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virDomainDe } /* def:maxvcpus -> vmx:numvcpus */ - if (def->vcpus != def->maxvcpus) { + if (virDomainDefHasVcpusOffline(def)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("No support for domain XML entry 'vcpu' attribute " "'current'")); diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index eb0d2e8..c90220c 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1933,7 +1933,7 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom, virDomainDefPtr def) return -1; } - if (def->vcpus != def->maxvcpus) { + if (virDomainDefHasVcpusOffline(def)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("current vcpus must be equal to maxvcpus")); return -1; diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index eba5be2..878d1ae 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -1531,7 +1531,7 @@ xenFormatCPUAllocation(virConfPtr conf, virDomainDefPtr def) /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is either 32, or 64 on a platform where long is big enough. */ - if (def->vcpus < def->maxvcpus && + if (virDomainDefHasVcpusOffline(def) && xenConfigSetInt(conf, "vcpu_avail", (1UL << def->vcpus) - 1) < 0) goto cleanup; diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c index 76c4051..7799f9e 100644 --- a/src/xenconfig/xen_sxpr.c +++ b/src/xenconfig/xen_sxpr.c @@ -2226,7 +2226,7 @@ xenFormatSxpr(virConnectPtr conn, virBufferAsprintf(&buf, "(vcpus %u)", def->maxvcpus); /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is either 32, or 64 on a platform where long is big enough. */ - if (def->vcpus < def->maxvcpus) + if (virDomainDefHasVcpusOffline(def)) virBufferAsprintf(&buf, "(vcpu_avail %lu)", (1UL << def->vcpus) - 1); if (def->cpumask) { @@ -2308,7 +2308,7 @@ xenFormatSxpr(virConnectPtr conn, virBufferEscapeSexpr(&buf, "(kernel '%s')", def->os.loader->path); virBufferAsprintf(&buf, "(vcpus %u)", def->maxvcpus); - if (def->vcpus < def->maxvcpus) + if (virDomainDefHasVcpusOffline(def)) virBufferAsprintf(&buf, "(vcpu_avail %lu)", (1UL << def->vcpus) - 1); -- 2.6.2

Finalize the refactor by adding the 'virDomainDefGetVCpusMax' getter and reusing it accross libvirt. --- src/conf/domain_conf.c | 22 +++++++++++++++------- src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + src/libxl/libxl_conf.c | 4 ++-- src/libxl/libxl_driver.c | 11 +++++++---- src/openvz/openvz_driver.c | 8 ++++---- src/qemu/qemu_command.c | 4 ++-- src/qemu/qemu_driver.c | 10 +++++----- src/qemu/qemu_process.c | 2 +- src/test/test_driver.c | 13 ++++++++----- src/vbox/vbox_common.c | 4 ++-- src/vmx/vmx.c | 16 +++++++++------- src/vz/vz_driver.c | 2 +- src/xen/xm_internal.c | 9 ++++++--- src/xenapi/xenapi_utils.c | 4 ++-- src/xenconfig/xen_common.c | 4 ++-- src/xenconfig/xen_sxpr.c | 8 ++++---- 17 files changed, 72 insertions(+), 51 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8289acf..7d7ace9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1450,6 +1450,13 @@ virDomainDefHasVcpusOffline(const virDomainDef *def) } +unsigned int +virDomainDefGetVcpusMax(const virDomainDef *def) +{ + return def->maxvcpus; +} + + virDomainDiskDefPtr virDomainDiskDefNew(virDomainXMLOptionPtr xmlopt) { @@ -15300,7 +15307,8 @@ virDomainDefParseXML(xmlDocPtr xml, for (i = 0; i < def->cputune.nvcpusched; i++) { if (virDomainThreadSchedParse(nodes[i], - 0, def->maxvcpus - 1, + 0, + virDomainDefGetVcpusMax(def) - 1, "vcpus", &def->cputune.vcpusched[i]) < 0) goto error; @@ -15377,7 +15385,7 @@ virDomainDefParseXML(xmlDocPtr xml, goto error; if (def->cpu->sockets && - def->maxvcpus > + virDomainDefGetVcpusMax(def) > def->cpu->sockets * def->cpu->cores * def->cpu->threads) { virReportError(VIR_ERR_XML_DETAIL, "%s", _("Maximum CPUs greater than topology limit")); @@ -15389,14 +15397,14 @@ virDomainDefParseXML(xmlDocPtr xml, if (virDomainNumaDefCPUParseXML(def->numa, ctxt) < 0) goto error; - if (virDomainNumaGetCPUCountTotal(def->numa) > def->maxvcpus) { + if (virDomainNumaGetCPUCountTotal(def->numa) > virDomainDefGetVcpusMax(def)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Number of CPUs in <numa> exceeds the" " <vcpu> count")); goto error; } - if (virDomainNumaGetMaxCPUID(def->numa) >= def->maxvcpus) { + if (virDomainNumaGetMaxCPUID(def->numa) >= virDomainDefGetVcpusMax(def)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("CPU IDs in <numa> exceed the <vcpu> count")); goto error; @@ -17863,10 +17871,10 @@ virDomainDefCheckABIStability(virDomainDefPtr src, dst->vcpus, src->vcpus); goto error; } - if (src->maxvcpus != dst->maxvcpus) { + if (virDomainDefGetVcpusMax(src) != virDomainDefGetVcpusMax(dst)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target domain vCPU max %d does not match source %d"), - dst->maxvcpus, src->maxvcpus); + virDomainDefGetVcpusMax(dst), virDomainDefGetVcpusMax(src)); goto error; } @@ -21830,7 +21838,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, } if (virDomainDefHasVcpusOffline(def)) virBufferAsprintf(buf, " current='%u'", def->vcpus); - virBufferAsprintf(buf, ">%u</vcpu>\n", def->maxvcpus); + virBufferAsprintf(buf, ">%u</vcpu>\n", virDomainDefGetVcpusMax(def)); if (def->niothreadids > 0) { virBufferAsprintf(buf, "<iothreads>%u</iothreads>\n", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 7f43d26..2b2952a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2339,6 +2339,7 @@ struct _virDomainDef { int virDomainDefSetVcpusMax(virDomainDefPtr def, unsigned int vcpus); bool virDomainDefHasVcpusOffline(const virDomainDef *def); +unsigned int virDomainDefGetVcpusMax(const virDomainDef *def); unsigned long long virDomainDefGetMemoryInitial(const virDomainDef *def); void virDomainDefSetMemoryTotal(virDomainDefPtr def, unsigned long long size); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 4ccf04c..bba42c9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -217,6 +217,7 @@ virDomainDefGetDefaultEmulator; virDomainDefGetMemoryActual; virDomainDefGetMemoryInitial; virDomainDefGetSecurityLabelDef; +virDomainDefGetVcpusMax; virDomainDefHasDeviceAddress; virDomainDefHasMemoryHotplug; virDomainDefHasVcpusOffline; diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 4eed5ca..35089ea 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -643,8 +643,8 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, else libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PV); - b_info->max_vcpus = def->maxvcpus; - if (libxl_cpu_bitmap_alloc(ctx, &b_info->avail_vcpus, def->maxvcpus)) + b_info->max_vcpus = virDomainDefGetVcpusMax(def); + if (libxl_cpu_bitmap_alloc(ctx, &b_info->avail_vcpus, b_info->max_vcpus)) return -1; libxl_bitmap_set_none(&b_info->avail_vcpus); for (i = 0; i < def->vcpus; i++) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 17d7736..217957c 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2159,8 +2159,8 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, goto endjob; } - if (!(flags & VIR_DOMAIN_VCPU_MAXIMUM) && vm->def->maxvcpus < max) - max = vm->def->maxvcpus; + if (!(flags & VIR_DOMAIN_VCPU_MAXIMUM) && virDomainDefGetVcpusMax(vm->def) < max) + max = virDomainDefGetVcpusMax(vm->def); if (nvcpus > max) { virReportError(VIR_ERR_INVALID_ARG, @@ -2297,7 +2297,10 @@ libxlDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) def = vm->newDef ? vm->newDef : vm->def; } - ret = (flags & VIR_DOMAIN_VCPU_MAXIMUM) ? def->maxvcpus : def->vcpus; + if (flags & VIR_DOMAIN_VCPU_MAXIMUM) + ret = virDomainDefGetVcpusMax(def); + else + ret = def->vcpus; cleanup: if (vm) @@ -4695,7 +4698,7 @@ libxlDomainGetPerCPUStats(libxlDriverPrivatePtr driver, if (nparams == 0 && ncpus != 0) return LIBXL_NB_TOTAL_CPU_STAT_PARAM; else if (nparams == 0) - return vm->def->maxvcpus; + return virDomainDefGetVcpusMax(vm->def); cfg = libxlDriverConfigGet(driver); if ((vcpuinfo = libxl_list_vcpu(cfg->ctx, vm->def->id, &maxcpu, diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 56569d1..5e55033 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1035,8 +1035,8 @@ openvzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int fla _("current vcpu count must equal maximum")); goto cleanup; } - if (vm->def->maxvcpus > 0) { - if (openvzDomainSetVcpusInternal(vm, vm->def->maxvcpus) < 0) { + if (virDomainDefGetVcpusMax(vm->def)) { + if (openvzDomainSetVcpusInternal(vm, virDomainDefGetVcpusMax(vm->def)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not set number of vCPUs")); goto cleanup; @@ -1133,8 +1133,8 @@ openvzDomainCreateXML(virConnectPtr conn, const char *xml, vm->def->id = vm->pid; virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_BOOTED); - if (vm->def->maxvcpus > 0) { - if (openvzDomainSetVcpusInternal(vm, vm->def->maxvcpus) < 0) { + if (virDomainDefGetVcpusMax(vm->def) > 0) { + if (openvzDomainSetVcpusInternal(vm, virDomainDefGetVcpusMax(vm->def)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not set number of vCPUs")); goto cleanup; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f3327c0..0a0bc3b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7867,7 +7867,7 @@ qemuBuildSmpArgStr(const virDomainDef *def, if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SMP_TOPOLOGY)) { if (virDomainDefHasVcpusOffline(def)) - virBufferAsprintf(&buf, ",maxcpus=%u", def->maxvcpus); + virBufferAsprintf(&buf, ",maxcpus=%u", virDomainDefGetVcpusMax(def)); /* sockets, cores, and threads are either all zero * or all non-zero, thus checking one of them is enough */ if (def->cpu && def->cpu->sockets) { @@ -7875,7 +7875,7 @@ qemuBuildSmpArgStr(const virDomainDef *def, virBufferAsprintf(&buf, ",cores=%u", def->cpu->cores); virBufferAsprintf(&buf, ",threads=%u", def->cpu->threads); } else { - virBufferAsprintf(&buf, ",sockets=%u", def->maxvcpus); + virBufferAsprintf(&buf, ",sockets=%u", virDomainDefGetVcpusMax(def)); virBufferAsprintf(&buf, ",cores=%u", 1); virBufferAsprintf(&buf, ",threads=%u", 1); } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6b59687..6013443 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4911,10 +4911,10 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, } if (def) - maxvcpus = def->maxvcpus; + maxvcpus = virDomainDefGetVcpusMax(def); if (persistentDef) { - if (!maxvcpus || maxvcpus > persistentDef->maxvcpus) - maxvcpus = persistentDef->maxvcpus; + if (!maxvcpus || maxvcpus > virDomainDefGetVcpusMax(persistentDef)) + maxvcpus = virDomainDefGetVcpusMax(persistentDef); } if (!(flags & VIR_DOMAIN_VCPU_MAXIMUM) && nvcpus > maxvcpus) { virReportError(VIR_ERR_INVALID_ARG, @@ -5557,7 +5557,7 @@ qemuDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) } } else { if (flags & VIR_DOMAIN_VCPU_MAXIMUM) - ret = def->maxvcpus; + ret = virDomainDefGetVcpusMax(def); else ret = def->vcpus; } @@ -19078,7 +19078,7 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, &record->nparams, maxparams, "vcpu.maximum", - (unsigned) dom->def->maxvcpus) < 0) + virDomainDefGetVcpusMax(dom->def)) < 0) return -1; if (VIR_ALLOC_N(cpuinfo, dom->def->vcpus) < 0) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 192730c..a2d2bfd 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3870,7 +3870,7 @@ qemuValidateCpuMax(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) if (!maxCpus) return true; - if (def->maxvcpus > maxCpus) { + if (virDomainDefGetVcpusMax(def) > maxCpus) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Maximum CPUs greater than specified machine type limit")); return false; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index f579e0e..71b4513 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2312,7 +2312,10 @@ testDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags) if (!(def = virDomainObjGetOneDef(vm, flags))) goto cleanup; - ret = (flags & VIR_DOMAIN_VCPU_MAXIMUM) ? def->maxvcpus : def->vcpus; + if (flags & VIR_DOMAIN_VCPU_MAXIMUM) + ret = virDomainDefGetVcpusMax(def); + else + ret = def->vcpus; cleanup: virDomainObjEndAPI(&vm); @@ -2355,19 +2358,19 @@ testDomainSetVcpusFlags(virDomainPtr domain, unsigned int nrCpus, if (virDomainObjGetDefs(privdom, flags, &def, &persistentDef) < 0) goto cleanup; - if (def && def->maxvcpus < nrCpus) { + if (def && virDomainDefGetVcpusMax(def) < nrCpus) { virReportError(VIR_ERR_INVALID_ARG, _("requested cpu amount exceeds maximum (%d > %d)"), - nrCpus, def->maxvcpus); + nrCpus, virDomainDefGetVcpusMax(def)); goto cleanup; } if (persistentDef && !(flags & VIR_DOMAIN_VCPU_MAXIMUM) && - persistentDef->maxvcpus < nrCpus) { + virDomainDefGetVcpusMax(persistentDef) < nrCpus) { virReportError(VIR_ERR_INVALID_ARG, _("requested cpu amount exceeds maximum (%d > %d)"), - nrCpus, persistentDef->maxvcpus); + nrCpus, virDomainDefGetVcpusMax(persistentDef)); goto cleanup; } diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 0a17aeb..e2e8ebc 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -1895,11 +1895,11 @@ vboxDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("current vcpu count must equal maximum")); } - rc = gVBoxAPI.UIMachine.SetCPUCount(machine, def->maxvcpus); + rc = gVBoxAPI.UIMachine.SetCPUCount(machine, virDomainDefGetVcpusMax(def)); if (NS_FAILED(rc)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("could not set the number of virtual CPUs to: %u, rc=%08x"), - def->maxvcpus, (unsigned)rc); + virDomainDefGetVcpusMax(def), (unsigned)rc); } rc = gVBoxAPI.UIMachine.SetCPUProperty(machine, CPUPropertyType_PAE, diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 0813553..4b94df2 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -3066,6 +3066,7 @@ virVMXFormatConfig(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virDomainDe bool scsi_present[4] = { false, false, false, false }; int scsi_virtualDev[4] = { -1, -1, -1, -1 }; bool floppy_present[2] = { false, false }; + unsigned int maxvcpus; if (ctx->formatFileName == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -3181,15 +3182,16 @@ virVMXFormatConfig(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virDomainDe "'current'")); goto cleanup; } - if (def->maxvcpus <= 0 || (def->maxvcpus % 2 != 0 && def->maxvcpus != 1)) { + maxvcpus = virDomainDefGetVcpusMax(def); + if (maxvcpus == 0 || (maxvcpus % 2 != 0 && maxvcpus != 1)) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Expecting domain XML entry 'vcpu' to be an unsigned " - "integer (1 or a multiple of 2) but found %d"), - def->maxvcpus); + _("Expecting domain XML entry 'vcpu' to be 1 or a " + "multiple of 2 but found %d"), + maxvcpus); goto cleanup; } - virBufferAsprintf(&buffer, "numvcpus = \"%d\"\n", def->maxvcpus); + virBufferAsprintf(&buffer, "numvcpus = \"%d\"\n", maxvcpus); /* def:cpumask -> vmx:sched.cpu.affinity */ if (def->cpumask && virBitmapSize(def->cpumask) > 0) { @@ -3202,11 +3204,11 @@ virVMXFormatConfig(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virDomainDe while ((bit = virBitmapNextSetBit(def->cpumask, bit)) >= 0) ++sched_cpu_affinity_length; - if (sched_cpu_affinity_length < def->maxvcpus) { + if (sched_cpu_affinity_length < maxvcpus) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Expecting domain XML attribute 'cpuset' of entry " "'vcpu' to contain at least %d CPU(s)"), - def->maxvcpus); + maxvcpus); goto cleanup; } diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index ea1090a..994720c 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -1372,7 +1372,7 @@ vzDomainGetVcpusFlags(virDomainPtr dom, goto cleanup; if (flags & VIR_DOMAIN_VCPU_MAXIMUM) - ret = privdom->def->maxvcpus; + ret = virDomainDefGetVcpusMax(privdom->def); else ret = privdom->def->vcpus; diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index bf8306a..a81a7d6 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -695,7 +695,8 @@ xenXMDomainSetVcpusFlags(virConnectPtr conn, /* Can't specify a current larger than stored maximum; but * reducing maximum can silently reduce current. */ if (!(flags & VIR_DOMAIN_VCPU_MAXIMUM)) - max = entry->def->maxvcpus; + max = virDomainDefGetVcpusMax(entry->def); + if (vcpus > max) { virReportError(VIR_ERR_INVALID_ARG, _("requested vcpus is greater than max allowable" @@ -760,8 +761,10 @@ xenXMDomainGetVcpusFlags(virConnectPtr conn, if (!(entry = virHashLookup(priv->configCache, filename))) goto cleanup; - ret = ((flags & VIR_DOMAIN_VCPU_MAXIMUM) ? entry->def->maxvcpus - : entry->def->vcpus); + if (flags & VIR_DOMAIN_VCPU_MAXIMUM) + ret = virDomainDefGetVcpusMax(entry->def); + else + ret = entry->def->vcpus; cleanup: xenUnifiedUnlock(priv); diff --git a/src/xenapi/xenapi_utils.c b/src/xenapi/xenapi_utils.c index a80e084..7ceb56a 100644 --- a/src/xenapi/xenapi_utils.c +++ b/src/xenapi/xenapi_utils.c @@ -504,8 +504,8 @@ createVMRecordFromXml(virConnectPtr conn, virDomainDefPtr def, else (*record)->memory_dynamic_max = (*record)->memory_static_max; - if (def->maxvcpus) { - (*record)->vcpus_max = (int64_t) def->maxvcpus; + if (virDomainDefGetVcpusMax(def) > 0) { + (*record)->vcpus_max = (int64_t) virDomainDefGetVcpusMax(def); (*record)->vcpus_at_startup = (int64_t) def->vcpus; } if (def->onPoweroff) diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index 878d1ae..26e2d0d 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -508,7 +508,7 @@ xenParseCPUFeatures(virConfPtr conf, virDomainDefPtr def) if (xenConfigGetULong(conf, "vcpu_avail", &count, -1) < 0) return -1; - def->vcpus = MIN(count_one_bits_l(count), def->maxvcpus); + def->vcpus = MIN(count_one_bits_l(count), virDomainDefGetVcpusMax(def)); if (xenConfigGetString(conf, "cpus", &str, NULL) < 0) return -1; @@ -1526,7 +1526,7 @@ xenFormatCPUAllocation(virConfPtr conf, virDomainDefPtr def) int ret = -1; char *cpus = NULL; - if (xenConfigSetInt(conf, "vcpus", def->maxvcpus) < 0) + if (xenConfigSetInt(conf, "vcpus", virDomainDefGetVcpusMax(def)) < 0) goto cleanup; /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c index 7799f9e..5386728 100644 --- a/src/xenconfig/xen_sxpr.c +++ b/src/xenconfig/xen_sxpr.c @@ -1176,8 +1176,8 @@ xenParseSxpr(const struct sexpr *root, if (virDomainDefSetVcpusMax(def, sexpr_int(root, "domain/vcpus")) < 0) goto error; def->vcpus = count_one_bits_l(sexpr_u64(root, "domain/vcpu_avail")); - if (!def->vcpus || def->maxvcpus < def->vcpus) - def->vcpus = def->maxvcpus; + if (!def->vcpus || virDomainDefGetVcpusMax(def) < def->vcpus) + def->vcpus = virDomainDefGetVcpusMax(def); tmp = sexpr_node(root, "domain/on_poweroff"); if (tmp != NULL) { @@ -2223,7 +2223,7 @@ xenFormatSxpr(virConnectPtr conn, virBufferAsprintf(&buf, "(memory %llu)(maxmem %llu)", VIR_DIV_UP(def->mem.cur_balloon, 1024), VIR_DIV_UP(virDomainDefGetMemoryActual(def), 1024)); - virBufferAsprintf(&buf, "(vcpus %u)", def->maxvcpus); + virBufferAsprintf(&buf, "(vcpus %u)", virDomainDefGetVcpusMax(def)); /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is either 32, or 64 on a platform where long is big enough. */ if (virDomainDefHasVcpusOffline(def)) @@ -2307,7 +2307,7 @@ xenFormatSxpr(virConnectPtr conn, else virBufferEscapeSexpr(&buf, "(kernel '%s')", def->os.loader->path); - virBufferAsprintf(&buf, "(vcpus %u)", def->maxvcpus); + virBufferAsprintf(&buf, "(vcpus %u)", virDomainDefGetVcpusMax(def)); if (virDomainDefHasVcpusOffline(def)) virBufferAsprintf(&buf, "(vcpu_avail %lu)", (1UL << def->vcpus) - 1); -- 2.6.2

--- src/conf/domain_conf.c | 20 +++++++++++++++++--- src/conf/domain_conf.h | 1 + src/hyperv/hyperv_driver.c | 5 ++++- src/libvirt_private.syms | 1 + src/libxl/libxl_driver.c | 14 +++++++++----- src/lxc/lxc_native.c | 3 ++- src/openvz/openvz_conf.c | 3 ++- src/openvz/openvz_driver.c | 4 +++- src/phyp/phyp_driver.c | 3 ++- src/qemu/qemu_command.c | 12 ++++++++---- src/qemu/qemu_driver.c | 8 +++++--- src/test/test_driver.c | 8 +++++--- src/vbox/vbox_common.c | 6 ++++-- src/vmx/vmx.c | 3 ++- src/vz/vz_sdk.c | 3 ++- src/xen/xm_internal.c | 3 ++- src/xenapi/xenapi_driver.c | 3 ++- src/xenconfig/xen_common.c | 5 ++++- src/xenconfig/xen_sxpr.c | 11 ++++++++--- 19 files changed, 83 insertions(+), 33 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7d7ace9..ab2048b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1435,7 +1435,7 @@ virDomainDefSetVcpusMax(virDomainDefPtr def, unsigned int maxvcpus) { if (maxvcpus < def->vcpus) - def->vcpus = maxvcpus; + virDomainDefSetVcpus(def, maxvcpus); def->maxvcpus = maxvcpus; @@ -1457,6 +1457,16 @@ virDomainDefGetVcpusMax(const virDomainDef *def) } +int +virDomainDefSetVcpus(virDomainDefPtr def, + unsigned int vcpus) +{ + def->vcpus = vcpus; + + return 0; +} + + virDomainDiskDefPtr virDomainDiskDefNew(virDomainXMLOptionPtr xmlopt) { @@ -14719,6 +14729,7 @@ virDomainVcpuParse(virDomainDefPtr def, int n; char *tmp = NULL; unsigned int maxvcpus; + unsigned int vcpus; int ret = -1; if ((n = virXPathUInt("string(./vcpu[1])", ctxt, &maxvcpus)) < 0) { @@ -14734,16 +14745,19 @@ virDomainVcpuParse(virDomainDefPtr def, if (virDomainDefSetVcpusMax(def, maxvcpus) < 0) goto cleanup; - if ((n = virXPathUInt("string(./vcpu[1]/@current)", ctxt, &def->vcpus)) < 0) { + if ((n = virXPathUInt("string(./vcpu[1]/@current)", ctxt, &vcpus)) < 0) { if (n == -2) { virReportError(VIR_ERR_XML_ERROR, "%s", _("current vcpus count must be an integer")); goto cleanup; } - def->vcpus = maxvcpus; + vcpus = maxvcpus; } + if (virDomainDefSetVcpus(def, vcpus) < 0) + goto cleanup; + if (maxvcpus < def->vcpus) { virReportError(VIR_ERR_INTERNAL_ERROR, _("maxvcpus must not be less than current vcpus " diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2b2952a..bab8a5e 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2340,6 +2340,7 @@ struct _virDomainDef { int virDomainDefSetVcpusMax(virDomainDefPtr def, unsigned int vcpus); bool virDomainDefHasVcpusOffline(const virDomainDef *def); unsigned int virDomainDefGetVcpusMax(const virDomainDef *def); +int virDomainDefSetVcpus(virDomainDefPtr def, unsigned int vcpus); unsigned long long virDomainDefGetMemoryInitial(const virDomainDef *def); void virDomainDefSetMemoryTotal(virDomainDefPtr def, unsigned long long size); diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 1e8db03..52e9e45 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -877,7 +877,10 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) processorSettingData->data->VirtualQuantity) < 0) goto cleanup; - def->vcpus = processorSettingData->data->VirtualQuantity; + if (virDomainDefSetVcpus(def, + processorSettingData->data->VirtualQuantity) < 0) + goto cleanup; + def->os.type = VIR_DOMAIN_OSTYPE_HVM; /* FIXME: devices section is totally missing */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index bba42c9..d9a0d4b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -232,6 +232,7 @@ virDomainDefParseString; virDomainDefPostParse; virDomainDefSetMemoryInitial; virDomainDefSetMemoryTotal; +virDomainDefSetVcpus; virDomainDefSetVcpusMax; virDomainDeleteConfig; virDomainDeviceAddressIsValid; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 217957c..905b392 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -555,7 +555,8 @@ libxlAddDom0(libxlDriverPrivatePtr driver) if (virDomainDefSetVcpusMax(vm->def, d_info.vcpu_max_id + 1)) goto cleanup; - vm->def->vcpus = d_info.vcpu_online; + if (virDomainDefSetVcpus(vm->def, d_info.vcpu_online) < 0) + goto cleanup; vm->def->mem.cur_balloon = d_info.current_memkb; virDomainDefSetMemoryTotal(vm->def, d_info.max_memkb); @@ -2191,7 +2192,8 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, break; case VIR_DOMAIN_VCPU_CONFIG: - def->vcpus = nvcpus; + if (virDomainDefSetVcpus(def, nvcpus) < 0) + goto cleanup; break; case VIR_DOMAIN_VCPU_LIVE: @@ -2201,7 +2203,8 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, " with libxenlight"), vm->def->id); goto endjob; } - vm->def->vcpus = nvcpus; + if (virDomainDefSetVcpus(vm->def, nvcpus) < 0) + goto endjob; break; case VIR_DOMAIN_VCPU_LIVE | VIR_DOMAIN_VCPU_CONFIG: @@ -2211,8 +2214,9 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, " with libxenlight"), vm->def->id); goto endjob; } - vm->def->vcpus = nvcpus; - def->vcpus = nvcpus; + if (virDomainDefSetVcpus(vm->def, nvcpus) < 0 || + virDomainDefSetVcpus(def, nvcpus) < 0) + goto endjob; break; } diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index 1c65475..5dd3db4 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -1022,7 +1022,8 @@ lxcParseConfigString(const char *config) if (virDomainDefSetVcpusMax(vmdef, 1) < 0) goto error; - vmdef->vcpus = 1; + if (virDomainDefSetVcpus(vmdef, 1) < 0) + goto error; vmdef->nfss = 0; vmdef->os.type = VIR_DOMAIN_OSTYPE_EXE; diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 6629105..e32dd6f 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -585,7 +585,8 @@ int openvzLoadDomains(struct openvz_driver *driver) if (virDomainDefSetVcpusMax(def, vcpus) < 0) goto cleanup; - def->vcpus = vcpus; + if (virDomainDefSetVcpus(def, vcpus) < 0) + goto cleanup; /* XXX load rest of VM config data .... */ diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 5e55033..2a9e6ac 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1371,7 +1371,9 @@ static int openvzDomainSetVcpusInternal(virDomainObjPtr vm, if (virDomainDefSetVcpusMax(vm->def, nvcpus) < 0) return -1; - vm->def->vcpus = nvcpus; + if (virDomainDefSetVcpus(vm->def, nvcpus) < 0) + return -1; + return 0; } diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index b60e5ba..3d086e3 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -3298,7 +3298,8 @@ phypDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) if (virDomainDefSetVcpusMax(&def, vcpus) < 0) goto err; - def.vcpus = vcpus; + if (virDomainDefSetVcpus(&def, vcpus) < 0) + goto err; return virDomainDefFormat(&def, virDomainDefFormatConvertXMLFlags(flags)); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0a0bc3b..ccea41b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -12607,6 +12607,7 @@ qemuParseCommandLineSmp(virDomainDefPtr dom, unsigned int cores = 0; unsigned int threads = 0; unsigned int maxcpus = 0; + unsigned int vcpus = 0; size_t i; int nkws; char **kws; @@ -12621,9 +12622,8 @@ qemuParseCommandLineSmp(virDomainDefPtr dom, for (i = 0; i < nkws; i++) { if (vals[i] == NULL) { if (i > 0 || - virStrToLong_i(kws[i], &end, 10, &n) < 0 || *end != '\0') + virStrToLong_ui(kws[i], &end, 10, &vcpus) < 0 || *end != '\0') goto syntax; - dom->vcpus = n; } else { if (virStrToLong_i(vals[i], &end, 10, &n) < 0 || *end != '\0') goto syntax; @@ -12641,11 +12641,14 @@ qemuParseCommandLineSmp(virDomainDefPtr dom, } if (maxcpus == 0) - maxcpus = dom->vcpus; + maxcpus = vcpus; if (virDomainDefSetVcpusMax(dom, maxcpus) < 0) goto error; + if (virDomainDefSetVcpus(dom, vcpus) < 0) + goto error; + if (sockets && cores && threads) { virCPUDefPtr cpu; @@ -12760,7 +12763,8 @@ qemuParseCommandLine(virCapsPtr qemuCaps, virDomainDefSetMemoryTotal(def, def->mem.cur_balloon); if (virDomainDefSetVcpusMax(def, 1) < 0) goto error; - def->vcpus = 1; + if (virDomainDefSetVcpus(def, 1) < 0) + goto error; def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_UTC; def->onReboot = VIR_DOMAIN_LIFECYCLE_RESTART; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6013443..f07d7a7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4835,8 +4835,9 @@ qemuDomainHotplugVcpus(virQEMUDriverPtr driver, cleanup: VIR_FREE(cpupids); VIR_FREE(mem_mask); - if (virDomainObjIsActive(vm)) - vm->def->vcpus = vcpus; + if (virDomainObjIsActive(vm) && + virDomainDefSetVcpus(vm->def, vcpus) < 0) + ret = -1; virDomainAuditVcpu(vm, oldvcpus, nvcpus, "update", rc == 1); if (cgroup_vcpu) virCgroupFree(&cgroup_vcpu); @@ -4982,7 +4983,8 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, if (virDomainDefSetVcpusMax(persistentDef, nvcpus) < 0) goto endjob; } else { - persistentDef->vcpus = nvcpus; + if (virDomainDefSetVcpus(persistentDef, nvcpus) < 0) + goto endjob; } if (virDomainSaveConfig(cfg->configDir, persistentDef) < 0) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 71b4513..c9e3eba 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2374,15 +2374,17 @@ testDomainSetVcpusFlags(virDomainPtr domain, unsigned int nrCpus, goto cleanup; } - if (def) - def->vcpus = nrCpus; + if (def && + virDomainDefSetVcpus(def, nrCpus) < 0) + goto cleanup; if (persistentDef) { if (flags & VIR_DOMAIN_VCPU_MAXIMUM) { if (virDomainDefSetVcpusMax(persistentDef, nrCpus) < 0) goto cleanup; } else { - persistentDef->vcpus = nrCpus; + if (virDomainDefSetVcpus(persistentDef, nrCpus) < 0) + goto cleanup; } } diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index e2e8ebc..087d2e2 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -3904,7 +3904,8 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) if (virDomainDefSetVcpusMax(def, CPUCount) < 0) goto cleanup; - def->vcpus = CPUCount; + if (virDomainDefSetVcpus(def, CPUCount) < 0) + goto cleanup; /* Skip cpumasklen, cpumask, onReboot, onPoweroff, onCrash */ @@ -6061,7 +6062,8 @@ static char *vboxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, if (virDomainDefSetVcpusMax(def->dom, CPUCount) < 0) goto cleanup; - def->dom->vcpus = CPUCount; + if (virDomainDefSetVcpus(def->dom, CPUCount) < 0) + goto cleanup; if (vboxSnapshotGetReadWriteDisks(def, snapshot) < 0) VIR_DEBUG("Could not get read write disks for snapshot"); diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 4b94df2..bcc4034 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -1460,7 +1460,8 @@ virVMXParseConfig(virVMXContext *ctx, if (virDomainDefSetVcpusMax(def, numvcpus) < 0) goto cleanup; - def->vcpus = numvcpus; + if (virDomainDefSetVcpus(def, numvcpus) < 0) + goto cleanup; /* vmx:sched.cpu.affinity -> def:cpumask */ /* NOTE: maps to VirtualMachine:config.cpuAffinity.affinitySet */ diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index c90220c..890dd52 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1153,7 +1153,8 @@ prlsdkConvertCpuInfo(PRL_HANDLE sdkdom, if (virDomainDefSetVcpusMax(def, cpuCount) < 0) goto cleanup; - def->vcpus = cpuCount; + if (virDomainDefSetVcpus(def, cpuCount) < 0) + goto cleanup; pret = PrlVmCfg_GetCpuMask(sdkdom, NULL, &buflen); prlsdkCheckRetGoto(pret, cleanup); diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index a81a7d6..4a7d0de 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -708,7 +708,8 @@ xenXMDomainSetVcpusFlags(virConnectPtr conn, if (virDomainDefSetVcpusMax(entry->def, vcpus) < 0) goto cleanup; } else { - entry->def->vcpus = vcpus; + if (virDomainDefSetVcpus(entry->def, vcpus) < 0) + goto cleanup; } /* If this fails, should we try to undo our changes to the diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index fa66bb1..e4e9936 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -1505,7 +1505,8 @@ xenapiDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) if (virDomainDefSetVcpusMax(defPtr, vcpus) < 0) goto error; - defPtr->vcpus = vcpus; + if (virDomainDefSetVcpus(defPtr, vcpus) < 0) + goto error; enum xen_on_normal_exit action; if (xen_vm_get_actions_after_shutdown(session, &action, vm)) diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index 26e2d0d..d11a919 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -508,7 +508,10 @@ xenParseCPUFeatures(virConfPtr conf, virDomainDefPtr def) if (xenConfigGetULong(conf, "vcpu_avail", &count, -1) < 0) return -1; - def->vcpus = MIN(count_one_bits_l(count), virDomainDefGetVcpusMax(def)); + if (virDomainDefSetVcpus(def, MIN(count_one_bits_l(count), + virDomainDefGetVcpusMax(def))) < 0) + return -1; + if (xenConfigGetString(conf, "cpus", &str, NULL) < 0) return -1; diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c index 5386728..28b8e4e 100644 --- a/src/xenconfig/xen_sxpr.c +++ b/src/xenconfig/xen_sxpr.c @@ -1092,6 +1092,7 @@ xenParseSxpr(const struct sexpr *root, const char *tmp; virDomainDefPtr def; int hvm = 0, vmlocaltime; + unsigned int vcpus; if (!(def = virDomainDefNew())) goto error; @@ -1175,9 +1176,13 @@ xenParseSxpr(const struct sexpr *root, if (virDomainDefSetVcpusMax(def, sexpr_int(root, "domain/vcpus")) < 0) goto error; - def->vcpus = count_one_bits_l(sexpr_u64(root, "domain/vcpu_avail")); - if (!def->vcpus || virDomainDefGetVcpusMax(def) < def->vcpus) - def->vcpus = virDomainDefGetVcpusMax(def); + + vcpus = count_one_bits_l(sexpr_u64(root, "domain/vcpu_avail")); + if (!vcpus || virDomainDefGetVcpusMax(def) < vcpus) + vcpus = virDomainDefGetVcpusMax(def); + + if (virDomainDefSetVcpus(def, vcpus) < 0) + goto error; tmp = sexpr_node(root, "domain/on_poweroff"); if (tmp != NULL) { -- 2.6.2

--- src/conf/domain_conf.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ab2048b..a2cb894 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1461,6 +1461,13 @@ int virDomainDefSetVcpus(virDomainDefPtr def, unsigned int vcpus) { + if (vcpus > def->maxvcpus) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("maxvcpus must not be less than current vcpus (%u < %u)"), + vcpus, def->maxvcpus); + return -1; + } + def->vcpus = vcpus; return 0; @@ -14758,13 +14765,6 @@ virDomainVcpuParse(virDomainDefPtr def, if (virDomainDefSetVcpus(def, vcpus) < 0) goto cleanup; - if (maxvcpus < def->vcpus) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("maxvcpus must not be less than current vcpus " - "(%u < %u)"), maxvcpus, def->vcpus); - goto cleanup; - } - tmp = virXPathString("string(./vcpu[1]/@placement)", ctxt); if (tmp) { if ((def->placement_mode = -- 2.6.2

--- src/bhyve/bhyve_command.c | 2 +- src/bhyve/bhyve_driver.c | 2 +- src/conf/domain_audit.c | 2 +- src/conf/domain_conf.c | 19 +++++++++++++------ src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + src/libxl/libxl_conf.c | 2 +- src/libxl/libxl_driver.c | 8 ++++---- src/lxc/lxc_controller.c | 2 +- src/lxc/lxc_driver.c | 2 +- src/openvz/openvz_driver.c | 2 +- src/phyp/phyp_driver.c | 5 +++-- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_driver.c | 36 ++++++++++++++++++------------------ src/qemu/qemu_process.c | 10 +++++----- src/test/test_driver.c | 14 +++++++------- src/uml/uml_driver.c | 2 +- src/vmware/vmware_driver.c | 2 +- src/vmx/vmx.c | 14 ++++++++------ src/vz/vz_driver.c | 6 +++--- src/vz/vz_sdk.c | 4 ++-- src/xen/xm_internal.c | 4 ++-- src/xenapi/xenapi_utils.c | 2 +- src/xenconfig/xen_common.c | 3 ++- src/xenconfig/xen_sxpr.c | 5 +++-- 25 files changed, 83 insertions(+), 69 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 6576029..5f3055d 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -232,7 +232,7 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, /* CPUs */ virCommandAddArg(cmd, "-c"); - virCommandAddArgFormat(cmd, "%d", def->vcpus); + virCommandAddArgFormat(cmd, "%d", virDomainDefGetVcpus(def)); /* Memory */ virCommandAddArg(cmd, "-m"); diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 4840a0e..0289e4a 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -304,7 +304,7 @@ bhyveDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) info->state = virDomainObjGetState(vm, NULL); info->maxMem = virDomainDefGetMemoryActual(vm->def); - info->nrVirtCpu = vm->def->vcpus; + info->nrVirtCpu = virDomainDefGetVcpus(vm->def); ret = 0; cleanup: diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index b842495..bd2eeb6 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -885,7 +885,7 @@ virDomainAuditStart(virDomainObjPtr vm, const char *reason, bool success) virDomainAuditMemory(vm, 0, virDomainDefGetMemoryActual(vm->def), "start", true); - virDomainAuditVcpu(vm, 0, vm->def->vcpus, "start", true); + virDomainAuditVcpu(vm, 0, virDomainDefGetVcpus(vm->def), "start", true); if (vm->def->niothreadids) virDomainAuditIOThread(vm, 0, vm->def->niothreadids, "start", true); diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a2cb894..890e1c7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1474,6 +1474,13 @@ virDomainDefSetVcpus(virDomainDefPtr def, } +unsigned int +virDomainDefGetVcpus(const virDomainDef *def) +{ + return def->vcpus; +} + + virDomainDiskDefPtr virDomainDiskDefNew(virDomainXMLOptionPtr xmlopt) { @@ -15235,7 +15242,7 @@ virDomainDefParseXML(xmlDocPtr xml, goto error; } - if (vcpupin->id >= def->vcpus) { + if (vcpupin->id >= virDomainDefGetVcpus(def)) { /* To avoid the regression when daemon loading * domain confs, we can't simply error out if * <vcpupin> nodes greater than current vcpus, @@ -15253,10 +15260,10 @@ virDomainDefParseXML(xmlDocPtr xml, * the policy specified explicitly as def->cpuset. */ if (def->cpumask) { - if (VIR_REALLOC_N(def->cputune.vcpupin, def->vcpus) < 0) + if (VIR_REALLOC_N(def->cputune.vcpupin, virDomainDefGetVcpus(def)) < 0) goto error; - for (i = 0; i < def->vcpus; i++) { + for (i = 0; i < virDomainDefGetVcpus(def); i++) { if (virDomainPinIsDuplicate(def->cputune.vcpupin, def->cputune.nvcpupin, i)) @@ -17879,10 +17886,10 @@ virDomainDefCheckABIStability(virDomainDefPtr src, goto error; } - if (src->vcpus != dst->vcpus) { + if (virDomainDefGetVcpus(src) != virDomainDefGetVcpus(dst)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target domain vCPU count %d does not match source %d"), - dst->vcpus, src->vcpus); + virDomainDefGetVcpus(dst), virDomainDefGetVcpus(src)); goto error; } if (virDomainDefGetVcpusMax(src) != virDomainDefGetVcpusMax(dst)) { @@ -21851,7 +21858,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, VIR_FREE(cpumask); } if (virDomainDefHasVcpusOffline(def)) - virBufferAsprintf(buf, " current='%u'", def->vcpus); + virBufferAsprintf(buf, " current='%u'", virDomainDefGetVcpus(def)); virBufferAsprintf(buf, ">%u</vcpu>\n", virDomainDefGetVcpusMax(def)); if (def->niothreadids > 0) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index bab8a5e..d7630c4 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2341,6 +2341,7 @@ int virDomainDefSetVcpusMax(virDomainDefPtr def, unsigned int vcpus); bool virDomainDefHasVcpusOffline(const virDomainDef *def); unsigned int virDomainDefGetVcpusMax(const virDomainDef *def); int virDomainDefSetVcpus(virDomainDefPtr def, unsigned int vcpus); +unsigned int virDomainDefGetVcpus(const virDomainDef *def); unsigned long long virDomainDefGetMemoryInitial(const virDomainDef *def); void virDomainDefSetMemoryTotal(virDomainDefPtr def, unsigned long long size); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d9a0d4b..6fb36c3 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -217,6 +217,7 @@ virDomainDefGetDefaultEmulator; virDomainDefGetMemoryActual; virDomainDefGetMemoryInitial; virDomainDefGetSecurityLabelDef; +virDomainDefGetVcpus; virDomainDefGetVcpusMax; virDomainDefHasDeviceAddress; virDomainDefHasMemoryHotplug; diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 35089ea..23c74e7 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -647,7 +647,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, if (libxl_cpu_bitmap_alloc(ctx, &b_info->avail_vcpus, b_info->max_vcpus)) return -1; libxl_bitmap_set_none(&b_info->avail_vcpus); - for (i = 0; i < def->vcpus; i++) + for (i = 0; i < virDomainDefGetVcpus(def); i++) libxl_bitmap_set((&b_info->avail_vcpus), i); if (def->clock.ntimers > 0 && diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 905b392..1c3da06 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1601,7 +1601,7 @@ libxlDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) } info->state = virDomainObjGetState(vm, NULL); - info->nrVirtCpu = vm->def->vcpus; + info->nrVirtCpu = virDomainDefGetVcpus(vm->def); ret = 0; cleanup: @@ -2304,7 +2304,7 @@ libxlDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) if (flags & VIR_DOMAIN_VCPU_MAXIMUM) ret = virDomainDefGetVcpusMax(def); else - ret = def->vcpus; + ret = virDomainDefGetVcpus(def); cleanup: if (vm) @@ -2441,8 +2441,8 @@ libxlDomainGetVcpuPinInfo(virDomainPtr dom, int ncpumaps, sa_assert(targetDef); /* Clamp to actual number of vcpus */ - if (ncpumaps > targetDef->vcpus) - ncpumaps = targetDef->vcpus; + if (ncpumaps > virDomainDefGetVcpus(targetDef)) + ncpumaps = virDomainDefGetVcpus(targetDef); if ((hostcpus = libxl_get_max_cpus(cfg->ctx)) < 0) goto cleanup; diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 3e5d2b4..438103a 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -771,7 +771,7 @@ static int virLXCControllerGetNumadAdvice(virLXCControllerPtr ctrl, * either <vcpu> or <numatune> is 'auto'. */ if (virDomainDefNeedsPlacementAdvice(ctrl->def)) { - nodeset = virNumaGetAutoPlacementAdvice(ctrl->def->vcpus, + nodeset = virNumaGetAutoPlacementAdvice(virDomainDefGetVcpus(ctrl->def), ctrl->def->mem.cur_balloon); if (!nodeset) goto cleanup; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 1a9550e..567dc1d 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -617,7 +617,7 @@ static int lxcDomainGetInfo(virDomainPtr dom, } info->maxMem = virDomainDefGetMemoryActual(vm->def); - info->nrVirtCpu = vm->def->vcpus; + info->nrVirtCpu = virDomainDefGetVcpus(vm->def); ret = 0; cleanup: diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 2a9e6ac..3a4a342 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -465,7 +465,7 @@ static int openvzDomainGetInfo(virDomainPtr dom, info->maxMem = virDomainDefGetMemoryActual(vm->def); info->memory = vm->def->mem.cur_balloon; - info->nrVirtCpu = vm->def->vcpus; + info->nrVirtCpu = virDomainDefGetVcpus(vm->def); ret = 0; cleanup: diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 3d086e3..cf674a0 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -3525,11 +3525,12 @@ phypBuildLpar(virConnectPtr conn, virDomainDefPtr def) if (system_type == HMC) virBufferAsprintf(&buf, " -m %s", managed_system); virBufferAsprintf(&buf, " -r lpar -p %s -i min_mem=%lld,desired_mem=%lld," - "max_mem=%lld,desired_procs=%d,virtual_scsi_adapters=%s", + "max_mem=%lld,desired_procs=%u,virtual_scsi_adapters=%s", def->name, def->mem.cur_balloon, def->mem.cur_balloon, virDomainDefGetMemoryInitial(def), - (int) def->vcpus, virDomainDiskGetSource(def->disks[0])); + virDomainDefGetVcpus(def), + virDomainDiskGetSource(def->disks[0])); ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0) { diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ccea41b..7ecaa66 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7863,7 +7863,7 @@ qemuBuildSmpArgStr(const virDomainDef *def, { virBuffer buf = VIR_BUFFER_INITIALIZER; - virBufferAsprintf(&buf, "%u", def->vcpus); + virBufferAsprintf(&buf, "%u", virDomainDefGetVcpus(def)); if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SMP_TOPOLOGY)) { if (virDomainDefHasVcpusOffline(def)) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f07d7a7..cfcc789 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2659,7 +2659,7 @@ qemuDomainGetInfo(virDomainPtr dom, } } - if (VIR_ASSIGN_IS_OVERFLOW(info->nrVirtCpu, vm->def->vcpus)) { + if (VIR_ASSIGN_IS_OVERFLOW(info->nrVirtCpu, virDomainDefGetVcpus(vm->def))) { virReportError(VIR_ERR_OVERFLOW, "%s", _("cpu count too large")); goto cleanup; } @@ -4700,7 +4700,7 @@ qemuDomainHotplugVcpus(virQEMUDriverPtr driver, size_t i; int rc = 1; int ret = -1; - int oldvcpus = vm->def->vcpus; + int oldvcpus = virDomainDefGetVcpus(vm->def); int vcpus = oldvcpus; pid_t *cpupids = NULL; int ncpupids; @@ -4929,11 +4929,11 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, if (!qemuDomainAgentAvailable(vm, true)) goto endjob; - if (nvcpus > vm->def->vcpus) { + if (nvcpus > virDomainDefGetVcpus(vm->def)) { virReportError(VIR_ERR_INVALID_ARG, _("requested vcpu count is greater than the count " "of enabled vcpus in the domain: %d > %d"), - nvcpus, vm->def->vcpus); + nvcpus, virDomainDefGetVcpus(vm->def)); goto endjob; } @@ -4972,8 +4972,8 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, if (persistentDef) { /* remove vcpupin entries for vcpus that were unplugged */ - if (nvcpus < persistentDef->vcpus) { - for (i = persistentDef->vcpus - 1; i >= nvcpus; i--) + if (nvcpus < virDomainDefGetVcpus(persistentDef)) { + for (i = virDomainDefGetVcpus(persistentDef) - 1; i >= nvcpus; i--) virDomainPinDel(&persistentDef->cputune.vcpupin, &persistentDef->cputune.nvcpupin, i); @@ -5067,17 +5067,17 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, priv = vm->privateData; - if (def && vcpu >= def->vcpus) { + if (def && vcpu >= virDomainDefGetVcpus(def)) { virReportError(VIR_ERR_INVALID_ARG, _("vcpu %d is out of range of live cpu count %d"), - vcpu, def->vcpus); + vcpu, virDomainDefGetVcpus(def)); goto endjob; } - if (persistentDef && vcpu >= persistentDef->vcpus) { + if (persistentDef && vcpu >= virDomainDefGetVcpus(persistentDef)) { virReportError(VIR_ERR_INVALID_ARG, _("vcpu %d is out of range of persistent cpu count %d"), - vcpu, persistentDef->vcpus); + vcpu, virDomainDefGetVcpus(persistentDef)); goto endjob; } @@ -5246,8 +5246,8 @@ qemuDomainGetVcpuPinInfo(virDomainPtr dom, priv = vm->privateData; /* Clamp to actual number of vcpus */ - if (ncpumaps > def->vcpus) - ncpumaps = def->vcpus; + if (ncpumaps > virDomainDefGetVcpus(def)) + ncpumaps = virDomainDefGetVcpus(def); if (ncpumaps < 1) goto cleanup; @@ -5561,7 +5561,7 @@ qemuDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) if (flags & VIR_DOMAIN_VCPU_MAXIMUM) ret = virDomainDefGetVcpusMax(def); else - ret = def->vcpus; + ret = virDomainDefGetVcpus(def); } @@ -10587,7 +10587,7 @@ qemuGetVcpusBWLive(virDomainObjPtr vm, goto cleanup; if (*quota > 0) - *quota /= vm->def->vcpus; + *quota /= virDomainDefGetVcpus(vm->def); goto out; } @@ -19073,7 +19073,7 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, &record->nparams, maxparams, "vcpu.current", - (unsigned) dom->def->vcpus) < 0) + virDomainDefGetVcpus(dom->def)) < 0) return -1; if (virTypedParamsAddUInt(&record->params, @@ -19083,17 +19083,17 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, virDomainDefGetVcpusMax(dom->def)) < 0) return -1; - if (VIR_ALLOC_N(cpuinfo, dom->def->vcpus) < 0) + if (VIR_ALLOC_N(cpuinfo, virDomainDefGetVcpus(dom->def)) < 0) return -1; - if (qemuDomainHelperGetVcpus(dom, cpuinfo, dom->def->vcpus, + if (qemuDomainHelperGetVcpus(dom, cpuinfo, virDomainDefGetVcpus(dom->def), NULL, 0) < 0) { virResetLastError(); ret = 0; /* it's ok to be silent and go ahead */ goto cleanup; } - for (i = 0; i < dom->def->vcpus; i++) { + for (i = 0; i < virDomainDefGetVcpus(dom->def); i++) { snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, "vcpu.%zu.state", i); if (virTypedParamsAddInt(&record->params, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a2d2bfd..a3ddb4a 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2021,11 +2021,11 @@ qemuProcessDetectVcpuPIDs(virQEMUDriverPtr driver, return 0; } - if (ncpupids != vm->def->vcpus) { + if (ncpupids != virDomainDefGetVcpus(vm->def)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("got wrong number of vCPU pids from QEMU monitor. " "got %d, wanted %d"), - ncpupids, vm->def->vcpus); + ncpupids, virDomainDefGetVcpus(vm->def)); VIR_FREE(cpupids); return -1; } @@ -2241,7 +2241,7 @@ qemuProcessSetVcpuAffinities(virDomainObjPtr vm) int n; int ret = -1; VIR_DEBUG("Setting affinity on CPUs nvcpupin=%zu nvcpus=%d nvcpupids=%d", - def->cputune.nvcpupin, def->vcpus, priv->nvcpupids); + def->cputune.nvcpupin, virDomainDefGetVcpus(def), priv->nvcpupids); if (!def->cputune.nvcpupin) return 0; @@ -2260,7 +2260,7 @@ qemuProcessSetVcpuAffinities(virDomainObjPtr vm) return 0; } - for (n = 0; n < def->vcpus; n++) { + for (n = 0; n < virDomainDefGetVcpus(def); n++) { /* set affinity only for existing vcpus */ if (!(pininfo = virDomainPinFind(def->cputune.vcpupin, def->cputune.nvcpupin, @@ -4667,7 +4667,7 @@ qemuProcessLaunch(virConnectPtr conn, * either <vcpu> or <numatune> is 'auto'. */ if (virDomainDefNeedsPlacementAdvice(vm->def)) { - nodeset = virNumaGetAutoPlacementAdvice(vm->def->vcpus, + nodeset = virNumaGetAutoPlacementAdvice(virDomainDefGetVcpus(vm->def), virDomainDefGetMemoryActual(vm->def)); if (!nodeset) goto cleanup; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index c9e3eba..f135365 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1927,7 +1927,7 @@ static int testDomainGetInfo(virDomainPtr domain, info->state = virDomainObjGetState(privdom, NULL); info->memory = privdom->def->mem.cur_balloon; info->maxMem = virDomainDefGetMemoryActual(privdom->def); - info->nrVirtCpu = privdom->def->vcpus; + info->nrVirtCpu = virDomainDefGetVcpus(privdom->def); info->cpuTime = ((tv.tv_sec * 1000ll * 1000ll * 1000ll) + (tv.tv_usec * 1000ll)); ret = 0; @@ -2315,7 +2315,7 @@ testDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags) if (flags & VIR_DOMAIN_VCPU_MAXIMUM) ret = virDomainDefGetVcpusMax(def); else - ret = def->vcpus; + ret = virDomainDefGetVcpus(def); cleanup: virDomainObjEndAPI(&vm); @@ -2447,8 +2447,8 @@ static int testDomainGetVcpus(virDomainPtr domain, virBitmapSetAll(allcpumap); /* Clamp to actual number of vcpus */ - if (maxinfo > privdom->def->vcpus) - maxinfo = privdom->def->vcpus; + if (maxinfo > virDomainDefGetVcpus(privdom->def)) + maxinfo = virDomainDefGetVcpus(privdom->def); memset(info, 0, sizeof(*info) * maxinfo); memset(cpumaps, 0, maxinfo * maplen); @@ -2506,7 +2506,7 @@ static int testDomainPinVcpu(virDomainPtr domain, goto cleanup; } - if (vcpu > privdom->def->vcpus) { + if (vcpu > virDomainDefGetVcpus(privdom->def)) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("requested vcpu is higher than allocated vcpus")); goto cleanup; @@ -2560,8 +2560,8 @@ testDomainGetVcpuPinInfo(virDomainPtr dom, virBitmapSetAll(allcpumap); /* Clamp to actual number of vcpus */ - if (ncpumaps > def->vcpus) - ncpumaps = def->vcpus; + if (ncpumaps > virDomainDefGetVcpus(def)) + ncpumaps = virDomainDefGetVcpus(def); for (vcpu = 0; vcpu < ncpumaps; vcpu++) { virDomainPinDefPtr pininfo; diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 14598fc..fc0ca67 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1916,7 +1916,7 @@ static int umlDomainGetInfo(virDomainPtr dom, info->maxMem = virDomainDefGetMemoryActual(vm->def); info->memory = vm->def->mem.cur_balloon; - info->nrVirtCpu = vm->def->vcpus; + info->nrVirtCpu = virDomainDefGetVcpus(vm->def); ret = 0; cleanup: diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index a12b03a..f23b67f 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -1142,7 +1142,7 @@ vmwareDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) info->cpuTime = 0; info->maxMem = virDomainDefGetMemoryActual(vm->def); info->memory = vm->def->mem.cur_balloon; - info->nrVirtCpu = vm->def->vcpus; + info->nrVirtCpu = virDomainDefGetVcpus(vm->def); ret = 0; cleanup: diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index bcc4034..b020b6f 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -1539,13 +1539,14 @@ virVMXParseConfig(virVMXContext *ctx, } if (sched_cpu_shares != NULL) { + unsigned int vcpus = virDomainDefGetVcpus(def); /* See http://www.vmware.com/support/developer/vc-sdk/visdk41pubs/ApiReference/vim.... */ if (STRCASEEQ(sched_cpu_shares, "low")) { - def->cputune.shares = def->vcpus * 500; + def->cputune.shares = vcpus * 500; } else if (STRCASEEQ(sched_cpu_shares, "normal")) { - def->cputune.shares = def->vcpus * 1000; + def->cputune.shares = vcpus * 1000; } else if (STRCASEEQ(sched_cpu_shares, "high")) { - def->cputune.shares = def->vcpus * 2000; + def->cputune.shares = vcpus * 2000; } else if (virStrToLong_ul(sched_cpu_shares, NULL, 10, &def->cputune.shares) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -3228,12 +3229,13 @@ virVMXFormatConfig(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virDomainDe /* def:cputune.shares -> vmx:sched.cpu.shares */ if (def->cputune.sharesSpecified) { + unsigned int vcpus = virDomainDefGetVcpus(def); /* See http://www.vmware.com/support/developer/vc-sdk/visdk41pubs/ApiReference/vim.... */ - if (def->cputune.shares == def->vcpus * 500) { + if (def->cputune.shares == vcpus * 500) { virBufferAddLit(&buffer, "sched.cpu.shares = \"low\"\n"); - } else if (def->cputune.shares == def->vcpus * 1000) { + } else if (def->cputune.shares == vcpus * 1000) { virBufferAddLit(&buffer, "sched.cpu.shares = \"normal\"\n"); - } else if (def->cputune.shares == def->vcpus * 2000) { + } else if (def->cputune.shares == vcpus * 2000) { virBufferAddLit(&buffer, "sched.cpu.shares = \"high\"\n"); } else { virBufferAsprintf(&buffer, "sched.cpu.shares = \"%lu\"\n", diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 994720c..2ef47e4 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -560,14 +560,14 @@ vzDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) info->state = virDomainObjGetState(privdom, NULL); info->memory = privdom->def->mem.cur_balloon; info->maxMem = virDomainDefGetMemoryActual(privdom->def); - info->nrVirtCpu = privdom->def->vcpus; + info->nrVirtCpu = virDomainDefGetVcpus(privdom->def); info->cpuTime = 0; if (virDomainObjIsActive(privdom)) { unsigned long long vtime; size_t i; - for (i = 0; i < privdom->def->vcpus; ++i) { + for (i = 0; i < virDomainDefGetVcpus(privdom->def); ++i) { if (prlsdkGetVcpuStats(privdom, i, &vtime) < 0) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("cannot read cputime for domain")); @@ -1374,7 +1374,7 @@ vzDomainGetVcpusFlags(virDomainPtr dom, if (flags & VIR_DOMAIN_VCPU_MAXIMUM) ret = virDomainDefGetVcpusMax(privdom->def); else - ret = privdom->def->vcpus; + ret = virDomainDefGetVcpus(privdom->def); cleanup: if (privdom) diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 890dd52..970c258 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1958,7 +1958,7 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom, virDomainDefPtr def) } if (def->cputune.vcpupin) { - for (i = 0; i < def->vcpus; i++) { + for (i = 0; i < virDomainDefGetVcpus(def); i++) { if (!virBitmapEqual(def->cpumask, def->cputune.vcpupin[i]->cpumask)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -3501,7 +3501,7 @@ prlsdkDoApplyConfig(virConnectPtr conn, pret = PrlVmCfg_SetRamSize(sdkdom, virDomainDefGetMemoryActual(def) >> 10); prlsdkCheckRetGoto(pret, error); - pret = PrlVmCfg_SetCpuCount(sdkdom, def->vcpus); + pret = PrlVmCfg_SetCpuCount(sdkdom, virDomainDefGetVcpus(def)); prlsdkCheckRetGoto(pret, error); if (!(mask = virBitmapFormat(def->cpumask))) diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 4a7d0de..0b7c71d 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -483,7 +483,7 @@ xenXMDomainGetInfo(virConnectPtr conn, memset(info, 0, sizeof(virDomainInfo)); info->maxMem = virDomainDefGetMemoryActual(entry->def); info->memory = entry->def->mem.cur_balloon; - info->nrVirtCpu = entry->def->vcpus; + info->nrVirtCpu = virDomainDefGetVcpus(entry->def); info->state = VIR_DOMAIN_SHUTOFF; info->cpuTime = 0; @@ -765,7 +765,7 @@ xenXMDomainGetVcpusFlags(virConnectPtr conn, if (flags & VIR_DOMAIN_VCPU_MAXIMUM) ret = virDomainDefGetVcpusMax(entry->def); else - ret = entry->def->vcpus; + ret = virDomainDefGetVcpus(entry->def); cleanup: xenUnifiedUnlock(priv); diff --git a/src/xenapi/xenapi_utils.c b/src/xenapi/xenapi_utils.c index 7ceb56a..5d6da36 100644 --- a/src/xenapi/xenapi_utils.c +++ b/src/xenapi/xenapi_utils.c @@ -506,7 +506,7 @@ createVMRecordFromXml(virConnectPtr conn, virDomainDefPtr def, if (virDomainDefGetVcpusMax(def) > 0) { (*record)->vcpus_max = (int64_t) virDomainDefGetVcpusMax(def); - (*record)->vcpus_at_startup = (int64_t) def->vcpus; + (*record)->vcpus_at_startup = (int64_t) virDomainDefGetVcpus(def); } if (def->onPoweroff) (*record)->actions_after_shutdown = actionShutdownLibvirt2XenapiEnum(def->onPoweroff); diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index d11a919..c8cb848 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -1535,7 +1535,8 @@ xenFormatCPUAllocation(virConfPtr conf, virDomainDefPtr def) /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is either 32, or 64 on a platform where long is big enough. */ if (virDomainDefHasVcpusOffline(def) && - xenConfigSetInt(conf, "vcpu_avail", (1UL << def->vcpus) - 1) < 0) + xenConfigSetInt(conf, "vcpu_avail", + (1UL << virDomainDefGetVcpus(def)) - 1) < 0) goto cleanup; if ((def->cpumask != NULL) && diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c index 28b8e4e..a6938c6 100644 --- a/src/xenconfig/xen_sxpr.c +++ b/src/xenconfig/xen_sxpr.c @@ -2232,7 +2232,8 @@ xenFormatSxpr(virConnectPtr conn, /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is either 32, or 64 on a platform where long is big enough. */ if (virDomainDefHasVcpusOffline(def)) - virBufferAsprintf(&buf, "(vcpu_avail %lu)", (1UL << def->vcpus) - 1); + virBufferAsprintf(&buf, "(vcpu_avail %lu)", + (1UL << virDomainDefGetVcpus(def)) - 1); if (def->cpumask) { char *ranges = virBitmapFormat(def->cpumask); @@ -2315,7 +2316,7 @@ xenFormatSxpr(virConnectPtr conn, virBufferAsprintf(&buf, "(vcpus %u)", virDomainDefGetVcpusMax(def)); if (virDomainDefHasVcpusOffline(def)) virBufferAsprintf(&buf, "(vcpu_avail %lu)", - (1UL << def->vcpus) - 1); + (1UL << virDomainDefGetVcpus(def)) - 1); for (i = 0; i < def->os.nBootDevs; i++) { switch (def->os.bootDevs[i]) { -- 2.6.2

Later on this will also be used to track size of the vcpu data array. Use size_t so that we can utilize the memory allocation helpers. --- src/conf/domain_conf.c | 2 +- src/conf/domain_conf.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 890e1c7..2bc0f1c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1463,7 +1463,7 @@ virDomainDefSetVcpus(virDomainDefPtr def, { if (vcpus > def->maxvcpus) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("maxvcpus must not be less than current vcpus (%u < %u)"), + _("maxvcpus must not be less than current vcpus (%u < %zu)"), vcpus, def->maxvcpus); return -1; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index d7630c4..b681bd2 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2213,7 +2213,7 @@ struct _virDomainDef { virDomainMemtune mem; unsigned int vcpus; - unsigned int maxvcpus; + size_t maxvcpus; int placement_mode; virBitmapPtr cpumask; -- 2.6.2

As in commit 88dc7e0c2fb, the helper can be used in cases where the function actually does not access anyting in the private data besides the agent. --- src/qemu/qemu_domain.c | 12 ++++++++++++ src/qemu/qemu_domain.h | 1 + 2 files changed, 13 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ed21245..c7687b7 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1873,6 +1873,18 @@ qemuDomainObjEnterMonitorAsync(virQEMUDriverPtr driver, } +/** + * qemuDomainGetAgent: + * @vm: domain object + * + * Returns the agent pointer of @vm; + */ +qemuAgentPtr +qemuDomainGetAgent(virDomainObjPtr vm) +{ + return (((qemuDomainObjPrivatePtr)(vm->privateData))->agent); +} + /* * obj must be locked before calling diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 14892fd..31c7d33 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -298,6 +298,7 @@ int qemuDomainObjEnterMonitorAsync(virQEMUDriverPtr driver, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +qemuAgentPtr qemuDomainGetAgent(virDomainObjPtr vm); void qemuDomainObjEnterAgent(virDomainObjPtr obj) ATTRIBUTE_NONNULL(1); void qemuDomainObjExitAgent(virDomainObjPtr obj) -- 2.6.2

Separate the code so that qemuDomainSetVcpusFlags contains only code relevant to hardware hotplug/unplug. --- src/qemu/qemu_driver.c | 137 +++++++++++++++++++++++++++---------------------- 1 file changed, 77 insertions(+), 60 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index cfcc789..62f1054 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4853,6 +4853,59 @@ qemuDomainHotplugVcpus(virQEMUDriverPtr driver, static int +qemuDomainSetVcpusAgent(virDomainObjPtr vm, + unsigned int nvcpus) +{ + qemuAgentCPUInfoPtr cpuinfo = NULL; + int ncpuinfo; + int ret = -1; + + if (!qemuDomainAgentAvailable(vm, true)) + goto cleanup; + + if (nvcpus > virDomainDefGetVcpus(vm->def)) { + virReportError(VIR_ERR_INVALID_ARG, + _("requested vcpu count is greater than the count " + "of enabled vcpus in the domain: %d > %d"), + nvcpus, virDomainDefGetVcpus(vm->def)); + goto cleanup; + } + + qemuDomainObjEnterAgent(vm); + ncpuinfo = qemuAgentGetVCPUs(qemuDomainGetAgent(vm), &cpuinfo); + qemuDomainObjExitAgent(vm); + + if (ncpuinfo < 0) + goto cleanup; + + if (qemuAgentUpdateCPUInfo(nvcpus, cpuinfo, ncpuinfo) < 0) + goto cleanup; + + qemuDomainObjEnterAgent(vm); + ret = qemuAgentSetVCPUs(qemuDomainGetAgent(vm), cpuinfo, ncpuinfo); + qemuDomainObjExitAgent(vm); + + if (ret < 0) + goto cleanup; + + if (ret < ncpuinfo) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to set state of cpu %d via guest agent"), + cpuinfo[ret-1].id); + ret = -1; + goto cleanup; + } + + ret = 0; + + cleanup: + VIR_FREE(cpuinfo); + + return ret; +} + + +static int qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, unsigned int flags) { @@ -4863,8 +4916,6 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, int ret = -1; unsigned int maxvcpus = 0; virQEMUDriverConfigPtr cfg = NULL; - qemuAgentCPUInfoPtr cpuinfo = NULL; - int ncpuinfo; qemuDomainObjPrivatePtr priv; size_t i; virCgroupPtr cgroup_temp = NULL; @@ -4891,10 +4942,15 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, 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 (def && !(flags & VIR_DOMAIN_VCPU_GUEST) && virNumaIsAvailable() && + if (def && virNumaIsAvailable() && virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) { if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0, false, &cgroup_temp) < 0) @@ -4925,71 +4981,33 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, goto endjob; } - if (flags & VIR_DOMAIN_VCPU_GUEST) { - if (!qemuDomainAgentAvailable(vm, true)) - goto endjob; - - if (nvcpus > virDomainDefGetVcpus(vm->def)) { - virReportError(VIR_ERR_INVALID_ARG, - _("requested vcpu count is greater than the count " - "of enabled vcpus in the domain: %d > %d"), - nvcpus, virDomainDefGetVcpus(vm->def)); - goto endjob; - } - - qemuDomainObjEnterAgent(vm); - ncpuinfo = qemuAgentGetVCPUs(priv->agent, &cpuinfo); - qemuDomainObjExitAgent(vm); - - if (ncpuinfo < 0) - goto endjob; - - if (qemuAgentUpdateCPUInfo(nvcpus, cpuinfo, ncpuinfo) < 0) + if (def) { + if (qemuDomainHotplugVcpus(driver, vm, nvcpus) < 0) goto endjob; - qemuDomainObjEnterAgent(vm); - ret = qemuAgentSetVCPUs(priv->agent, cpuinfo, ncpuinfo); - qemuDomainObjExitAgent(vm); - - if (ret < 0) + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) goto endjob; + } - if (ret < ncpuinfo) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("failed to set state of cpu %d via guest agent"), - cpuinfo[ret-1].id); - ret = -1; - goto endjob; + if (persistentDef) { + /* remove vcpupin entries for vcpus that were unplugged */ + if (nvcpus < virDomainDefGetVcpus(persistentDef)) { + for (i = virDomainDefGetVcpus(persistentDef) - 1; i >= nvcpus; i--) + virDomainPinDel(&persistentDef->cputune.vcpupin, + &persistentDef->cputune.nvcpupin, + i); } - } else { - if (def) { - if (qemuDomainHotplugVcpus(driver, vm, nvcpus) < 0) - goto endjob; - if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) + if (flags & VIR_DOMAIN_VCPU_MAXIMUM) { + if (virDomainDefSetVcpusMax(persistentDef, nvcpus) < 0) goto endjob; - } - - if (persistentDef) { - /* remove vcpupin entries for vcpus that were unplugged */ - if (nvcpus < virDomainDefGetVcpus(persistentDef)) { - for (i = virDomainDefGetVcpus(persistentDef) - 1; i >= nvcpus; i--) - virDomainPinDel(&persistentDef->cputune.vcpupin, - &persistentDef->cputune.nvcpupin, - i); - } - - if (flags & VIR_DOMAIN_VCPU_MAXIMUM) { - if (virDomainDefSetVcpusMax(persistentDef, nvcpus) < 0) - goto endjob; - } else { - if (virDomainDefSetVcpus(persistentDef, nvcpus) < 0) - goto endjob; - } - - if (virDomainSaveConfig(cfg->configDir, persistentDef) < 0) + } else { + if (virDomainDefSetVcpus(persistentDef, nvcpus) < 0) goto endjob; } + + if (virDomainSaveConfig(cfg->configDir, persistentDef) < 0) + goto endjob; } ret = 0; @@ -5006,7 +5024,6 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, cleanup: virDomainObjEndAPI(&vm); - VIR_FREE(cpuinfo); VIR_FREE(mem_mask); VIR_FREE(all_nodes_str); virBitmapFree(all_nodes); -- 2.6.2

With a very unfortunate timing, the agent might vanish before we do the second call while the locks were down. Re-check that the agent is available before attempting it again. --- src/qemu/qemu_driver.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 62f1054..7a99ed2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4881,6 +4881,9 @@ qemuDomainSetVcpusAgent(virDomainObjPtr vm, if (qemuAgentUpdateCPUInfo(nvcpus, cpuinfo, ncpuinfo) < 0) goto cleanup; + if (!qemuDomainAgentAvailable(vm, true)) + goto cleanup; + qemuDomainObjEnterAgent(vm); ret = qemuAgentSetVCPUs(qemuDomainGetAgent(vm), cpuinfo, ncpuinfo); qemuDomainObjExitAgent(vm); -- 2.6.2

There's only very little common code among the two operations. Split the functions so that the internals are easier to understand and refactor later. --- src/qemu/qemu_driver.c | 216 +++++++++++++++++++++++++++++++------------------ 1 file changed, 139 insertions(+), 77 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7a99ed2..1e606a6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4692,9 +4692,9 @@ qemuDomainDelCgroupForThread(virCgroupPtr cgroup, } static int -qemuDomainHotplugVcpus(virQEMUDriverPtr driver, - virDomainObjPtr vm, - unsigned int nvcpus) +qemuDomainHotplugAddVcpu(virQEMUDriverPtr driver, + virDomainObjPtr vm, + unsigned int nvcpus) { qemuDomainObjPrivatePtr priv = vm->privateData; size_t i; @@ -4710,31 +4710,15 @@ qemuDomainHotplugVcpus(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); - /* We need different branches here, because we want to offline - * in reverse order to onlining, so any partial fail leaves us in a - * reasonably sensible state */ - if (nvcpus > vcpus) { - for (i = vcpus; i < nvcpus; i++) { - /* Online new CPU */ - rc = qemuMonitorSetCPU(priv->mon, i, true); - if (rc == 0) - goto unsupported; - if (rc < 0) - goto exit_monitor; - - vcpus++; - } - } else { - for (i = vcpus - 1; i >= nvcpus; i--) { - /* Offline old CPU */ - rc = qemuMonitorSetCPU(priv->mon, i, false); - if (rc == 0) - goto unsupported; - if (rc < 0) - goto exit_monitor; + for (i = vcpus; i < nvcpus; i++) { + /* Online new CPU */ + rc = qemuMonitorSetCPU(priv->mon, i, true); + if (rc == 0) + goto unsupported; + if (rc < 0) + goto exit_monitor; - vcpus--; - } + vcpus++; } /* hotplug succeeded */ @@ -4755,15 +4739,6 @@ qemuDomainHotplugVcpus(virQEMUDriverPtr driver, goto cleanup; } - /* check if hotplug has failed */ - if (vcpus < oldvcpus && ncpupids == oldvcpus) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("qemu didn't unplug the vCPUs properly")); - vcpus = oldvcpus; - ret = -1; - goto cleanup; - } - if (ncpupids != vcpus) { virReportError(VIR_ERR_INTERNAL_ERROR, _("got wrong number of vCPU pids from QEMU monitor. " @@ -4781,50 +4756,37 @@ qemuDomainHotplugVcpus(virQEMUDriverPtr driver, &mem_mask, -1) < 0) goto cleanup; - if (nvcpus > oldvcpus) { - for (i = oldvcpus; i < nvcpus; i++) { - if (priv->cgroup) { - cgroup_vcpu = - qemuDomainAddCgroupForThread(priv->cgroup, - VIR_CGROUP_THREAD_VCPU, - i, mem_mask, - cpupids[i]); - if (!cgroup_vcpu) - goto cleanup; - } + for (i = oldvcpus; i < nvcpus; i++) { + if (priv->cgroup) { + cgroup_vcpu = + qemuDomainAddCgroupForThread(priv->cgroup, + VIR_CGROUP_THREAD_VCPU, + i, mem_mask, + cpupids[i]); + if (!cgroup_vcpu) + goto cleanup; + } - /* Inherit def->cpuset */ - if (vm->def->cpumask) { - if (qemuDomainHotplugAddPin(vm->def->cpumask, i, - &vm->def->cputune.vcpupin, - &vm->def->cputune.nvcpupin) < 0) { - ret = -1; - goto cleanup; - } - if (qemuDomainHotplugPinThread(vm->def->cpumask, i, cpupids[i], - cgroup_vcpu) < 0) { - ret = -1; - goto cleanup; - } + /* Inherit def->cpuset */ + if (vm->def->cpumask) { + if (qemuDomainHotplugAddPin(vm->def->cpumask, i, + &vm->def->cputune.vcpupin, + &vm->def->cputune.nvcpupin) < 0) { + ret = -1; + goto cleanup; } - virCgroupFree(&cgroup_vcpu); - - if (qemuProcessSetSchedParams(i, cpupids[i], - vm->def->cputune.nvcpusched, - vm->def->cputune.vcpusched) < 0) + if (qemuDomainHotplugPinThread(vm->def->cpumask, i, cpupids[i], + cgroup_vcpu) < 0) { + ret = -1; goto cleanup; + } } - } else { - for (i = oldvcpus - 1; i >= nvcpus; i--) { - if (qemuDomainDelCgroupForThread(priv->cgroup, - VIR_CGROUP_THREAD_VCPU, i) < 0) - goto cleanup; + virCgroupFree(&cgroup_vcpu); - /* Free vcpupin setting */ - virDomainPinDel(&vm->def->cputune.vcpupin, - &vm->def->cputune.nvcpupin, - i); - } + if (qemuProcessSetSchedParams(i, cpupids[i], + vm->def->cputune.nvcpusched, + vm->def->cputune.vcpusched) < 0) + goto cleanup; } priv->nvcpupids = ncpupids; @@ -4853,6 +4815,101 @@ qemuDomainHotplugVcpus(virQEMUDriverPtr driver, static int +qemuDomainHotplugDelVcpu(virQEMUDriverPtr driver, + virDomainObjPtr vm, + unsigned int nvcpus) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + size_t i; + int rc = 1; + int ret = -1; + int oldvcpus = virDomainDefGetVcpus(vm->def); + int vcpus = oldvcpus; + pid_t *cpupids = NULL; + int ncpupids; + + qemuDomainObjEnterMonitor(driver, vm); + + for (i = vcpus - 1; i >= nvcpus; i--) { + /* Offline old CPU */ + rc = qemuMonitorSetCPU(priv->mon, i, false); + if (rc == 0) + goto unsupported; + if (rc < 0) + goto exit_monitor; + + vcpus--; + } + + ret = 0; + + /* After hotplugging the CPUs we need to re-detect threads corresponding + * to the virtual CPUs. Some older versions don't provide the thread ID + * or don't have the "info cpus" command (and they don't support multiple + * CPUs anyways), so errors in the re-detection will not be treated + * fatal */ + if ((ncpupids = qemuMonitorGetCPUInfo(priv->mon, &cpupids)) <= 0) { + virResetLastError(); + goto exit_monitor; + } + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + ret = -1; + goto cleanup; + } + + /* check if hotunplug has failed */ + if (ncpupids == oldvcpus) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("qemu didn't unplug the vCPUs properly")); + vcpus = oldvcpus; + ret = -1; + goto cleanup; + } + + if (ncpupids != vcpus) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("got wrong number of vCPU pids from QEMU monitor. " + "got %d, wanted %d"), + ncpupids, vcpus); + vcpus = oldvcpus; + ret = -1; + goto cleanup; + } + + for (i = oldvcpus - 1; i >= nvcpus; i--) { + if (qemuDomainDelCgroupForThread(priv->cgroup, + VIR_CGROUP_THREAD_VCPU, i) < 0) + goto cleanup; + + /* Free vcpupin setting */ + virDomainPinDel(&vm->def->cputune.vcpupin, + &vm->def->cputune.nvcpupin, + i); + } + + priv->nvcpupids = ncpupids; + VIR_FREE(priv->vcpupids); + priv->vcpupids = cpupids; + cpupids = NULL; + + cleanup: + VIR_FREE(cpupids); + if (virDomainObjIsActive(vm) && + virDomainDefSetVcpus(vm->def, vcpus) < 0) + ret = -1; + virDomainAuditVcpu(vm, oldvcpus, nvcpus, "update", rc == 1); + return ret; + + unsupported: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot change vcpu count of this domain")); + exit_monitor: + ignore_value(qemuDomainObjExitMonitor(driver, vm)); + goto cleanup; +} + + +static int qemuDomainSetVcpusAgent(virDomainObjPtr vm, unsigned int nvcpus) { @@ -4985,8 +5042,13 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, } if (def) { - if (qemuDomainHotplugVcpus(driver, vm, nvcpus) < 0) - goto endjob; + if (nvcpus > virDomainDefGetVcpus(def)) { + if (qemuDomainHotplugAddVcpu(driver, vm, nvcpus) < 0) + goto endjob; + } else { + if (qemuDomainHotplugDelVcpu(driver, vm, nvcpus) < 0) + goto endjob; + } if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) goto endjob; -- 2.6.2

The cpu hotplug helper functions used negative error handling in a part of them, although some code that was added later didn't properly set the error codes in some cases. This would cause improper error messages in cases where we couldn't modify the numa cpu mask and a few other cases. Fix the logic by converting it to the regularly used pattern. --- src/qemu/qemu_driver.c | 33 ++++++++++++--------------------- 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1e606a6..bbd43f4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4721,10 +4721,6 @@ qemuDomainHotplugAddVcpu(virQEMUDriverPtr driver, vcpus++; } - /* hotplug succeeded */ - - ret = 0; - /* After hotplugging the CPUs we need to re-detect threads corresponding * to the virtual CPUs. Some older versions don't provide the thread ID * or don't have the "info cpus" command (and they don't support multiple @@ -4732,12 +4728,12 @@ qemuDomainHotplugAddVcpu(virQEMUDriverPtr driver, * fatal */ if ((ncpupids = qemuMonitorGetCPUInfo(priv->mon, &cpupids)) <= 0) { virResetLastError(); + ret = 0; goto exit_monitor; } - if (qemuDomainObjExitMonitor(driver, vm) < 0) { - ret = -1; + + if (qemuDomainObjExitMonitor(driver, vm) < 0) goto cleanup; - } if (ncpupids != vcpus) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -4745,7 +4741,6 @@ qemuDomainHotplugAddVcpu(virQEMUDriverPtr driver, "got %d, wanted %d"), ncpupids, vcpus); vcpus = oldvcpus; - ret = -1; goto cleanup; } @@ -4771,15 +4766,12 @@ qemuDomainHotplugAddVcpu(virQEMUDriverPtr driver, if (vm->def->cpumask) { if (qemuDomainHotplugAddPin(vm->def->cpumask, i, &vm->def->cputune.vcpupin, - &vm->def->cputune.nvcpupin) < 0) { - ret = -1; + &vm->def->cputune.nvcpupin) < 0) goto cleanup; - } + if (qemuDomainHotplugPinThread(vm->def->cpumask, i, cpupids[i], - cgroup_vcpu) < 0) { - ret = -1; + cgroup_vcpu) < 0) goto cleanup; - } } virCgroupFree(&cgroup_vcpu); @@ -4794,6 +4786,8 @@ qemuDomainHotplugAddVcpu(virQEMUDriverPtr driver, priv->vcpupids = cpupids; cpupids = NULL; + ret = 0; + cleanup: VIR_FREE(cpupids); VIR_FREE(mem_mask); @@ -4841,8 +4835,6 @@ qemuDomainHotplugDelVcpu(virQEMUDriverPtr driver, vcpus--; } - ret = 0; - /* After hotplugging the CPUs we need to re-detect threads corresponding * to the virtual CPUs. Some older versions don't provide the thread ID * or don't have the "info cpus" command (and they don't support multiple @@ -4850,19 +4842,17 @@ qemuDomainHotplugDelVcpu(virQEMUDriverPtr driver, * fatal */ if ((ncpupids = qemuMonitorGetCPUInfo(priv->mon, &cpupids)) <= 0) { virResetLastError(); + ret = 0; goto exit_monitor; } - if (qemuDomainObjExitMonitor(driver, vm) < 0) { - ret = -1; + if (qemuDomainObjExitMonitor(driver, vm) < 0) goto cleanup; - } /* check if hotunplug has failed */ if (ncpupids == oldvcpus) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("qemu didn't unplug the vCPUs properly")); vcpus = oldvcpus; - ret = -1; goto cleanup; } @@ -4872,7 +4862,6 @@ qemuDomainHotplugDelVcpu(virQEMUDriverPtr driver, "got %d, wanted %d"), ncpupids, vcpus); vcpus = oldvcpus; - ret = -1; goto cleanup; } @@ -4892,6 +4881,8 @@ qemuDomainHotplugDelVcpu(virQEMUDriverPtr driver, priv->vcpupids = cpupids; cpupids = NULL; + ret = 0; + cleanup: VIR_FREE(cpupids); if (virDomainObjIsActive(vm) && -- 2.6.2

Let the function report errors internally and change it to return standard return codes. --- src/qemu/qemu_driver.c | 22 ++++------------------ src/qemu/qemu_monitor.c | 3 +++ src/qemu/qemu_monitor_json.c | 8 -------- src/qemu/qemu_monitor_text.c | 23 ++++++++++------------- 4 files changed, 17 insertions(+), 39 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bbd43f4..0ffe079 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4698,7 +4698,6 @@ qemuDomainHotplugAddVcpu(virQEMUDriverPtr driver, { qemuDomainObjPrivatePtr priv = vm->privateData; size_t i; - int rc = 1; int ret = -1; int oldvcpus = virDomainDefGetVcpus(vm->def); int vcpus = oldvcpus; @@ -4712,10 +4711,7 @@ qemuDomainHotplugAddVcpu(virQEMUDriverPtr driver, for (i = vcpus; i < nvcpus; i++) { /* Online new CPU */ - rc = qemuMonitorSetCPU(priv->mon, i, true); - if (rc == 0) - goto unsupported; - if (rc < 0) + if (qemuMonitorSetCPU(priv->mon, i, true) < 0) goto exit_monitor; vcpus++; @@ -4794,14 +4790,11 @@ qemuDomainHotplugAddVcpu(virQEMUDriverPtr driver, if (virDomainObjIsActive(vm) && virDomainDefSetVcpus(vm->def, vcpus) < 0) ret = -1; - virDomainAuditVcpu(vm, oldvcpus, nvcpus, "update", rc == 1); + virDomainAuditVcpu(vm, oldvcpus, nvcpus, "update", ret == 0); if (cgroup_vcpu) virCgroupFree(&cgroup_vcpu); return ret; - unsupported: - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot change vcpu count of this domain")); exit_monitor: ignore_value(qemuDomainObjExitMonitor(driver, vm)); goto cleanup; @@ -4815,7 +4808,6 @@ qemuDomainHotplugDelVcpu(virQEMUDriverPtr driver, { qemuDomainObjPrivatePtr priv = vm->privateData; size_t i; - int rc = 1; int ret = -1; int oldvcpus = virDomainDefGetVcpus(vm->def); int vcpus = oldvcpus; @@ -4826,10 +4818,7 @@ qemuDomainHotplugDelVcpu(virQEMUDriverPtr driver, for (i = vcpus - 1; i >= nvcpus; i--) { /* Offline old CPU */ - rc = qemuMonitorSetCPU(priv->mon, i, false); - if (rc == 0) - goto unsupported; - if (rc < 0) + if (qemuMonitorSetCPU(priv->mon, i, false) < 0) goto exit_monitor; vcpus--; @@ -4888,12 +4877,9 @@ qemuDomainHotplugDelVcpu(virQEMUDriverPtr driver, if (virDomainObjIsActive(vm) && virDomainDefSetVcpus(vm->def, vcpus) < 0) ret = -1; - virDomainAuditVcpu(vm, oldvcpus, nvcpus, "update", rc == 1); + virDomainAuditVcpu(vm, oldvcpus, nvcpus, "update", ret == 0); return ret; - unsupported: - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot change vcpu count of this domain")); exit_monitor: ignore_value(qemuDomainObjExitMonitor(driver, vm)); goto cleanup; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index bc5ed33..4906faa 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1955,6 +1955,9 @@ qemuMonitorSetBalloon(qemuMonitorPtr mon, } +/* + * Returns: 0 if CPU modification was successful or -1 on failure + */ int qemuMonitorSetCPU(qemuMonitorPtr mon, int cpu, bool online) { diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 86b8c7b..84c0be2 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2158,10 +2158,6 @@ qemuMonitorJSONSetBalloon(qemuMonitorPtr mon, } -/* - * Returns: 0 if CPU hotplug not supported, +1 if CPU hotplug worked - * or -1 on failure - */ int qemuMonitorJSONSetCPU(qemuMonitorPtr mon, int cpu, bool online) { @@ -2188,10 +2184,6 @@ int qemuMonitorJSONSetCPU(qemuMonitorPtr mon, else ret = qemuMonitorJSONCheckError(cmd, reply); - /* this function has non-standard return values, so adapt it */ - if (ret == 0) - ret = 1; - cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index f44da20..665723d 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -1136,10 +1136,6 @@ qemuMonitorTextSetBalloon(qemuMonitorPtr mon, } -/* - * Returns: 0 if CPU hotplug not supported, +1 if CPU hotplug worked - * or -1 on failure - */ int qemuMonitorTextSetCPU(qemuMonitorPtr mon, int cpu, bool online) { char *cmd; @@ -1149,22 +1145,23 @@ int qemuMonitorTextSetCPU(qemuMonitorPtr mon, int cpu, bool online) if (virAsprintf(&cmd, "cpu_set %d %s", cpu, online ? "online" : "offline") < 0) return -1; - if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { - VIR_FREE(cmd); - return -1; - } - VIR_FREE(cmd); + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) + goto cleanup; /* If the command failed qemu prints: 'unknown command' * No message is printed on success it seems */ if (strstr(reply, "unknown command:")) { - /* Don't set error - it is expected CPU onlining fails on many qemu - caller will handle */ - ret = 0; - } else { - ret = 1; + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot change vcpu count of this domain")); + goto cleanup; } + ret = 0; + + cleanup: VIR_FREE(reply); + VIR_FREE(cmd); + return ret; } -- 2.6.2

qemuDomainHotplugVcpus/qemuDomainHotunplugVcpus are complex enough in regards of adding one CPU. Additionally it will be desired to reuse those functions later with specific vCPU hotplug. Move the loops for adding vCPUs into qemuDomainSetVcpusFlags so that the helpers can be made simpler and more straightforward. --- src/qemu/qemu_driver.c | 104 +++++++++++++++++++++++-------------------------- 1 file changed, 48 insertions(+), 56 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0ffe079..bc3c5dd 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4694,10 +4694,9 @@ qemuDomainDelCgroupForThread(virCgroupPtr cgroup, static int qemuDomainHotplugAddVcpu(virQEMUDriverPtr driver, virDomainObjPtr vm, - unsigned int nvcpus) + unsigned int vcpu) { qemuDomainObjPrivatePtr priv = vm->privateData; - size_t i; int ret = -1; int oldvcpus = virDomainDefGetVcpus(vm->def); int vcpus = oldvcpus; @@ -4709,13 +4708,10 @@ qemuDomainHotplugAddVcpu(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); - for (i = vcpus; i < nvcpus; i++) { - /* Online new CPU */ - if (qemuMonitorSetCPU(priv->mon, i, true) < 0) - goto exit_monitor; + if (qemuMonitorSetCPU(priv->mon, vcpu, true) < 0) + goto exit_monitor; - vcpus++; - } + vcpus++; /* After hotplugging the CPUs we need to re-detect threads corresponding * to the virtual CPUs. Some older versions don't provide the thread ID @@ -4747,36 +4743,34 @@ qemuDomainHotplugAddVcpu(virQEMUDriverPtr driver, &mem_mask, -1) < 0) goto cleanup; - for (i = oldvcpus; i < nvcpus; i++) { - if (priv->cgroup) { - cgroup_vcpu = - qemuDomainAddCgroupForThread(priv->cgroup, - VIR_CGROUP_THREAD_VCPU, - i, mem_mask, - cpupids[i]); - if (!cgroup_vcpu) - goto cleanup; - } - - /* Inherit def->cpuset */ - if (vm->def->cpumask) { - if (qemuDomainHotplugAddPin(vm->def->cpumask, i, - &vm->def->cputune.vcpupin, - &vm->def->cputune.nvcpupin) < 0) - goto cleanup; + if (priv->cgroup) { + cgroup_vcpu = + qemuDomainAddCgroupForThread(priv->cgroup, + VIR_CGROUP_THREAD_VCPU, + vcpu, mem_mask, + cpupids[vcpu]); + if (!cgroup_vcpu) + goto cleanup; + } - if (qemuDomainHotplugPinThread(vm->def->cpumask, i, cpupids[i], - cgroup_vcpu) < 0) - goto cleanup; - } - virCgroupFree(&cgroup_vcpu); + /* Inherit def->cpuset */ + if (vm->def->cpumask) { + if (qemuDomainHotplugAddPin(vm->def->cpumask, vcpu, + &vm->def->cputune.vcpupin, + &vm->def->cputune.nvcpupin) < 0) + goto cleanup; - if (qemuProcessSetSchedParams(i, cpupids[i], - vm->def->cputune.nvcpusched, - vm->def->cputune.vcpusched) < 0) + if (qemuDomainHotplugPinThread(vm->def->cpumask, vcpu, cpupids[vcpu], + cgroup_vcpu) < 0) { goto cleanup; + } } + if (qemuProcessSetSchedParams(vcpu, cpupids[vcpu], + vm->def->cputune.nvcpusched, + vm->def->cputune.vcpusched) < 0) + goto cleanup; + priv->nvcpupids = ncpupids; VIR_FREE(priv->vcpupids); priv->vcpupids = cpupids; @@ -4790,7 +4784,7 @@ qemuDomainHotplugAddVcpu(virQEMUDriverPtr driver, if (virDomainObjIsActive(vm) && virDomainDefSetVcpus(vm->def, vcpus) < 0) ret = -1; - virDomainAuditVcpu(vm, oldvcpus, nvcpus, "update", ret == 0); + virDomainAuditVcpu(vm, oldvcpus, vcpus, "update", ret == 0); if (cgroup_vcpu) virCgroupFree(&cgroup_vcpu); return ret; @@ -4804,10 +4798,9 @@ qemuDomainHotplugAddVcpu(virQEMUDriverPtr driver, static int qemuDomainHotplugDelVcpu(virQEMUDriverPtr driver, virDomainObjPtr vm, - unsigned int nvcpus) + unsigned int vcpu) { qemuDomainObjPrivatePtr priv = vm->privateData; - size_t i; int ret = -1; int oldvcpus = virDomainDefGetVcpus(vm->def); int vcpus = oldvcpus; @@ -4816,13 +4809,10 @@ qemuDomainHotplugDelVcpu(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); - for (i = vcpus - 1; i >= nvcpus; i--) { - /* Offline old CPU */ - if (qemuMonitorSetCPU(priv->mon, i, false) < 0) - goto exit_monitor; + if (qemuMonitorSetCPU(priv->mon, vcpu, false) < 0) + goto exit_monitor; - vcpus--; - } + vcpus--; /* After hotplugging the CPUs we need to re-detect threads corresponding * to the virtual CPUs. Some older versions don't provide the thread ID @@ -4854,16 +4844,14 @@ qemuDomainHotplugDelVcpu(virQEMUDriverPtr driver, goto cleanup; } - for (i = oldvcpus - 1; i >= nvcpus; i--) { - if (qemuDomainDelCgroupForThread(priv->cgroup, - VIR_CGROUP_THREAD_VCPU, i) < 0) - goto cleanup; + if (qemuDomainDelCgroupForThread(priv->cgroup, + VIR_CGROUP_THREAD_VCPU, vcpu) < 0) + goto cleanup; - /* Free vcpupin setting */ - virDomainPinDel(&vm->def->cputune.vcpupin, - &vm->def->cputune.nvcpupin, - i); - } + /* Free vcpupin setting */ + virDomainPinDel(&vm->def->cputune.vcpupin, + &vm->def->cputune.nvcpupin, + vcpu); priv->nvcpupids = ncpupids; VIR_FREE(priv->vcpupids); @@ -4877,7 +4865,7 @@ qemuDomainHotplugDelVcpu(virQEMUDriverPtr driver, if (virDomainObjIsActive(vm) && virDomainDefSetVcpus(vm->def, vcpus) < 0) ret = -1; - virDomainAuditVcpu(vm, oldvcpus, nvcpus, "update", ret == 0); + virDomainAuditVcpu(vm, oldvcpus, vcpus, "update", ret == 0); return ret; exit_monitor: @@ -5020,11 +5008,15 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, if (def) { if (nvcpus > virDomainDefGetVcpus(def)) { - if (qemuDomainHotplugAddVcpu(driver, vm, nvcpus) < 0) - goto endjob; + for (i = virDomainDefGetVcpus(def); i < nvcpus; i++) { + if (qemuDomainHotplugAddVcpu(driver, vm, i) < 0) + goto endjob; + } } else { - if (qemuDomainHotplugDelVcpu(driver, vm, nvcpus) < 0) - goto endjob; + for (i = virDomainDefGetVcpus(def) - 1; i >= nvcpus; i--) { + if (qemuDomainHotplugDelVcpu(driver, vm, i) < 0) + goto endjob; + } } if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) -- 2.6.2

Refactor the code flow so that 'exit_monitor:' can be removed. This patch also moves the auditing and setting of the new vCPU count right to the place where the hotplug happens, since it's possible that the hotplug succeeds and adds a cpu while other stuff fails. Lastly, failures of qemuMonitorGetCPUInfo are now reported rather than ignored. The function retuns 0 if it "successfully" detected 0 threads. --- src/qemu/qemu_driver.c | 54 +++++++++++++++++++++++--------------------------- 1 file changed, 25 insertions(+), 29 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bc3c5dd..da4db36 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4698,41 +4698,46 @@ qemuDomainHotplugAddVcpu(virQEMUDriverPtr driver, { qemuDomainObjPrivatePtr priv = vm->privateData; int ret = -1; + int rc; int oldvcpus = virDomainDefGetVcpus(vm->def); - int vcpus = oldvcpus; pid_t *cpupids = NULL; - int ncpupids; + int ncpupids = 0; virCgroupPtr cgroup_vcpu = NULL; char *mem_mask = NULL; virDomainNumatuneMemMode mem_mode; qemuDomainObjEnterMonitor(driver, vm); - if (qemuMonitorSetCPU(priv->mon, vcpu, true) < 0) - goto exit_monitor; - - vcpus++; + rc = qemuMonitorSetCPU(priv->mon, vcpu, true); - /* After hotplugging the CPUs we need to re-detect threads corresponding - * to the virtual CPUs. Some older versions don't provide the thread ID - * or don't have the "info cpus" command (and they don't support multiple - * CPUs anyways), so errors in the re-detection will not be treated - * fatal */ - if ((ncpupids = qemuMonitorGetCPUInfo(priv->mon, &cpupids)) <= 0) { - virResetLastError(); - ret = 0; - goto exit_monitor; - } + if (rc == 0) + ncpupids = qemuMonitorGetCPUInfo(priv->mon, &cpupids); if (qemuDomainObjExitMonitor(driver, vm) < 0) goto cleanup; - if (ncpupids != vcpus) { + virDomainAuditVcpu(vm, oldvcpus, oldvcpus + 1, "update", rc == 0); + + if (rc < 0) + goto cleanup; + + ignore_value(virDomainDefSetVcpus(vm->def, oldvcpus + 1)); + + if (ncpupids < 0) + goto cleanup; + + /* failure to re-detect vCPU pids after hotplug due to lack of support was + * historically deemed not fatal. We need to skip the rest of the steps though. */ + if (ncpupids == 0) { + ret = 0; + goto cleanup; + } + + if (ncpupids != oldvcpus + 1) { virReportError(VIR_ERR_INTERNAL_ERROR, _("got wrong number of vCPU pids from QEMU monitor. " "got %d, wanted %d"), - ncpupids, vcpus); - vcpus = oldvcpus; + ncpupids, oldvcpus + 1); goto cleanup; } @@ -4781,17 +4786,8 @@ qemuDomainHotplugAddVcpu(virQEMUDriverPtr driver, cleanup: VIR_FREE(cpupids); VIR_FREE(mem_mask); - if (virDomainObjIsActive(vm) && - virDomainDefSetVcpus(vm->def, vcpus) < 0) - ret = -1; - virDomainAuditVcpu(vm, oldvcpus, vcpus, "update", ret == 0); - if (cgroup_vcpu) - virCgroupFree(&cgroup_vcpu); + virCgroupFree(&cgroup_vcpu); return ret; - - exit_monitor: - ignore_value(qemuDomainObjExitMonitor(driver, vm)); - goto cleanup; } -- 2.6.2

Refactor the code flow so that 'exit_monitor:' can be removed. This patch moves the auditing functions into places where it's certain that hotunplug was or was not successful and reports errors from qemuMonitorGetCPUInfo properly. --- src/qemu/qemu_driver.c | 50 +++++++++++++++----------------------------------- 1 file changed, 15 insertions(+), 35 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index da4db36..dc95f91 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4798,48 +4798,36 @@ qemuDomainHotplugDelVcpu(virQEMUDriverPtr driver, { qemuDomainObjPrivatePtr priv = vm->privateData; int ret = -1; + int rc; int oldvcpus = virDomainDefGetVcpus(vm->def); - int vcpus = oldvcpus; pid_t *cpupids = NULL; - int ncpupids; + int ncpupids = 0; qemuDomainObjEnterMonitor(driver, vm); - if (qemuMonitorSetCPU(priv->mon, vcpu, false) < 0) - goto exit_monitor; + rc = qemuMonitorSetCPU(priv->mon, vcpu, false); - vcpus--; + if (rc == 0) + ncpupids = qemuMonitorGetCPUInfo(priv->mon, &cpupids); - /* After hotplugging the CPUs we need to re-detect threads corresponding - * to the virtual CPUs. Some older versions don't provide the thread ID - * or don't have the "info cpus" command (and they don't support multiple - * CPUs anyways), so errors in the re-detection will not be treated - * fatal */ - if ((ncpupids = qemuMonitorGetCPUInfo(priv->mon, &cpupids)) <= 0) { - virResetLastError(); - ret = 0; - goto exit_monitor; - } if (qemuDomainObjExitMonitor(driver, vm) < 0) goto cleanup; - /* check if hotunplug has failed */ - if (ncpupids == oldvcpus) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("qemu didn't unplug the vCPUs properly")); - vcpus = oldvcpus; + virDomainAuditVcpu(vm, oldvcpus, oldvcpus - 1, "update", + rc == 0 && ncpupids == oldvcpus -1); + + if (rc < 0 || ncpupids < 0) goto cleanup; - } - if (ncpupids != vcpus) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("got wrong number of vCPU pids from QEMU monitor. " - "got %d, wanted %d"), - ncpupids, vcpus); - vcpus = oldvcpus; + /* check if hotunplug has failed */ + if (ncpupids != oldvcpus - 1) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("qemu didn't unplug vCPU '%u' properly"), vcpu); goto cleanup; } + ignore_value(virDomainDefSetVcpus(vm->def, oldvcpus - 1)); + if (qemuDomainDelCgroupForThread(priv->cgroup, VIR_CGROUP_THREAD_VCPU, vcpu) < 0) goto cleanup; @@ -4858,15 +4846,7 @@ qemuDomainHotplugDelVcpu(virQEMUDriverPtr driver, cleanup: VIR_FREE(cpupids); - if (virDomainObjIsActive(vm) && - virDomainDefSetVcpus(vm->def, vcpus) < 0) - ret = -1; - virDomainAuditVcpu(vm, oldvcpus, vcpus, "update", ret == 0); return ret; - - exit_monitor: - ignore_value(qemuDomainObjExitMonitor(driver, vm)); - goto cleanup; } -- 2.6.2

To allow collecting all relevant data at one place let's make def->vcpus a structure and then we can start moving stuff into it. --- src/conf/domain_conf.c | 55 ++++++++++++++++++++++++++++++++++++++++++++------ src/conf/domain_conf.h | 10 ++++++++- 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2bc0f1c..ed4fe29 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1430,14 +1430,32 @@ void virDomainLeaseDefFree(virDomainLeaseDefPtr def) } +static void +virDomainVcpuInfoClear(virDomainVcpuInfoPtr info) +{ + if (!info) + return; +} + + int virDomainDefSetVcpusMax(virDomainDefPtr def, unsigned int maxvcpus) { - if (maxvcpus < def->vcpus) - virDomainDefSetVcpus(def, maxvcpus); + size_t i; - def->maxvcpus = maxvcpus; + if (def->maxvcpus == maxvcpus) + return 0; + + if (def->maxvcpus < maxvcpus) { + if (VIR_EXPAND_N(def->vcpus, def->maxvcpus, maxvcpus - def->maxvcpus) < 0) + return -1; + } else { + for (i = maxvcpus; i < def->maxvcpus; i++) + virDomainVcpuInfoClear(&def->vcpus[i]); + + VIR_SHRINK_N(def->vcpus, def->maxvcpus, def->maxvcpus - maxvcpus); + } return 0; } @@ -1446,7 +1464,14 @@ virDomainDefSetVcpusMax(virDomainDefPtr def, bool virDomainDefHasVcpusOffline(const virDomainDef *def) { - return def->vcpus < def->maxvcpus; + size_t i; + + for (i = 0; i < def->maxvcpus; i++) { + if (!def->vcpus[i].online) + return true; + } + + return false; } @@ -1461,6 +1486,8 @@ int virDomainDefSetVcpus(virDomainDefPtr def, unsigned int vcpus) { + size_t i; + if (vcpus > def->maxvcpus) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("maxvcpus must not be less than current vcpus (%u < %zu)"), @@ -1468,7 +1495,11 @@ virDomainDefSetVcpus(virDomainDefPtr def, return -1; } - def->vcpus = vcpus; + for (i = 0; i < vcpus; i++) + def->vcpus[i].online = true; + + for (i = vcpus; i < def->maxvcpus; i++) + def->vcpus[i].online = false; return 0; } @@ -1477,7 +1508,15 @@ virDomainDefSetVcpus(virDomainDefPtr def, unsigned int virDomainDefGetVcpus(const virDomainDef *def) { - return def->vcpus; + size_t i; + unsigned int ret = 0; + + for (i = 0; i < def->maxvcpus; i++) { + if (def->vcpus[i].online) + ret++; + } + + return ret; } @@ -2505,6 +2544,10 @@ void virDomainDefFree(virDomainDefPtr def) virDomainResourceDefFree(def->resource); + for (i = 0; i < def->maxvcpus; i++) + virDomainVcpuInfoClear(&def->vcpus[i]); + VIR_FREE(def->vcpus); + /* hostdevs must be freed before nets (or any future "intelligent * hostdevs") because the pointer to the hostdev is really * pointing into the middle of the higher level device's object, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b681bd2..ad6c401 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2139,6 +2139,14 @@ struct _virDomainCputune { virDomainThreadSchedParamPtr iothreadsched; }; + +typedef struct _virDomainVcpuInfo virDomainVcpuInfo; +typedef virDomainVcpuInfo *virDomainVcpuInfoPtr; + +struct _virDomainVcpuInfo { + bool online; +}; + typedef struct _virDomainBlkiotune virDomainBlkiotune; typedef virDomainBlkiotune *virDomainBlkiotunePtr; @@ -2212,7 +2220,7 @@ struct _virDomainDef { virDomainBlkiotune blkio; virDomainMemtune mem; - unsigned int vcpus; + virDomainVcpuInfoPtr vcpus; size_t maxvcpus; int placement_mode; virBitmapPtr cpumask; -- 2.6.2

Extract the checking code into a separate function and prepare the infrastructure for checking the new structure type. --- src/conf/domain_conf.c | 41 ++++++++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ed4fe29..d7c1a73 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17856,6 +17856,35 @@ virDomainMemoryDefCheckABIStability(virDomainMemoryDefPtr src, } +static bool +virDomainDefVcpuCheckAbiStability(virDomainDefPtr src, + virDomainDefPtr dst) +{ + size_t i; + + if (src->maxvcpus != dst->maxvcpus) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target domain vCPU max %zu does not match source %zu"), + dst->maxvcpus, src->maxvcpus); + return false; + } + + for (i = 0; i < src->maxvcpus; i++) { + virDomainVcpuInfoPtr svcpu = &src->vcpus[i]; + virDomainVcpuInfoPtr dvcpu = &dst->vcpus[i]; + + if (svcpu->online != dvcpu->online) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("State of vCPU '%zu' differs between source and " + "destination definitions"), i); + return false; + } + } + + return true; +} + + /* This compares two configurations and looks for any differences * which will affect the guest ABI. This is primarily to allow * validation of custom XML config passed in during migration @@ -17929,18 +17958,8 @@ virDomainDefCheckABIStability(virDomainDefPtr src, goto error; } - if (virDomainDefGetVcpus(src) != virDomainDefGetVcpus(dst)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target domain vCPU count %d does not match source %d"), - virDomainDefGetVcpus(dst), virDomainDefGetVcpus(src)); + if (!virDomainDefVcpuCheckAbiStability(src, dst)) goto error; - } - if (virDomainDefGetVcpusMax(src) != virDomainDefGetVcpusMax(dst)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target domain vCPU max %d does not match source %d"), - virDomainDefGetVcpusMax(dst), virDomainDefGetVcpusMax(src)); - goto error; - } if (src->iothreads != dst->iothreads) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, -- 2.6.2

Once more stuff will be moved into the vCPU data structure it will be necessary to get a specific one in some ocasions. Add a helper that will simplify this task. --- src/conf/domain_conf.c | 15 +++++++++++++++ src/conf/domain_conf.h | 2 ++ src/libvirt_private.syms | 1 + 3 files changed, 18 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d7c1a73..da533c6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1520,6 +1520,21 @@ virDomainDefGetVcpus(const virDomainDef *def) } +virDomainVcpuInfoPtr +virDomainDefGetVcpu(virDomainDefPtr def, + unsigned int vcpu) +{ + if (vcpu > def->maxvcpus) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("vCPU '%u' is not present in domain definition"), + vcpu); + return NULL; + } + + return &def->vcpus[vcpu]; +} + + virDomainDiskDefPtr virDomainDiskDefNew(virDomainXMLOptionPtr xmlopt) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ad6c401..a396253 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2350,6 +2350,8 @@ bool virDomainDefHasVcpusOffline(const virDomainDef *def); unsigned int virDomainDefGetVcpusMax(const virDomainDef *def); int virDomainDefSetVcpus(virDomainDefPtr def, unsigned int vcpus); unsigned int virDomainDefGetVcpus(const virDomainDef *def); +virDomainVcpuInfoPtr virDomainDefGetVcpu(virDomainDefPtr def, unsigned int vcpu) + ATTRIBUTE_RETURN_CHECK; unsigned long long virDomainDefGetMemoryInitial(const virDomainDef *def); void virDomainDefSetMemoryTotal(virDomainDefPtr def, unsigned long long size); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6fb36c3..c800e0e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -217,6 +217,7 @@ virDomainDefGetDefaultEmulator; virDomainDefGetMemoryActual; virDomainDefGetMemoryInitial; virDomainDefGetSecurityLabelDef; +virDomainDefGetVcpu; virDomainDefGetVcpus; virDomainDefGetVcpusMax; virDomainDefHasDeviceAddress; -- 2.6.2

Since commit 0c04906fa the check for priv->cgroup doesn't make sense as the calls to virCgroupHasController return the same information. Remove it and move it's comment partially to the new check. The already spurious check was also later copied to the iothreads code. --- src/qemu/qemu_cgroup.c | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 2e69024..fa0b97b 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -993,18 +993,13 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm) /* * If CPU cgroup controller is not initialized here, then we need * neither period nor quota settings. And if CPUSET controller is - * not initialized either, then there's nothing to do anyway. + * not initialized either, then there's nothing to do anyway. CPU pinning + * will be set via virProcessSetAffinity. */ if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU) && !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) return 0; - /* We are trying to setup cgroups for CPU pinning, which can also be done - * with virProcessSetAffinity, thus the lack of cgroups is not fatal here. - */ - if (priv->cgroup == NULL) - return 0; - if (priv->nvcpupids == 0 || priv->vcpupids[0] == vm->pid) { /* If we don't know VCPU<->PID mapping or all vcpu runs in the same * thread, we cannot control each vcpu. @@ -1104,9 +1099,6 @@ qemuSetupCgroupForEmulator(virDomainObjPtr vm) !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) return 0; - if (priv->cgroup == NULL) - return 0; /* Not supported, so claim success */ - if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0, true, &cgroup_emulator) < 0) goto cleanup; @@ -1177,12 +1169,6 @@ qemuSetupCgroupForIOThreads(virDomainObjPtr vm) !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) return 0; - /* We are trying to setup cgroups for CPU pinning, which can also be done - * with virProcessSetAffinity, thus the lack of cgroups is not fatal here. - */ - if (priv->cgroup == NULL) - return 0; - if (virDomainNumatuneGetMode(vm->def->numa, -1, &mem_mode) == 0 && mem_mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT && virDomainNumatuneMaybeFormatNodeset(vm->def->numa, -- 2.6.2

The vCPU threads make sense in the counterparts that set the vCPU bandwidth/quota, not in the emulator one. The emulator tunables are set all the time anyways. Drop the extra check and remove the now unneeded vm argument. --- src/qemu/qemu_driver.c | 33 ++++++++++----------------------- 1 file changed, 10 insertions(+), 23 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index dc95f91..5c3703f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10312,18 +10312,15 @@ qemuSetVcpusBWLive(virDomainObjPtr vm, virCgroupPtr cgroup, } static int -qemuSetEmulatorBandwidthLive(virDomainObjPtr vm, virCgroupPtr cgroup, - unsigned long long period, long long quota) +qemuSetEmulatorBandwidthLive(virCgroupPtr cgroup, + unsigned long long period, + long long quota) { - qemuDomainObjPrivatePtr priv = vm->privateData; virCgroupPtr cgroup_emulator = NULL; if (period == 0 && quota == 0) return 0; - if (priv->nvcpupids == 0 || priv->vcpupids[0] == vm->pid) - return 0; - if (virCgroupNewThread(cgroup, VIR_CGROUP_THREAD_EMULATOR, 0, false, &cgroup_emulator) < 0) goto cleanup; @@ -10500,7 +10497,7 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, QEMU_SCHED_MIN_PERIOD, QEMU_SCHED_MAX_PERIOD); if (flags & VIR_DOMAIN_AFFECT_LIVE && value_ul) { - if ((rc = qemuSetEmulatorBandwidthLive(vm, priv->cgroup, + if ((rc = qemuSetEmulatorBandwidthLive(priv->cgroup, value_ul, 0))) goto endjob; @@ -10521,7 +10518,7 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, QEMU_SCHED_MIN_QUOTA, QEMU_SCHED_MAX_QUOTA); if (flags & VIR_DOMAIN_AFFECT_LIVE && value_l) { - if ((rc = qemuSetEmulatorBandwidthLive(vm, priv->cgroup, + if ((rc = qemuSetEmulatorBandwidthLive(priv->cgroup, 0, value_l))) goto endjob; @@ -10636,29 +10633,19 @@ qemuGetVcpusBWLive(virDomainObjPtr vm, } static int -qemuGetEmulatorBandwidthLive(virDomainObjPtr vm, virCgroupPtr cgroup, - unsigned long long *period, long long *quota) +qemuGetEmulatorBandwidthLive(virCgroupPtr cgroup, + unsigned long long *period, + long long *quota) { virCgroupPtr cgroup_emulator = NULL; - qemuDomainObjPrivatePtr priv = NULL; - int rc; int ret = -1; - priv = vm->privateData; - if (priv->nvcpupids == 0 || priv->vcpupids[0] == vm->pid) { - /* We don't create sub dir for each vcpu */ - *period = 0; - *quota = 0; - return 0; - } - /* get period and quota for emulator */ if (virCgroupNewThread(cgroup, VIR_CGROUP_THREAD_EMULATOR, 0, false, &cgroup_emulator) < 0) goto cleanup; - rc = qemuGetVcpuBWLive(cgroup_emulator, period, quota); - if (rc < 0) + if (qemuGetVcpuBWLive(cgroup_emulator, period, quota) < 0) goto cleanup; ret = 0; @@ -10748,7 +10735,7 @@ qemuDomainGetSchedulerParametersFlags(virDomainPtr dom, } if (*nparams > 3 && cpu_bw_status) { - rc = qemuGetEmulatorBandwidthLive(vm, priv->cgroup, &emulator_period, + rc = qemuGetEmulatorBandwidthLive(priv->cgroup, &emulator_period, &emulator_quota); if (rc != 0) goto cleanup; -- 2.6.2

Add qemuDomainHasVCpuPids to do the checking and replace in place checks with it. We no longer need checking whether the thread contains fake data (vcpupids[0] == vm->pid) as in b07f3d821dfb11a118ee75ea275fd6ab737d9500 and 65686e5a81d654d834d338fceeaf0229b2ca4f0d this was removed. --- src/qemu/qemu_cgroup.c | 7 ++----- src/qemu/qemu_domain.c | 15 +++++++++++++++ src/qemu/qemu_domain.h | 2 ++ src/qemu/qemu_driver.c | 29 +++++++++++++---------------- src/qemu/qemu_process.c | 7 ++++--- 5 files changed, 36 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index fa0b97b..a9cf9e8 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -1000,12 +1000,9 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm) !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) return 0; - if (priv->nvcpupids == 0 || priv->vcpupids[0] == vm->pid) { - /* If we don't know VCPU<->PID mapping or all vcpu runs in the same - * thread, we cannot control each vcpu. - */ + /* If vCPU<->pid mapping is missing we can't do vCPU pinning */ + if (!qemuDomainHasVcpuPids(vm)) return 0; - } if (virDomainNumatuneGetMode(vm->def->numa, -1, &mem_mode) == 0 && mem_mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT && diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c7687b7..32ee5de 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4098,3 +4098,18 @@ qemuDomainRequiresMlock(virDomainDefPtr def) return false; } + + +/** + * qemuDomainHasVcpuPids: + * @vm: Domain object + * + * Returns true if we were able to successfully detect vCPU pids for the VM. + */ +bool +qemuDomainHasVcpuPids(virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + + return priv->nvcpupids > 0; +} diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 31c7d33..5e2b699 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -505,4 +505,6 @@ int qemuDomainDefValidateMemoryHotplug(const virDomainDef *def, virQEMUCapsPtr qemuCaps, const virDomainMemoryDef *mem); +bool qemuDomainHasVcpuPids(virDomainObjPtr vm); + #endif /* __QEMU_DOMAIN_H__ */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5c3703f..3b3761a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1428,7 +1428,7 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm, virVcpuInfoPtr info, int maxinfo, size_t i, v; qemuDomainObjPrivatePtr priv = vm->privateData; - if (priv->vcpupids == NULL) { + if (!qemuDomainHasVcpuPids(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cpu affinity is not supported")); return -1; @@ -5118,7 +5118,7 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, } if (def) { - if (priv->vcpupids == NULL) { + if (!qemuDomainHasVcpuPids(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cpu affinity is not supported")); goto endjob; @@ -10287,21 +10287,18 @@ qemuSetVcpusBWLive(virDomainObjPtr vm, virCgroupPtr cgroup, if (period == 0 && quota == 0) return 0; - /* If we does not know VCPU<->PID mapping or all vcpu runs in the same - * thread, we cannot control each vcpu. So we only modify cpu bandwidth - * when each vcpu has a separated thread. - */ - if (priv->nvcpupids != 0 && priv->vcpupids[0] != vm->pid) { - for (i = 0; i < priv->nvcpupids; i++) { - if (virCgroupNewThread(cgroup, VIR_CGROUP_THREAD_VCPU, i, - false, &cgroup_vcpu) < 0) - goto cleanup; + if (!qemuDomainHasVcpuPids(vm)) + return 0; - if (qemuSetupCgroupVcpuBW(cgroup_vcpu, period, quota) < 0) - goto cleanup; + for (i = 0; i < priv->nvcpupids; i++) { + if (virCgroupNewThread(cgroup, VIR_CGROUP_THREAD_VCPU, i, + false, &cgroup_vcpu) < 0) + goto cleanup; - virCgroupFree(&cgroup_vcpu); - } + if (qemuSetupCgroupVcpuBW(cgroup_vcpu, period, quota) < 0) + goto cleanup; + + virCgroupFree(&cgroup_vcpu); } return 0; @@ -10604,7 +10601,7 @@ qemuGetVcpusBWLive(virDomainObjPtr vm, int ret = -1; priv = vm->privateData; - if (priv->nvcpupids == 0 || priv->vcpupids[0] == vm->pid) { + if (!qemuDomainHasVcpuPids(vm)) { /* We do not create sub dir for each vcpu */ rc = qemuGetVcpuBWLive(priv->cgroup, period, quota); if (rc < 0) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a3ddb4a..2de2248 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2240,12 +2240,13 @@ qemuProcessSetVcpuAffinities(virDomainObjPtr vm) virDomainPinDefPtr pininfo; int n; int ret = -1; - VIR_DEBUG("Setting affinity on CPUs nvcpupin=%zu nvcpus=%d nvcpupids=%d", - def->cputune.nvcpupin, virDomainDefGetVcpus(def), priv->nvcpupids); + VIR_DEBUG("Setting affinity on CPUs nvcpupin=%zu nvcpus=%d hasVcpupids=%d", + def->cputune.nvcpupin, virDomainDefGetVcpus(def), + qemuDomainHasVcpuPids(vm)); if (!def->cputune.nvcpupin) return 0; - if (priv->vcpupids == NULL) { + if (!qemuDomainHasVcpuPids(vm)) { /* If any CPU has custom affinity that differs from the * VM default affinity, we must reject it */ -- 2.6.2

Instead of directly accessing the array add a helper to do this. --- src/qemu/qemu_cgroup.c | 3 ++- src/qemu/qemu_domain.c | 20 ++++++++++++++++++++ src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 7 ++++--- src/qemu/qemu_process.c | 5 ++--- 5 files changed, 29 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index a9cf9e8..402940f 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -1018,7 +1018,8 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm) goto cleanup; /* move the thread for vcpu to sub dir */ - if (virCgroupAddTask(cgroup_vcpu, priv->vcpupids[i]) < 0) + if (virCgroupAddTask(cgroup_vcpu, + qemuDomainGetVcpuPid(vm, i)) < 0) goto cleanup; if (period || quota) { diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 32ee5de..cdda129 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4113,3 +4113,23 @@ qemuDomainHasVcpuPids(virDomainObjPtr vm) return priv->nvcpupids > 0; } + + +/** + * qemuDomainGetVcpuPid: + * @vm: domain object + * @vcpu: cpu id + * + * Returns the vCPU pid. If @vcpu is offline or out of range 0 is returned. + */ +pid_t +qemuDomainGetVcpuPid(virDomainObjPtr vm, + unsigned int vcpu) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (vcpu >= priv->nvcpupids) + return 0; + + return priv->vcpupids[vcpu]; +} diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 5e2b699..916d5d3 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -506,5 +506,6 @@ int qemuDomainDefValidateMemoryHotplug(const virDomainDef *def, const virDomainMemoryDef *mem); bool qemuDomainHasVcpuPids(virDomainObjPtr vm); +pid_t qemuDomainGetVcpuPid(virDomainObjPtr vm, unsigned int vcpu); #endif /* __QEMU_DOMAIN_H__ */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3b3761a..14a325a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1449,7 +1449,7 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm, virVcpuInfoPtr info, int maxinfo, &(info[i].cpu), NULL, vm->pid, - priv->vcpupids[i]) < 0) { + qemuDomainGetVcpuPid(vm, i)) < 0) { virReportSystemError(errno, "%s", _("cannot get vCPU placement & pCPU time")); return -1; @@ -1462,7 +1462,7 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm, virVcpuInfoPtr info, int maxinfo, unsigned char *cpumap = VIR_GET_CPUMAP(cpumaps, maplen, v); virBitmapPtr map = NULL; - if (!(map = virProcessGetAffinity(priv->vcpupids[v]))) + if (!(map = virProcessGetAffinity(qemuDomainGetVcpuPid(vm, v)))) return -1; virBitmapToDataBuf(map, cpumap, maplen); @@ -5156,7 +5156,8 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, goto endjob; } } else { - if (virProcessSetAffinity(priv->vcpupids[vcpu], pcpumap) < 0) { + if (virProcessSetAffinity(qemuDomainGetVcpuPid(vm, vcpu), + pcpumap) < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, _("failed to set cpu affinity for vcpu %d"), vcpu); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 2de2248..bfb338e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2235,7 +2235,6 @@ qemuProcessSetLinkStates(virQEMUDriverPtr driver, static int qemuProcessSetVcpuAffinities(virDomainObjPtr vm) { - qemuDomainObjPrivatePtr priv = vm->privateData; virDomainDefPtr def = vm->def; virDomainPinDefPtr pininfo; int n; @@ -2268,7 +2267,7 @@ qemuProcessSetVcpuAffinities(virDomainObjPtr vm) n))) continue; - if (virProcessSetAffinity(priv->vcpupids[n], + if (virProcessSetAffinity(qemuDomainGetVcpuPid(vm, n), pininfo->cpumask) < 0) { goto cleanup; } @@ -2356,7 +2355,7 @@ qemuProcessSetSchedulers(virDomainObjPtr vm) size_t i = 0; for (i = 0; i < priv->nvcpupids; i++) { - if (qemuProcessSetSchedParams(i, priv->vcpupids[i], + if (qemuProcessSetSchedParams(i, qemuDomainGetVcpuPid(vm, i), vm->def->cputune.nvcpusched, vm->def->cputune.vcpusched) < 0) return -1; -- 2.6.2

Change some of the control structures and switch to using the new vcpu structure. --- src/qemu/qemu_driver.c | 77 ++++++++++++++++++++++++++++---------------------- 1 file changed, 43 insertions(+), 34 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 14a325a..783a7cd 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1422,11 +1422,17 @@ qemuGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss, static int -qemuDomainHelperGetVcpus(virDomainObjPtr vm, virVcpuInfoPtr info, int maxinfo, - unsigned char *cpumaps, int maplen) +qemuDomainHelperGetVcpus(virDomainObjPtr vm, + virVcpuInfoPtr info, + int maxinfo, + unsigned char *cpumaps, + int maplen) { - size_t i, v; - qemuDomainObjPrivatePtr priv = vm->privateData; + size_t ncpuinfo = 0; + size_t i; + + if (maxinfo == 0) + return 0; if (!qemuDomainHasVcpuPids(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -1434,43 +1440,46 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm, virVcpuInfoPtr info, int maxinfo, return -1; } - /* Clamp to actual number of vcpus */ - if (maxinfo > priv->nvcpupids) - maxinfo = priv->nvcpupids; - - if (maxinfo >= 1) { - if (info != NULL) { - memset(info, 0, sizeof(*info) * maxinfo); - for (i = 0; i < maxinfo; i++) { - info[i].number = i; - info[i].state = VIR_VCPU_RUNNING; - - if (qemuGetProcessInfo(&(info[i].cpuTime), - &(info[i].cpu), - NULL, - vm->pid, - qemuDomainGetVcpuPid(vm, i)) < 0) { - virReportSystemError(errno, "%s", - _("cannot get vCPU placement & pCPU time")); - return -1; - } + if (info) + memset(info, 0, sizeof(*info) * maxinfo); + + if (cpumaps) + memset(cpumaps, 0, sizeof(*cpumaps) * maxinfo); + + for (i = 0; i < virDomainDefGetVcpusMax(vm->def) && ncpuinfo < maxinfo; i++) { + virDomainVcpuInfoPtr vcpu = virDomainDefGetVcpu(vm->def, i); + pid_t vcpupid = qemuDomainGetVcpuPid(vm, i); + + if (!vcpu->online) + continue; + + if (info) { + info[i].number = i; + info[i].state = VIR_VCPU_RUNNING; + + if (qemuGetProcessInfo(&(info[i].cpuTime), &(info[i].cpu), NULL, + vm->pid, vcpupid) < 0) { + virReportSystemError(errno, "%s", + _("cannot get vCPU placement & pCPU time")); + return -1; } } - if (cpumaps != NULL) { - for (v = 0; v < maxinfo; v++) { - unsigned char *cpumap = VIR_GET_CPUMAP(cpumaps, maplen, v); - virBitmapPtr map = NULL; + if (cpumaps) { + unsigned char *cpumap = VIR_GET_CPUMAP(cpumaps, maplen, i); + virBitmapPtr map = NULL; - if (!(map = virProcessGetAffinity(qemuDomainGetVcpuPid(vm, v)))) - return -1; + if (!(map = virProcessGetAffinity(vcpupid))) + return -1; - virBitmapToDataBuf(map, cpumap, maplen); - virBitmapFree(map); - } + virBitmapToDataBuf(map, cpumap, maplen); + virBitmapFree(map); } + + ncpuinfo++; } - return maxinfo; + + return ncpuinfo; } -- 2.6.2

Use the proper data structures for the iteration since ncpupids will be made private later. --- src/qemu/qemu_cgroup.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 402940f..c38d760 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -795,7 +795,12 @@ qemuRestoreCgroupState(virDomainObjPtr vm) if (virCgroupSetCpusetMems(priv->cgroup, mem_mask) < 0) goto error; - 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 || virCgroupSetCpusetMemoryMigrate(cgroup_temp, true) < 0 || @@ -1011,7 +1016,12 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm) &mem_mask, -1) < 0) goto cleanup; - for (i = 0; i < priv->nvcpupids; i++) { + for (i = 0; i < virDomainDefGetVcpusMax(def); i++) { + virDomainVcpuInfoPtr vcpu = virDomainDefGetVcpu(def, i); + + if (!vcpu->online) + continue; + virCgroupFree(&cgroup_vcpu); if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_VCPU, i, true, &cgroup_vcpu) < 0) -- 2.6.2

On Fri, Nov 27, 2015 at 17:12:44 +0100, Peter Krempa wrote:
This series is getting rather big. The target is to refactor the way libvirt stores info about vCPUs into a single structure (okay, two structures for the qemu driver. Part 1 is not yet completely there, well, not even halfway.
Future work will involve fully allocating priv->vcpupids to the maxcpus size and moving around few other bits of data in cputune and other parts to the new structure. Yet another follow up work is then to add new APIs for vCPU hotplug, which will enable adding vCPUs sparsely (useful if you have NUMA).
Since this refactor will result in tracking all vcpu-related data in one struct, the result will automagically fix a few bugs where we'd end up with invalid config after vcpu unplug or other operations.
Version 2 does not contain already pushed patches and incorporates feedback from John's review. Since I've changed quite a few things I'm reposting this.
Since all the patches were ACKed previously and the 1.3.0 release is out and nobody objected I'll push these shortly. Peter
participants (1)
-
Peter Krempa