[libvirt] [PATCH v2 0/7] Allow memory hotplug without NUMA on ppc64
This fixes most of the stuff pointed out in the review. Peter Krempa (7): qemu: command: Make qemuBuildMemoryBackendStr usable without NUMA qemu: command: Always execute memory device formatter qemu: domain: Add common function to perform memory hotplug checks qemu: command: Move dimm device checks from formatter to checker conf: Prepare making memory device target node optional qemu: command: Prepare memory device def formatter for missing target node qemu: ppc64: Support memory hotplug without NUMA enabled docs/formatdomain.html.in | 5 +- docs/schemas/domaincommon.rng | 8 +- src/conf/domain_conf.c | 16 +- src/conf/domain_conf.h | 2 +- src/qemu/qemu_command.c | 161 ++++++------------ src/qemu/qemu_command.h | 4 +- src/qemu/qemu_domain.c | 179 ++++++++++++++++++++- src/qemu/qemu_domain.h | 4 + src/qemu/qemu_hotplug.c | 11 +- .../qemuxml2argv-memory-hotplug-ppc64-nonuma.args | 28 ++++ .../qemuxml2argv-memory-hotplug-ppc64-nonuma.xml | 38 +++++ tests/qemuxml2argvtest.c | 2 + 12 files changed, 314 insertions(+), 144 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-ppc64-nonuma.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-ppc64-nonuma.xml -- 2.6.2
Make the function usable so that -1 can be passed to it as cell ID so that we can later enable memory hotplug on non-NUMA guests for certain architectures. --- src/qemu/qemu_command.c | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 792ada3..5e1e04e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4965,7 +4965,8 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, * qemuBuildMemoryBackendStr: * @size: size of the memory device in kibibytes * @pagesize: size of the requested memory page in KiB, 0 for default - * @guestNode: NUMA node in the guest that the memory object will be attached to + * @guestNode: NUMA node in the guest that the memory object will be attached + * to, or -1 if NUMA is not used in the guest * @hostNodes: map of host nodes to alloc the memory in, NULL for default * @autoNodeset: fallback nodeset in case of automatic numa placement * @def: domain definition object @@ -5011,19 +5012,19 @@ qemuBuildMemoryBackendStr(unsigned long long size, *backendProps = NULL; *backendType = NULL; - /* memory devices could provide a invalid guest node */ - if (guestNode >= virDomainNumaGetNodeCount(def->numa)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("can't add memory backend for guest node '%d' as " - "the guest has only '%zu' NUMA nodes configured"), - guestNode, virDomainNumaGetNodeCount(def->numa)); - return -1; - } + if (guestNode >= 0) { + /* memory devices could provide a invalid guest node */ + if (guestNode >= virDomainNumaGetNodeCount(def->numa)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("can't add memory backend for guest node '%d' as " + "the guest has only '%zu' NUMA nodes configured"), + guestNode, virDomainNumaGetNodeCount(def->numa)); + return -1; + } - if (!(props = virJSONValueNewObject())) - return -1; + memAccess = virDomainNumaGetNodeMemoryAccessMode(def->numa, guestNode); + } - memAccess = virDomainNumaGetNodeMemoryAccessMode(def->numa, guestNode); if (virDomainNumatuneGetMode(def->numa, guestNode, &mode) < 0 && virDomainNumatuneGetMode(def->numa, -1, &mode) < 0) mode = VIR_DOMAIN_NUMATUNE_MEM_STRICT; @@ -5040,6 +5041,10 @@ qemuBuildMemoryBackendStr(unsigned long long size, continue; } + /* just find the master hugepage in case we don't use NUMA */ + if (guestNode < 0) + continue; + if (virBitmapGetBit(hugepage->nodemask, guestNode, &thisHugepage) < 0) { /* Ignore this error. It's not an error after all. Well, @@ -5073,6 +5078,9 @@ qemuBuildMemoryBackendStr(unsigned long long size, hugepage = NULL; } + if (!(props = virJSONValueNewObject())) + return -1; + if (pagesize || hugepage) { if (pagesize) { /* Now lets see, if the huge page we want to use is even mounted -- 2.6.2
Since we already make sure before that the domain configuration is valid we may execute it always at the cost of doing 0 iterations of the for loop. This patch will simplify later refactor as it will avoid whitespace changes. --- src/qemu/qemu_command.c | 47 +++++++++++++++++++++++------------------------ 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5e1e04e..c0b361b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9401,38 +9401,37 @@ qemuBuildCommandLine(virConnectPtr conn, } } - if (virDomainNumaGetNodeCount(def->numa)) { - if (qemuBuildNumaArgStr(cfg, def, cmd, qemuCaps, nodeset) < 0) + if (virDomainNumaGetNodeCount(def->numa) && + qemuBuildNumaArgStr(cfg, def, cmd, qemuCaps, nodeset) < 0) goto error; - /* memory hotplug requires NUMA to be enabled - we already checked - * that memory devices are present only when NUMA is */ + /* memory hotplug requires NUMA to be enabled - we already checked + * that memory devices are present only when NUMA is */ - if (def->nmems > def->mem.memory_slots) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("memory device count '%zu' exceeds slots count '%u'"), - def->nmems, def->mem.memory_slots); - goto error; - } - - for (i = 0; i < def->nmems; i++) { - char *backStr; - char *dimmStr; - - if (!(backStr = qemuBuildMemoryDimmBackendStr(def->mems[i], def, - qemuCaps, cfg))) - goto error; + if (def->nmems > def->mem.memory_slots) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("memory device count '%zu' exceeds slots count '%u'"), + def->nmems, def->mem.memory_slots); + goto error; + } - if (!(dimmStr = qemuBuildMemoryDeviceStr(def->mems[i], def, qemuCaps))) { - VIR_FREE(backStr); - goto error; - } + for (i = 0; i < def->nmems; i++) { + char *backStr; + char *dimmStr; - virCommandAddArgList(cmd, "-object", backStr, "-device", dimmStr, NULL); + if (!(backStr = qemuBuildMemoryDimmBackendStr(def->mems[i], def, + qemuCaps, cfg))) + goto error; + if (!(dimmStr = qemuBuildMemoryDeviceStr(def->mems[i], def, qemuCaps))) { VIR_FREE(backStr); - VIR_FREE(dimmStr); + goto error; } + + virCommandAddArgList(cmd, "-object", backStr, "-device", dimmStr, NULL); + + VIR_FREE(backStr); + VIR_FREE(dimmStr); } virCommandAddArgList(cmd, "-uuid", uuid, NULL); -- 2.6.2
Add a function that will aggregate various checks related to memory hotplug so that they aren't scattered accross various parts of the code. --- src/qemu/qemu_command.c | 26 ++--------------- src/qemu/qemu_domain.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 4 +++ src/qemu/qemu_hotplug.c | 9 ++---- 4 files changed, 88 insertions(+), 29 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c0b361b..5dbe650 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9326,26 +9326,12 @@ qemuBuildCommandLine(virConnectPtr conn, qemuDomainAlignMemorySizes(def) < 0) goto error; + if (qemuDomainDefValidateMemoryHotplug(def, qemuCaps, NULL) < 0) + goto error; + virCommandAddArg(cmd, "-m"); if (virDomainDefHasMemoryHotplug(def)) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PC_DIMM)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("memory hotplug isn't supported by this QEMU binary")); - goto error; - } - - /* due to guest support, qemu would silently enable NUMA with one node - * once the memory hotplug backend is enabled. To avoid possible - * confusion we will enforce user originated numa configuration along - * with memory hotplug. */ - if (virDomainNumaGetNodeCount(def->numa) == 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("At least one numa node has to be configured when " - "enabling memory hotplug")); - goto error; - } - /* Use the 'k' suffix to let qemu handle the units */ virCommandAddArgFormat(cmd, "size=%lluk,slots=%u,maxmem=%lluk", virDomainDefGetMemoryInitial(def), @@ -9408,12 +9394,6 @@ qemuBuildCommandLine(virConnectPtr conn, /* memory hotplug requires NUMA to be enabled - we already checked * that memory devices are present only when NUMA is */ - if (def->nmems > def->mem.memory_slots) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("memory device count '%zu' exceeds slots count '%u'"), - def->nmems, def->mem.memory_slots); - goto error; - } for (i = 0; i < def->nmems; i++) { char *backStr; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 416ab5b..cca66ac 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3553,6 +3553,84 @@ qemuDomainMachineIsS390CCW(const virDomainDef *def) /** + * qemuDomainDefValidateMemoryHotplug: + * @def: domain definition + * @qemuCaps: qemu capabilities object + * @mem: definition of memory device that is to be added to @def with hotplug, + * NULL in case of regular VM startup + * + * Validates that the domain definition and memory modules have valid + * configuration and are possibly able to accept @mem via hotplug if it's + * non-NULL. + * + * Returns 0 on success; -1 and a libvirt error on error. + */ +int +qemuDomainDefValidateMemoryHotplug(const virDomainDef *def, + virQEMUCapsPtr qemuCaps, + const virDomainMemoryDef *mem) +{ + unsigned int nmems = def->nmems; + unsigned long long hotplugSpace; + unsigned long long hotplugMemory = 0; + size_t i; + + hotplugSpace = def->mem.max_memory - virDomainDefGetMemoryInitial(def); + + if (mem) { + nmems++; + hotplugMemory = mem->size; + } + + if (!virDomainDefHasMemoryHotplug(def)) { + if (nmems) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("cannot use/hotplug a memory device when domain " + "'maxMemory' is not defined")); + return -1; + } + + return 0; + } + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PC_DIMM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("memory hotplug isn't supported by this QEMU binary")); + return -1; + } + + /* due to guest support, qemu would silently enable NUMA with one node + * once the memory hotplug backend is enabled. To avoid possible + * confusion we will enforce user originated numa configuration along + * with memory hotplug. */ + if (virDomainNumaGetNodeCount(def->numa) == 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("At least one numa node has to be configured when " + "enabling memory hotplug")); + return -1; + } + + if (nmems > def->mem.memory_slots) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("memory device count '%u' exceeds slots count '%u'"), + nmems, def->mem.memory_slots); + return -1; + } + + for (i = 0; i < def->nmems; i++) + hotplugMemory += def->mems[i]->size; + + if (hotplugMemory > hotplugSpace) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("memory device total size exceeds hotplug space")); + return -1; + } + + return 0; +} + + +/** * qemuDomainUpdateCurrentMemorySize: * * Updates the current balloon size from the monitor if necessary. In case when diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 4be998c..90b482d 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -485,4 +485,8 @@ int qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver, unsigned long long qemuDomainGetMlockLimitBytes(virDomainDefPtr def); bool qemuDomainRequiresMlock(virDomainDefPtr def); +int qemuDomainDefValidateMemoryHotplug(const virDomainDef *def, + virQEMUCapsPtr qemuCaps, + const virDomainMemoryDef *mem); + #endif /* __QEMU_DOMAIN_H__ */ diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 9a8bd9e..ccef8d5 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1767,11 +1767,10 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, int id; int ret = -1; - if (vm->def->nmems == vm->def->mem.memory_slots) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("no free memory device slot available")); + qemuDomainMemoryDeviceAlignSize(vm->def, mem); + + if (qemuDomainDefValidateMemoryHotplug(vm->def, priv->qemuCaps, mem) < 0) goto cleanup; - } if (virAsprintf(&mem->info.alias, "dimm%zu", vm->def->nmems) < 0) goto cleanup; @@ -1785,8 +1784,6 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, if (!(devstr = qemuBuildMemoryDeviceStr(mem, vm->def, priv->qemuCaps))) goto cleanup; - qemuDomainMemoryDeviceAlignSize(vm->def, mem); - if (qemuBuildMemoryBackendStr(mem->size, mem->pagesize, mem->targetNode, mem->sourceNodes, NULL, vm->def, priv->qemuCaps, cfg, -- 2.6.2
Aggregate the checks of the dimm device into the verification function rather than having them in the formatter. --- src/qemu/qemu_command.c | 65 ++-------------------------------- src/qemu/qemu_command.h | 4 +-- src/qemu/qemu_domain.c | 92 ++++++++++++++++++++++++++++++++++++++++++++----- src/qemu/qemu_hotplug.c | 2 +- 4 files changed, 87 insertions(+), 76 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5dbe650..e6c240c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5272,44 +5272,8 @@ qemuBuildMemoryDimmBackendStr(virDomainMemoryDefPtr mem, } -static bool -qemuCheckMemoryDimmConflict(virDomainDefPtr def, - virDomainMemoryDefPtr mem) -{ - size_t i; - - for (i = 0; i < def->nmems; i++) { - virDomainMemoryDefPtr tmp = def->mems[i]; - - if (tmp == mem || - tmp->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM) - continue; - - if (mem->info.addr.dimm.slot == tmp->info.addr.dimm.slot) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("memory device slot '%u' is already being " - "used by another memory device"), - mem->info.addr.dimm.slot); - return true; - } - - if (mem->info.addr.dimm.base != 0 && - mem->info.addr.dimm.base == tmp->info.addr.dimm.base) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("memory device base '0x%llx' is already being " - "used by another memory device"), - mem->info.addr.dimm.base); - return true; - } - } - - return false; -} - char * -qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem, - virDomainDefPtr def, - virQEMUCapsPtr qemuCaps) +qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -5321,35 +5285,10 @@ qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem, switch ((virDomainMemoryModel) mem->model) { case VIR_DOMAIN_MEMORY_MODEL_DIMM: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PC_DIMM)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this qemu doesn't support the pc-dimm device")); - return NULL; - } - - if (mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM && - mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("only 'dimm' addresses are supported for the " - "pc-dimm device")); - return NULL; - } - - if (mem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM && - mem->info.addr.dimm.slot >= def->mem.memory_slots) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("memory device slot '%u' exceeds slots count '%u'"), - mem->info.addr.dimm.slot, def->mem.memory_slots); - return NULL; - } - virBufferAsprintf(&buf, "pc-dimm,node=%d,memdev=mem%s,id=%s", mem->targetNode, mem->info.alias, mem->info.alias); if (mem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM) { - if (qemuCheckMemoryDimmConflict(def, mem)) - return NULL; - virBufferAsprintf(&buf, ",slot=%d", mem->info.addr.dimm.slot); virBufferAsprintf(&buf, ",addr=%llu", mem->info.addr.dimm.base); } @@ -9403,7 +9342,7 @@ qemuBuildCommandLine(virConnectPtr conn, qemuCaps, cfg))) goto error; - if (!(dimmStr = qemuBuildMemoryDeviceStr(def->mems[i], def, qemuCaps))) { + if (!(dimmStr = qemuBuildMemoryDeviceStr(def->mems[i]))) { VIR_FREE(backStr); goto error; } diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index d1af3b7..6e550a8 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -173,9 +173,7 @@ int qemuBuildMemoryBackendStr(unsigned long long size, virJSONValuePtr *backendProps, bool force); -char *qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem, - virDomainDefPtr def, - virQEMUCapsPtr qemuCaps); +char *qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem); /* Legacy, pre device support */ char *qemuBuildPCIHostdevPCIDevStr(virDomainHostdevDefPtr dev, diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index cca66ac..e17c57d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1347,14 +1347,6 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, } } - if (dev->type == VIR_DOMAIN_DEVICE_MEMORY && - !virDomainDefHasMemoryHotplug(def)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("maxMemory has to be specified when using memory " - "devices ")); - goto cleanup; - } - ret = 0; cleanup: @@ -3552,6 +3544,79 @@ qemuDomainMachineIsS390CCW(const virDomainDef *def) } +static bool +qemuCheckMemoryDimmConflict(const virDomainDef *def, + const virDomainMemoryDef *mem) +{ + size_t i; + + for (i = 0; i < def->nmems; i++) { + virDomainMemoryDefPtr tmp = def->mems[i]; + + if (tmp == mem || + tmp->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM) + continue; + + if (mem->info.addr.dimm.slot == tmp->info.addr.dimm.slot) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("memory device slot '%u' is already being " + "used by another memory device"), + mem->info.addr.dimm.slot); + return true; + } + + if (mem->info.addr.dimm.base != 0 && + mem->info.addr.dimm.base == tmp->info.addr.dimm.base) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("memory device base '0x%llx' is already being " + "used by another memory device"), + mem->info.addr.dimm.base); + return true; + } + } + + return false; +} +static int +qemuDomainDefValidateMemoryHotplugDevice(const virDomainMemoryDef *mem, + const virDomainDef *def) +{ + switch ((virDomainMemoryModel) mem->model) { + case VIR_DOMAIN_MEMORY_MODEL_DIMM: + if (mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM && + mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only 'dimm' addresses are supported for the " + "pc-dimm device")); + return -1; + } + + if (mem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM) { + if (mem->info.addr.dimm.slot >= def->mem.memory_slots) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("memory device slot '%u' exceeds slots " + "count '%u'"), + mem->info.addr.dimm.slot, def->mem.memory_slots); + return -1; + } + + + if (qemuCheckMemoryDimmConflict(def, mem)) + return -1; + } + break; + + case VIR_DOMAIN_MEMORY_MODEL_NONE: + case VIR_DOMAIN_MEMORY_MODEL_LAST: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("invalid memory device type")); + return -1; + } + + return 0; +} + + /** * qemuDomainDefValidateMemoryHotplug: * @def: domain definition @@ -3580,6 +3645,9 @@ qemuDomainDefValidateMemoryHotplug(const virDomainDef *def, if (mem) { nmems++; hotplugMemory = mem->size; + + if (qemuDomainDefValidateMemoryHotplugDevice(mem, def) < 0) + return -1; } if (!virDomainDefHasMemoryHotplug(def)) { @@ -3617,9 +3685,15 @@ qemuDomainDefValidateMemoryHotplug(const virDomainDef *def, return -1; } - for (i = 0; i < def->nmems; i++) + for (i = 0; i < def->nmems; i++) { hotplugMemory += def->mems[i]->size; + /* already existing devices don't need to be checked on hotplug */ + if (!mem && + qemuDomainDefValidateMemoryHotplugDevice(def->mems[i], def) < 0) + return -1; + } + if (hotplugMemory > hotplugSpace) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("memory device total size exceeds hotplug space")); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index ccef8d5..89e5c0d 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1781,7 +1781,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, if (vm->def->mem.cur_balloon == virDomainDefGetMemoryActual(vm->def)) fix_balloon = true; - if (!(devstr = qemuBuildMemoryDeviceStr(mem, vm->def, priv->qemuCaps))) + if (!(devstr = qemuBuildMemoryDeviceStr(mem))) goto cleanup; if (qemuBuildMemoryBackendStr(mem->size, mem->pagesize, -- 2.6.2
Adjust the config code so that it does not enforce that target memory node is specified. To avoid breakage, adjust the qemu memory hotplug config checker to disallow such config for now. --- docs/formatdomain.html.in | 5 +++-- docs/schemas/domaincommon.rng | 8 +++++--- src/conf/domain_conf.c | 16 +++++++++++----- src/conf/domain_conf.h | 2 +- src/qemu/qemu_domain.c | 7 +++++++ 5 files changed, 27 insertions(+), 11 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index c88b032..e5e0167 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6300,8 +6300,9 @@ qemu-kvm -net nic,model=? /dev/null added memory as a scaled integer. </p> <p> - The mandatory <code>node</code> subelement configures the guest NUMA - node to attach the memory to. + The <code>node</code> subelement configures the guest NUMA node to + attach the memory to. The element shall be used only if the guest has + NUMA nodes configured. </p> </dd> </dl> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index f196177..994face 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4532,9 +4532,11 @@ <element name="size"> <ref name="scaledInteger"/> </element> - <element name="node"> - <ref name="unsignedInt"/> - </element> + <optional> + <element name="node"> + <ref name="unsignedInt"/> + </element> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2edf123..52b76fb 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12544,10 +12544,15 @@ virDomainMemoryTargetDefParseXML(xmlNodePtr node, int ret = -1; xmlNodePtr save = ctxt->node; ctxt->node = node; + int rv; - if (virXPathUInt("string(./node)", ctxt, &def->targetNode) < 0) { + /* initialize to value which marks that the user didn't specify it */ + def->targetNode = -1; + + if ((rv = virXPathInt("string(./node)", ctxt, &def->targetNode)) == -2 || + (rv == 0 && def->targetNode < 0)) { virReportError(VIR_ERR_XML_ERROR, "%s", - _("invalid or missing value of memory device node")); + _("invalid value of memory device node")); goto cleanup; } @@ -17700,8 +17705,8 @@ virDomainMemoryDefCheckABIStability(virDomainMemoryDefPtr src, if (src->targetNode != dst->targetNode) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target memory device targetNode '%u' " - "doesn't match source targetNode '%u'"), + _("Target memory device targetNode '%d' " + "doesn't match source targetNode '%d'"), dst->targetNode, src->targetNode); return false; } @@ -20791,7 +20796,8 @@ virDomainMemoryTargetDefFormat(virBufferPtr buf, virBufferAdjustIndent(buf, 2); virBufferAsprintf(buf, "<size unit='KiB'>%llu</size>\n", def->size); - virBufferAsprintf(buf, "<node>%u</node>\n", def->targetNode); + if (def->targetNode >= 0) + virBufferAsprintf(buf, "<node>%d</node>\n", def->targetNode); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</target>\n"); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f10b534..8d43ee6 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2022,7 +2022,7 @@ struct _virDomainMemoryDef { /* target */ int model; /* virDomainMemoryModel */ - unsigned int targetNode; + int targetNode; unsigned long long size; /* kibibytes */ virDomainDeviceInfo info; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index e17c57d..69ddeb4 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3591,6 +3591,13 @@ qemuDomainDefValidateMemoryHotplugDevice(const virDomainMemoryDef *mem, return -1; } + if (mem->targetNode == -1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("target NUMA node needs to be specified for " + "memory device")); + return -1; + } + if (mem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM) { if (mem->info.addr.dimm.slot >= def->mem.memory_slots) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, -- 2.6.2
Prepare the command line generator for the possibility that in some configurations the target NUMA node info will be missing. --- src/qemu/qemu_command.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e6c240c..8fdf90c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5285,8 +5285,13 @@ qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem) switch ((virDomainMemoryModel) mem->model) { case VIR_DOMAIN_MEMORY_MODEL_DIMM: - virBufferAsprintf(&buf, "pc-dimm,node=%d,memdev=mem%s,id=%s", - mem->targetNode, mem->info.alias, mem->info.alias); + virBufferAddLit(&buf, "pc-dimm,"); + + if (mem->targetNode >= 0) + virBufferAsprintf(&buf, "node=%d,", mem->targetNode); + + virBufferAsprintf(&buf, "memdev=mem%s,id=%s", + mem->info.alias, mem->info.alias); if (mem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM) { virBufferAsprintf(&buf, ",slot=%d", mem->info.addr.dimm.slot); -- 2.6.2
ppc64 guests don't require adding a NUMA node for hotplug memory to work. Lift the requirement and add test cases. --- src/qemu/qemu_domain.c | 32 ++++++++++-------- .../qemuxml2argv-memory-hotplug-ppc64-nonuma.args | 28 ++++++++++++++++ .../qemuxml2argv-memory-hotplug-ppc64-nonuma.xml | 38 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 4 files changed, 86 insertions(+), 14 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-ppc64-nonuma.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-ppc64-nonuma.xml diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 69ddeb4..dc02efe 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3591,11 +3591,13 @@ qemuDomainDefValidateMemoryHotplugDevice(const virDomainMemoryDef *mem, return -1; } - if (mem->targetNode == -1) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("target NUMA node needs to be specified for " - "memory device")); - return -1; + if (virDomainNumaGetNodeCount(def->numa) != 0) { + if (mem->targetNode == -1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("target NUMA node needs to be specifed for " + "memory device")); + return -1; + } } if (mem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM) { @@ -3674,15 +3676,17 @@ qemuDomainDefValidateMemoryHotplug(const virDomainDef *def, return -1; } - /* due to guest support, qemu would silently enable NUMA with one node - * once the memory hotplug backend is enabled. To avoid possible - * confusion we will enforce user originated numa configuration along - * with memory hotplug. */ - if (virDomainNumaGetNodeCount(def->numa) == 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("At least one numa node has to be configured when " - "enabling memory hotplug")); - return -1; + if (!ARCH_IS_PPC64(def->os.arch)) { + /* due to guest support, qemu would silently enable NUMA with one node + * once the memory hotplug backend is enabled. To avoid possible + * confusion we will enforce user originated numa configuration along + * with memory hotplug. */ + if (virDomainNumaGetNodeCount(def->numa) == 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("At least one numa node has to be configured when " + "enabling memory hotplug")); + return -1; + } } if (nmems > def->mem.memory_slots) { diff --git a/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-ppc64-nonuma.args b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-ppc64-nonuma.args new file mode 100644 index 0000000..8b6bf3e --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-ppc64-nonuma.args @@ -0,0 +1,28 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-ppc64 \ +-name QEMUGuest1 \ +-S \ +-M pseries \ +-m size=1310720k,slots=16,maxmem=4194304k \ +-smp 1 \ +-object memory-backend-ram,id=memdimm0,size=536870912 \ +-device pc-dimm,memdev=memdimm0,id=dimm0 \ +-object memory-backend-ram,id=memdimm1,size=536870912 \ +-device pc-dimm,memdev=memdimm1,id=dimm1 \ +-uuid 49545eb3-75e1-2d0a-acdd-f0294406c99e \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait \ +-no-acpi \ +-boot c \ +-kernel /media/ram/uImage \ +-initrd /media/ram/ramdisk \ +-append 'root=/dev/ram rw console=ttyS0,115200' \ +-usb \ +-serial pty \ +-device virtio-balloon-pci,id=balloon0,bus=pci,addr=0x2 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-ppc64-nonuma.xml b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-ppc64-nonuma.xml new file mode 100644 index 0000000..b6696e2 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-ppc64-nonuma.xml @@ -0,0 +1,38 @@ +<domain type='kvm'> + <name>QEMUGuest1</name> + <uuid>49545eb3-75e1-2d0a-acdd-f0294406c99e</uuid> + <maxMemory slots='16' unit='KiB'>4194304</maxMemory> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>hvm</type> + <kernel>/media/ram/uImage</kernel> + <initrd>/media/ram/ramdisk</initrd> + <cmdline>root=/dev/ram rw console=ttyS0,115200</cmdline> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <serial type='pty'> + <target port='0'/> + </serial> + <console type='pty'> + <target type='serial' port='0'/> + </console> + <memballoon model='virtio'/> + <memory model='dimm'> + <target> + <size unit='KiB'>523264</size> + </target> + </memory> + <memory model='dimm'> + <target> + <size unit='KiB'>524287</size> + </target> + </memory> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 256027e..cd44acf 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1651,6 +1651,8 @@ mymain(void) QEMU_CAPS_DEVICE, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE); DO_TEST("memory-hotplug-dimm-addr", QEMU_CAPS_DEVICE_PC_DIMM, QEMU_CAPS_NUMA, QEMU_CAPS_DEVICE, QEMU_CAPS_OBJECT_MEMORY_FILE); + DO_TEST("memory-hotplug-ppc64-nonuma", QEMU_CAPS_KVM, QEMU_CAPS_DEVICE_PC_DIMM, QEMU_CAPS_NUMA, + QEMU_CAPS_DEVICE, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE); DO_TEST("machine-aeskeywrap-on-caps", QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_AES_KEY_WRAP, -- 2.6.2
On 11/10/2015 09:44 AM, Peter Krempa wrote:
This fixes most of the stuff pointed out in the review.
Peter Krempa (7): qemu: command: Make qemuBuildMemoryBackendStr usable without NUMA qemu: command: Always execute memory device formatter qemu: domain: Add common function to perform memory hotplug checks qemu: command: Move dimm device checks from formatter to checker conf: Prepare making memory device target node optional qemu: command: Prepare memory device def formatter for missing target node qemu: ppc64: Support memory hotplug without NUMA enabled
docs/formatdomain.html.in | 5 +- docs/schemas/domaincommon.rng | 8 +- src/conf/domain_conf.c | 16 +- src/conf/domain_conf.h | 2 +- src/qemu/qemu_command.c | 161 ++++++------------ src/qemu/qemu_command.h | 4 +- src/qemu/qemu_domain.c | 179 ++++++++++++++++++++- src/qemu/qemu_domain.h | 4 + src/qemu/qemu_hotplug.c | 11 +- .../qemuxml2argv-memory-hotplug-ppc64-nonuma.args | 28 ++++ .../qemuxml2argv-memory-hotplug-ppc64-nonuma.xml | 38 +++++ tests/qemuxml2argvtest.c | 2 + 12 files changed, 314 insertions(+), 144 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-ppc64-nonuma.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-ppc64-nonuma.xml
ACK series John
participants (2)
-
John Ferlan -
Peter Krempa