[libvirt] [PATCH] qemuBuildMemPathStr: Produce -mem-path more frequently

Michal Privoznik posted 1 patch 5 years, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/925f592c75e51060175448dd54919c969174a69f.1535630512.git.mprivozn@redhat.com
src/qemu/qemu_command.c                            | 29 +++++++++++-----------
.../fd-memory-no-numa-topology.args                |  1 +
2 files changed, 16 insertions(+), 14 deletions(-)
[libvirt] [PATCH] qemuBuildMemPathStr: Produce -mem-path more frequently
Posted by Michal Privoznik 5 years, 7 months ago
https://bugzilla.redhat.com/show_bug.cgi?id=1622455

If a domain is configured to use <source type='file'/> under
<memoryBacking/> we have to honour that setting and produce
-mem-path on the command line. We are not doing so if domain has
no guest NUMA nodes nor hugepages.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_command.c                            | 29 +++++++++++-----------
 .../fd-memory-no-numa-topology.args                |  1 +
 2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8aa20496bc..df5e5841c2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7531,21 +7531,22 @@ qemuBuildMemPathStr(virQEMUDriverConfigPtr cfg,
     const long system_page_size = virGetSystemPageSizeKB();
     char *mem_path = NULL;
 
-    /*
-     *  No-op if hugepages were not requested.
-     */
-    if (!def->mem.nhugepages)
+    /* There are two cases where we want to put -mem-path onto
+     * the command line: First one is when there are no guest
+     * NUMA nodes and hugepages are configured. The second one is
+     * if user requested file allocation. */
+    if (def->mem.nhugepages &&
+        def->mem.hugepages[0].size != system_page_size) {
+        if (qemuGetDomainHupageMemPath(def, cfg,
+                                       def->mem.hugepages[0].size,
+                                       &mem_path) < 0)
+            return -1;
+    } else if (def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_FILE) {
+        if (qemuGetMemoryBackingPath(def, cfg, "ram", &mem_path) < 0)
+            return -1;
+    } else {
         return 0;
-
-    /* There is one special case: if user specified "huge"
-     * pages of regular system pages size.
-     * And there is nothing to do in this case.
-     */
-    if (def->mem.hugepages[0].size == system_page_size)
-        return 0;
-
-    if (qemuGetDomainHupageMemPath(def, cfg, def->mem.hugepages[0].size, &mem_path) < 0)
-        return -1;
+    }
 
     if (def->mem.allocation != VIR_DOMAIN_MEMORY_ALLOCATION_IMMEDIATE)
         virCommandAddArgList(cmd, "-mem-prealloc", NULL);
diff --git a/tests/qemuxml2argvdata/fd-memory-no-numa-topology.args b/tests/qemuxml2argvdata/fd-memory-no-numa-topology.args
index 0e0d0830e8..76c7556468 100644
--- a/tests/qemuxml2argvdata/fd-memory-no-numa-topology.args
+++ b/tests/qemuxml2argvdata/fd-memory-no-numa-topology.args
@@ -10,6 +10,7 @@ QEMU_AUDIO_DRV=none \
 -machine pc-i440fx-wily,accel=kvm,usb=off,dump-guest-core=off \
 -m 14336 \
 -mem-prealloc \
+-mem-path /var/lib/libvirt/qemu/ram/libvirt/qemu/-1-instance-00000092/ram \
 -smp 8,sockets=8,cores=1,threads=1 \
 -uuid 126f2720-6f8e-45ab-a886-ec9277079a67 \
 -display none \
-- 
2.16.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemuBuildMemPathStr: Produce -mem-path more frequently
Posted by John Ferlan 5 years, 7 months ago

On 08/30/2018 08:01 AM, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1622455
> 
> If a domain is configured to use <source type='file'/> under
> <memoryBacking/> we have to honour that setting and produce
> -mem-path on the command line. We are not doing so if domain has
> no guest NUMA nodes nor hugepages.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/qemu/qemu_command.c                            | 29 +++++++++++-----------
>  .../fd-memory-no-numa-topology.args                |  1 +
>  2 files changed, 16 insertions(+), 14 deletions(-)
> 

The code and result look right and a domain can boot like this, so

Reviewed-by: John Ferlan <jferlan@redhat.com>

John

Still, looking at the original commit series it's not exactly "clear"
whether numa mattered or not...

Commit 0857a3bf5 (news.xml) notes "Add support in numa topology...", but
commit bc6d3121a (formatdomain.html.in) doesn't mention numa. It's too
bad the added <source> wasn't a bit more detailed and let's face it the
<allocation> is seriously lacking any substance. I wonder if <source>
should also mention hypervisor specific, yadda, yadda, but more
importantly the qemu memory_backing_dir "link" so that people understand
for qemu where things get created.  I also note that for my 2G guest
there was a *definite* pause when just adding the <memoryBacking>
<source type='file'/> </memoryBacking> when starting up...

In any case, hopefully you can post a followup for formatdomain
'details' - it doesn't have to be part of the bz unless you believe
doing so is "that important" (so to speak).

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list