On 09/07/2018 07:32 AM, marcandre.lureau@redhat.com wrote:
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Would be nice to have a few more words here. If you provide them I can add them... The if statement is difficult to read unless you know what each field really means. secondary question - should we document what gets used?, e.g.: https://libvirt.org/formatdomain.html#elementsMemoryBacking Seems to me the preference to use memfd is for memory backing using anonymous source for nvdimm's without a defined path, but sometimes my wording doesn't match reality.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_command.c | 61 +++++++++++++++++++++++++++++------------ 1 file changed, 43 insertions(+), 18 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c9e3a91e32..97cfc8a18d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3113,6 +3113,24 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd, return ret; }
+static int +qemuBuildMemoryBackendPropsShare(virJSONValuePtr props, + virDomainMemoryAccess memAccess) +{ + switch (memAccess) { + case VIR_DOMAIN_MEMORY_ACCESS_SHARED: + return virJSONValueObjectAdd(props, "b:share", true, NULL); + + case VIR_DOMAIN_MEMORY_ACCESS_PRIVATE: + return virJSONValueObjectAdd(props, "b:share", false, NULL); + + case VIR_DOMAIN_MEMORY_ACCESS_DEFAULT: + case VIR_DOMAIN_MEMORY_ACCESS_LAST: + break; + } + + return 0; +}
/** * qemuBuildMemoryBackendProps:
The comments should have been updated... In particular: "Then, if one of the two memory-backend-* should be used..."
@@ -3259,7 +3277,23 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps, if (!(props = virJSONValueNewObject())) return -1;
- if (useHugepage || mem->nvdimmPath || memAccess ||
Is this preference over "-ram" or "-file"? It would seem to me someone choosing "file" has a specific case and this is more for those other options where if capabilities exist, then we try to use them.
+ if (!mem->nvdimmPath && + def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_ANONYMOUS && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_MEMFD) && + (!useHugepage || virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_MEMFD_HUGETLB))) { + backendType = "memory-backend-memfd"; + + if (useHugepage && + (virJSONValueObjectAdd(props, "b:hugetlb", useHugepage, NULL) < 0 || + virJSONValueObjectAdd(props, "U:hugetlbsize", pagesize << 10, NULL) < 0)) { + goto cleanup; + } + + if (qemuBuildMemoryBackendPropsShare(props, memAccess) < 0) { + goto cleanup; + }
Running syntax-check would fail because of the { }
+ + } else if (useHugepage || mem->nvdimmPath || memAccess || def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_FILE) {
if (mem->nvdimmPath) { @@ -3297,20 +3331,8 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps, goto cleanup; }
- switch (memAccess) { - case VIR_DOMAIN_MEMORY_ACCESS_SHARED: - if (virJSONValueObjectAdd(props, "b:share", true, NULL) < 0) - goto cleanup; - break; - - case VIR_DOMAIN_MEMORY_ACCESS_PRIVATE: - if (virJSONValueObjectAdd(props, "b:share", false, NULL) < 0) - goto cleanup; - break; - - case VIR_DOMAIN_MEMORY_ACCESS_DEFAULT: - case VIR_DOMAIN_MEMORY_ACCESS_LAST: - break; + if (qemuBuildMemoryBackendPropsShare(props, memAccess) < 0) { + goto cleanup; }
Running syntax-check would fail here because of the { } All this is fix-able without you needing to post another series, but I need you to provide the verbiage for the intro and perhaps something that could be added to the web page. I can adjust the patch accordingly. Assuming of course Michal doesn't have other reservations... Reviewed-by: John Ferlan <jferlan@redhat.com> John
} else { backendType = "memory-backend-ram"; @@ -7609,7 +7631,8 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
if (virDomainNumatuneHasPerNodeBinding(def->numa) && !(virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM) || - virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE))) { + virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE) || + virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_MEMFD))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Per-node memory binding is not supported " "with this QEMU")); @@ -7618,7 +7641,8 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
if (def->mem.nhugepages && def->mem.hugepages[0].size != system_page_size && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) { + !(virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE) || + virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_MEMFD_HUGETLB))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("huge pages per NUMA node are not " "supported with this QEMU")); @@ -7635,7 +7659,8 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, * need to check which approach to use */ for (i = 0; i < ncells; i++) { if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM) || - virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) { + virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE) || + virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_MEMFD)) {
if ((rc = qemuBuildMemoryCellBackendStr(def, cfg, i, priv, &nodeBackends[i])) < 0)