In some cases management application needs to allocate memory for
qemu upfront and then just let qemu use that. Since we don't want
to expose path for memory-backend-file anywhere in the domain
XML, we can generate predictable paths. In this case:

  $memoryBackingDir/libvirt/qemu/$shortName/$alias

where $shortName is result of virDomainObjGetShortName().

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_command.c                            |  2 +-
 src/qemu/qemu_conf.c                               | 55 +++++++++++++++++-
 src/qemu/qemu_conf.h                               |  9 ++-
 src/qemu/qemu_driver.c                             | 17 ++++++
 src/qemu/qemu_process.c                            | 65 ++++++++++++++++++----
 .../qemuxml2argv-cpu-numa-memshared.args           |  6 +-
 .../qemuxml2argv-fd-memory-numa-topology.args      |  3 +-
 .../qemuxml2argv-fd-memory-numa-topology2.args     |  6 +-
 .../qemuxml2argv-fd-memory-numa-topology3.args     |  9 ++-
 .../qemuxml2argv-hugepages-memaccess2.args         |  9 ++-
 10 files changed, 155 insertions(+), 26 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c949afe90..c12cfec7a 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3419,7 +3419,7 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
         } else {
             /* We can have both pagesize and mem source. If that's the case,
              * prefer hugepages as those are more specific. */
-            if (qemuGetMemoryBackingPath(cfg, &memPath) < 0)
+            if (qemuGetMemoryBackingPath(def, cfg, mem->info.alias, &memPath) < 0)
                 goto cleanup;
         }
 
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index f3cff1503..af503d31c 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1750,9 +1750,41 @@ qemuGetDomainHupageMemPath(const virDomainDef *def,
 }
 
 
+int
+qemuGetMemoryBackingBasePath(virQEMUDriverConfigPtr cfg,
+                             char **path)
+{
+    return virAsprintf(path, "%s/libvirt/qemu", cfg->memoryBackingDir);
+}
+
+
+int
+qemuGetMemoryBackingDomainPath(const virDomainDef *def,
+                               virQEMUDriverConfigPtr cfg,
+                               char **path)
+{
+    char *shortName = NULL;
+    char *base = NULL;
+    int ret = -1;
+
+    if (!(shortName = virDomainDefGetShortName(def)) ||
+        qemuGetMemoryBackingBasePath(cfg, &base) < 0 ||
+        virAsprintf(path, "%s/%s", base, shortName) < 0)
+        goto cleanup;
+
+    ret = 0;
+ cleanup:
+    VIR_FREE(base);
+    VIR_FREE(shortName);
+    return ret;
+}
+
+
 /**
  * qemuGetMemoryBackingPath:
+ * @def: domain definition
  * @cfg: the driver config
+ * @alias: memory object alias
  * @memPath: constructed path
  *
  * Constructs path to memory backing dir and stores it at @memPath.
@@ -1761,8 +1793,27 @@ qemuGetDomainHupageMemPath(const virDomainDef *def,
  *          -1 otherwise (with error reported).
  */
 int
-qemuGetMemoryBackingPath(virQEMUDriverConfigPtr cfg,
+qemuGetMemoryBackingPath(const virDomainDef *def,
+                         virQEMUDriverConfigPtr cfg,
+                         const char *alias,
                          char **memPath)
 {
-    return VIR_STRDUP(*memPath, cfg->memoryBackingDir);
+    char *domainPath = NULL;
+    int ret = -1;
+
+    if (!alias) {
+        /* This should never happen (TM) */
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("memory device alias is not assigned"));
+        goto cleanup;
+    }
+
+    if (qemuGetMemoryBackingDomainPath(def, cfg, &domainPath) < 0 ||
+        virAsprintf(memPath, "%s/%s", domainPath, alias) < 0)
+        goto cleanup;
+
+    ret = 0;
+ cleanup:
+    VIR_FREE(domainPath);
+    return ret;
 }
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 9d6866816..a553e30e2 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -364,6 +364,13 @@ int qemuGetDomainHupageMemPath(const virDomainDef *def,
                                unsigned long long pagesize,
                                char **memPath);
 
-int qemuGetMemoryBackingPath(virQEMUDriverConfigPtr cfg,
+int qemuGetMemoryBackingBasePath(virQEMUDriverConfigPtr cfg,
+                                 char **path);
+int qemuGetMemoryBackingDomainPath(const virDomainDef *def,
+                                   virQEMUDriverConfigPtr cfg,
+                                   char **path);
+int qemuGetMemoryBackingPath(const virDomainDef *def,
+                             virQEMUDriverConfigPtr cfg,
+                             const char *alias,
                              char **memPath);
 #endif /* __QEMUD_CONF_H */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index bb53d78ea..47f85b9bf 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -631,6 +631,7 @@ qemuStateInitialize(bool privileged,
     uid_t run_uid = -1;
     gid_t run_gid = -1;
     char *hugepagePath = NULL;
+    char *memoryBackingPath = NULL;
     size_t i;
 
     if (VIR_ALLOC(qemu_driver) < 0)
@@ -889,6 +890,21 @@ qemuStateInitialize(bool privileged,
         VIR_FREE(hugepagePath);
     }
 
+    if (qemuGetMemoryBackingBasePath(cfg, &memoryBackingPath) < 0)
+        goto error;
+
+    if (virFileMakePath(memoryBackingPath) < 0) {
+        virReportSystemError(errno,
+                             _("unable to create memory backing path %s"),
+                             memoryBackingPath);
+        goto error;
+    }
+
+    if (privileged &&
+        virFileUpdatePerm(memoryBackingPath,
+                          0, S_IXGRP | S_IXOTH) < 0)
+        goto error;
+
     if (!(qemu_driver->closeCallbacks = virCloseCallbacksNew()))
         goto error;
 
@@ -946,6 +962,7 @@ qemuStateInitialize(bool privileged,
     virObjectUnref(conn);
     VIR_FREE(driverConf);
     VIR_FREE(hugepagePath);
+    VIR_FREE(memoryBackingPath);
     qemuStateCleanup();
     return -1;
 }
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 8eef2794e..eca41261a 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3324,6 +3324,36 @@ qemuProcessNeedHugepagesPath(virDomainDefPtr def,
 }
 
 
+static bool
+qemuProcessNeedMemoryBackingPath(virDomainDefPtr def,
+                                 virDomainMemoryDefPtr mem)
+{
+    size_t i;
+    size_t numaNodes;
+
+    if (def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_FILE ||
+        def->mem.access != VIR_DOMAIN_MEMORY_ACCESS_DEFAULT)
+        return true;
+
+    numaNodes = virDomainNumaGetNodeCount(def->numa);
+    for (i = 0; i < numaNodes; i++) {
+        if (virDomainNumaGetNodeMemoryAccessMode(def->numa, i)
+            != VIR_DOMAIN_MEMORY_ACCESS_DEFAULT)
+            return true;
+    }
+
+    if (mem &&
+        mem->model == VIR_DOMAIN_MEMORY_MODEL_DIMM &&
+        (mem->access != VIR_DOMAIN_MEMORY_ACCESS_DEFAULT ||
+         (mem->targetNode >= 0 &&
+          virDomainNumaGetNodeMemoryAccessMode(def->numa, mem->targetNode)
+          != VIR_DOMAIN_MEMORY_ACCESS_DEFAULT)))
+        return true;
+
+    return false;
+}
+
+
 static int
 qemuProcessBuildDestroyMemoryPathsImpl(virQEMUDriverPtr driver,
                                        virDomainDefPtr def,
@@ -3365,33 +3395,46 @@ qemuProcessBuildDestroyMemoryPaths(virQEMUDriverPtr driver,
                                    bool build)
 {
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
-    char *hugepagePath = NULL;
+    char *path = NULL;
     size_t i;
-    bool shouldBuild = false;
+    bool shouldBuildHP = false;
+    bool shouldBuildMB = false;
     int ret = -1;
 
-    if (build)
-        shouldBuild = qemuProcessNeedHugepagesPath(vm->def, mem);
+    if (build) {
+        shouldBuildHP = qemuProcessNeedHugepagesPath(vm->def, mem);
+        shouldBuildMB = qemuProcessNeedMemoryBackingPath(vm->def, mem);
+    }
 
-    if (!build || shouldBuild) {
+    if (!build || shouldBuildHP) {
         for (i = 0; i < cfg->nhugetlbfs; i++) {
-            VIR_FREE(hugepagePath);
-            hugepagePath = qemuGetDomainHugepagePath(vm->def, &cfg->hugetlbfs[i]);
+            path = qemuGetDomainHugepagePath(vm->def, &cfg->hugetlbfs[i]);
 
-            if (!hugepagePath)
+            if (!path)
                 goto cleanup;
 
             if (qemuProcessBuildDestroyMemoryPathsImpl(driver, vm->def,
-                                                       hugepagePath, build) < 0)
+                                                       path, build) < 0)
                 goto cleanup;
 
-            VIR_FREE(hugepagePath);
+            VIR_FREE(path);
         }
     }
 
+    if (!build || shouldBuildMB) {
+        if (qemuGetMemoryBackingDomainPath(vm->def, cfg, &path) < 0)
+            goto cleanup;
+
+        if (qemuProcessBuildDestroyMemoryPathsImpl(driver, vm->def,
+                                                   path, build) < 0)
+            goto cleanup;
+
+        VIR_FREE(path);
+    }
+
     ret = 0;
  cleanup:
-    VIR_FREE(hugepagePath);
+    VIR_FREE(path);
     virObjectUnref(cfg);
     return ret;
 }
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args
index 5700c3413..352819429 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args
@@ -10,10 +10,12 @@ QEMU_AUDIO_DRV=none \
 -M pc \
 -m 214 \
 -smp 16,sockets=2,cores=4,threads=2 \
--object memory-backend-file,id=ram-node0,mem-path=/var/lib/libvirt/qemu/ram,\
+-object memory-backend-file,id=ram-node0,\
+mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-QEMUGuest1/ram-node0,\
 share=yes,size=112197632 \
 -numa node,nodeid=0,cpus=0-7,memdev=ram-node0 \
--object memory-backend-file,id=ram-node1,mem-path=/var/lib/libvirt/qemu/ram,\
+-object memory-backend-file,id=ram-node1,\
+mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-QEMUGuest1/ram-node1,\
 share=no,size=112197632 \
 -numa node,nodeid=1,cpus=8-15,memdev=ram-node1 \
 -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology.args b/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology.args
index 12f3d8ab8..fa1353259 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology.args
@@ -11,7 +11,8 @@ QEMU_AUDIO_DRV=none \
 -m 14336 \
 -mem-prealloc \
 -smp 8,sockets=1,cores=8,threads=1 \
--object memory-backend-file,id=ram-node0,mem-path=/var/lib/libvirt/qemu/ram,\
+-object memory-backend-file,id=ram-node0,\
+mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-instance-00000092/ram-node0,\
 share=yes,size=15032385536 \
 -numa node,nodeid=0,cpus=0-7,memdev=ram-node0 \
 -uuid 126f2720-6f8e-45ab-a886-ec9277079a67 \
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology2.args b/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology2.args
index 585e4d506..6f73a1b99 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology2.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology2.args
@@ -11,10 +11,12 @@ QEMU_AUDIO_DRV=none \
 -m 28672 \
 -mem-prealloc \
 -smp 20,sockets=1,cores=8,threads=1 \
--object memory-backend-file,id=ram-node0,mem-path=/var/lib/libvirt/qemu/ram,\
+-object memory-backend-file,id=ram-node0,\
+mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-instance-00000092/ram-node0,\
 share=no,size=15032385536 \
 -numa node,nodeid=0,cpus=0-7,memdev=ram-node0 \
--object memory-backend-file,id=ram-node1,mem-path=/var/lib/libvirt/qemu/ram,\
+-object memory-backend-file,id=ram-node1,\
+mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-instance-00000092/ram-node1,\
 share=yes,size=15032385536 \
 -numa node,nodeid=1,cpus=8-15,memdev=ram-node1 \
 -uuid 126f2720-6f8e-45ab-a886-ec9277079a67 \
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology3.args b/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology3.args
index e9a57a69e..3c352fe03 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology3.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology3.args
@@ -11,13 +11,16 @@ QEMU_AUDIO_DRV=none \
 -m 43008 \
 -mem-prealloc \
 -smp 32,sockets=1,cores=24,threads=1 \
--object memory-backend-file,id=ram-node0,mem-path=/var/lib/libvirt/qemu/ram,\
+-object memory-backend-file,id=ram-node0,\
+mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-instance-00000092/ram-node0,\
 share=yes,size=15032385536 \
 -numa node,nodeid=0,cpus=0-1,memdev=ram-node0 \
--object memory-backend-file,id=ram-node1,mem-path=/var/lib/libvirt/qemu/ram,\
+-object memory-backend-file,id=ram-node1,\
+mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-instance-00000092/ram-node1,\
 share=yes,size=15032385536 \
 -numa node,nodeid=1,cpus=2-3,memdev=ram-node1 \
--object memory-backend-file,id=ram-node2,mem-path=/var/lib/libvirt/qemu/ram,\
+-object memory-backend-file,id=ram-node2,\
+mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-instance-00000092/ram-node2,\
 share=no,size=15032385536 \
 -numa node,nodeid=2,cpus=4-5,memdev=ram-node2 \
 -uuid 126f2720-6f8e-45ab-a886-ec9277079a67 \
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-memaccess2.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-memaccess2.args
index 55db49171..d8e506c19 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-memaccess2.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-memaccess2.args
@@ -10,17 +10,20 @@ QEMU_AUDIO_DRV=none \
 -M pc \
 -m size=4194304k,slots=16,maxmem=8388608k \
 -smp 4,sockets=4,cores=1,threads=1 \
--object memory-backend-file,id=ram-node0,mem-path=/var/lib/libvirt/qemu/ram,\
+-object memory-backend-file,id=ram-node0,\
+mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-QEMUGuest1/ram-node0,\
 share=no,size=1073741824,host-nodes=0-3,policy=bind \
 -numa node,nodeid=0,cpus=0,memdev=ram-node0 \
 -object memory-backend-file,id=ram-node1,prealloc=yes,\
 mem-path=/dev/hugepages2M/libvirt/qemu/-1-QEMUGuest1,share=yes,size=1073741824,\
 host-nodes=0-3,policy=bind \
 -numa node,nodeid=1,cpus=1,memdev=ram-node1 \
--object memory-backend-file,id=ram-node2,mem-path=/var/lib/libvirt/qemu/ram,\
+-object memory-backend-file,id=ram-node2,\
+mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-QEMUGuest1/ram-node2,\
 share=no,size=1073741824,host-nodes=0-3,policy=bind \
 -numa node,nodeid=2,cpus=2,memdev=ram-node2 \
--object memory-backend-file,id=ram-node3,mem-path=/var/lib/libvirt/qemu/ram,\
+-object memory-backend-file,id=ram-node3,\
+mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-QEMUGuest1/ram-node3,\
 share=no,size=1073741824,host-nodes=3,policy=bind \
 -numa node,nodeid=3,cpus=3,memdev=ram-node3 \
 -object memory-backend-file,id=memdimm0,prealloc=yes,\
-- 
2.13.6

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

On Tue, Nov 07, 2017 at 04:51:03PM +0100, Michal Privoznik wrote:
> In some cases management application needs to allocate memory for
> qemu upfront and then just let qemu use that. Since we don't want
> to expose path for memory-backend-file anywhere in the domain
> XML, we can generate predictable paths. In this case:
> 
>   $memoryBackingDir/libvirt/qemu/$shortName/$alias
> 
> where $shortName is result of virDomainObjGetShortName().
> 

> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args
> index 5700c3413..352819429 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args
> @@ -10,10 +10,12 @@ QEMU_AUDIO_DRV=none \
>  -M pc \
>  -m 214 \
>  -smp 16,sockets=2,cores=4,threads=2 \
> --object memory-backend-file,id=ram-node0,mem-path=/var/lib/libvirt/qemu/ram,\
> +-object memory-backend-file,id=ram-node0,\
> +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-QEMUGuest1/ram-node0,\

Heh, we're getting '-1' in all the paths here. Presumably this is because in
the test suite we've not bothered to set the 'id' field to any value.

I can't help thinking that virDomainObjGetShortName ought to return a fatal
error if 'id' is still -1, as in non-test suite code, this would be indication
of a significant screw up by someone trying to create names before the VM config
is updated to reflect running state.  This would of course mean our tests
should set 'id' to a sensible value too.

No need to fix for this patch though - just a curiosity to look at a later
date


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

On 11/07/2017 04:57 PM, Daniel P. Berrange wrote:
> On Tue, Nov 07, 2017 at 04:51:03PM +0100, Michal Privoznik wrote:
>> In some cases management application needs to allocate memory for
>> qemu upfront and then just let qemu use that. Since we don't want
>> to expose path for memory-backend-file anywhere in the domain
>> XML, we can generate predictable paths. In this case:
>>
>>   $memoryBackingDir/libvirt/qemu/$shortName/$alias
>>
>> where $shortName is result of virDomainObjGetShortName().
>>
> 
>> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args
>> index 5700c3413..352819429 100644
>> --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args
>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args
>> @@ -10,10 +10,12 @@ QEMU_AUDIO_DRV=none \
>>  -M pc \
>>  -m 214 \
>>  -smp 16,sockets=2,cores=4,threads=2 \
>> --object memory-backend-file,id=ram-node0,mem-path=/var/lib/libvirt/qemu/ram,\
>> +-object memory-backend-file,id=ram-node0,\
>> +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-QEMUGuest1/ram-node0,\
> 
> Heh, we're getting '-1' in all the paths here. Presumably this is because in
> the test suite we've not bothered to set the 'id' field to any value.
> 
> I can't help thinking that virDomainObjGetShortName ought to return a fatal
> error if 'id' is still -1, as in non-test suite code, this would be indication
> of a significant screw up by someone trying to create names before the VM config
> is updated to reflect running state.  This would of course mean our tests
> should set 'id' to a sensible value too.
> 
> No need to fix for this patch though - just a curiosity to look at a later
> date

Yeah well,

libvirt.git $ git grep "domain.*-1" tests/ | grep args | wc -l
603

I strongly vote for fixing it up separately then.

Michal

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

On Tue, Nov 07, 2017 at 03:57:45PM +0000, Daniel P. Berrange wrote:
>On Tue, Nov 07, 2017 at 04:51:03PM +0100, Michal Privoznik wrote:
>> In some cases management application needs to allocate memory for
>> qemu upfront and then just let qemu use that. Since we don't want
>> to expose path for memory-backend-file anywhere in the domain
>> XML, we can generate predictable paths. In this case:
>>
>>   $memoryBackingDir/libvirt/qemu/$shortName/$alias
>>
>> where $shortName is result of virDomainObjGetShortName().
>>
>
>> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args
>> index 5700c3413..352819429 100644
>> --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args
>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args
>> @@ -10,10 +10,12 @@ QEMU_AUDIO_DRV=none \
>>  -M pc \
>>  -m 214 \
>>  -smp 16,sockets=2,cores=4,threads=2 \
>> --object memory-backend-file,id=ram-node0,mem-path=/var/lib/libvirt/qemu/ram,\
>> +-object memory-backend-file,id=ram-node0,\
>> +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-QEMUGuest1/ram-node0,\
>
>Heh, we're getting '-1' in all the paths here. Presumably this is because in
>the test suite we've not bothered to set the 'id' field to any value.
>
>I can't help thinking that virDomainObjGetShortName ought to return a fatal
>error if 'id' is still -1, as in non-test suite code, this would be indication
>of a significant screw up by someone trying to create names before the VM config
>is updated to reflect running state.  This would of course mean our tests
>should set 'id' to a sensible value too.
>
>No need to fix for this patch though - just a curiosity to look at a later
>date
>

That's actually on purpose, this should not happen in the code, if it did, then
it would still function normally.  We should start setting id for live XML in
tests, yes.  When trying to change the fact that parsing was done with _INACTIVE
even for live domains, I found out that we are changing parse flags to _INACTIVE
if the id for the domain is not set, but actually the fallout from that change
is kind of bigger than expected and I did not bother fixing it everywhere.
Dunno about Michal, though, maybe he did :)

>
>Regards,
>Daniel
>-- 
>|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
>|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
>|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 11/07/2017 10:51 AM, Michal Privoznik wrote:
> In some cases management application needs to allocate memory for
> qemu upfront and then just let qemu use that. Since we don't want
> to expose path for memory-backend-file anywhere in the domain
> XML, we can generate predictable paths. In this case:
> 
>   $memoryBackingDir/libvirt/qemu/$shortName/$alias
> 
> where $shortName is result of virDomainObjGetShortName().

Just realized the s/Obj/Def/ from my previous review was missed.

John

> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/qemu/qemu_command.c                            |  2 +-
>  src/qemu/qemu_conf.c                               | 55 +++++++++++++++++-
>  src/qemu/qemu_conf.h                               |  9 ++-
>  src/qemu/qemu_driver.c                             | 17 ++++++
>  src/qemu/qemu_process.c                            | 65 ++++++++++++++++++----
>  .../qemuxml2argv-cpu-numa-memshared.args           |  6 +-
>  .../qemuxml2argv-fd-memory-numa-topology.args      |  3 +-
>  .../qemuxml2argv-fd-memory-numa-topology2.args     |  6 +-
>  .../qemuxml2argv-fd-memory-numa-topology3.args     |  9 ++-
>  .../qemuxml2argv-hugepages-memaccess2.args         |  9 ++-
>  10 files changed, 155 insertions(+), 26 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index c949afe90..c12cfec7a 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -3419,7 +3419,7 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
>          } else {
>              /* We can have both pagesize and mem source. If that's the case,
>               * prefer hugepages as those are more specific. */
> -            if (qemuGetMemoryBackingPath(cfg, &memPath) < 0)
> +            if (qemuGetMemoryBackingPath(def, cfg, mem->info.alias, &memPath) < 0)
>                  goto cleanup;
>          }
>  
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index f3cff1503..af503d31c 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -1750,9 +1750,41 @@ qemuGetDomainHupageMemPath(const virDomainDef *def,
>  }
>  
>  
> +int
> +qemuGetMemoryBackingBasePath(virQEMUDriverConfigPtr cfg,
> +                             char **path)
> +{
> +    return virAsprintf(path, "%s/libvirt/qemu", cfg->memoryBackingDir);
> +}
> +
> +
> +int
> +qemuGetMemoryBackingDomainPath(const virDomainDef *def,
> +                               virQEMUDriverConfigPtr cfg,
> +                               char **path)
> +{
> +    char *shortName = NULL;
> +    char *base = NULL;
> +    int ret = -1;
> +
> +    if (!(shortName = virDomainDefGetShortName(def)) ||
> +        qemuGetMemoryBackingBasePath(cfg, &base) < 0 ||
> +        virAsprintf(path, "%s/%s", base, shortName) < 0)
> +        goto cleanup;
> +
> +    ret = 0;
> + cleanup:
> +    VIR_FREE(base);
> +    VIR_FREE(shortName);
> +    return ret;
> +}
> +
> +
>  /**
>   * qemuGetMemoryBackingPath:
> + * @def: domain definition
>   * @cfg: the driver config
> + * @alias: memory object alias
>   * @memPath: constructed path
>   *
>   * Constructs path to memory backing dir and stores it at @memPath.
> @@ -1761,8 +1793,27 @@ qemuGetDomainHupageMemPath(const virDomainDef *def,
>   *          -1 otherwise (with error reported).
>   */
>  int
> -qemuGetMemoryBackingPath(virQEMUDriverConfigPtr cfg,
> +qemuGetMemoryBackingPath(const virDomainDef *def,
> +                         virQEMUDriverConfigPtr cfg,
> +                         const char *alias,
>                           char **memPath)
>  {
> -    return VIR_STRDUP(*memPath, cfg->memoryBackingDir);
> +    char *domainPath = NULL;
> +    int ret = -1;
> +
> +    if (!alias) {
> +        /* This should never happen (TM) */
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("memory device alias is not assigned"));
> +        goto cleanup;
> +    }
> +
> +    if (qemuGetMemoryBackingDomainPath(def, cfg, &domainPath) < 0 ||
> +        virAsprintf(memPath, "%s/%s", domainPath, alias) < 0)
> +        goto cleanup;
> +
> +    ret = 0;
> + cleanup:
> +    VIR_FREE(domainPath);
> +    return ret;
>  }
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index 9d6866816..a553e30e2 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -364,6 +364,13 @@ int qemuGetDomainHupageMemPath(const virDomainDef *def,
>                                 unsigned long long pagesize,
>                                 char **memPath);
>  
> -int qemuGetMemoryBackingPath(virQEMUDriverConfigPtr cfg,
> +int qemuGetMemoryBackingBasePath(virQEMUDriverConfigPtr cfg,
> +                                 char **path);
> +int qemuGetMemoryBackingDomainPath(const virDomainDef *def,
> +                                   virQEMUDriverConfigPtr cfg,
> +                                   char **path);
> +int qemuGetMemoryBackingPath(const virDomainDef *def,
> +                             virQEMUDriverConfigPtr cfg,
> +                             const char *alias,
>                               char **memPath);
>  #endif /* __QEMUD_CONF_H */
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index bb53d78ea..47f85b9bf 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -631,6 +631,7 @@ qemuStateInitialize(bool privileged,
>      uid_t run_uid = -1;
>      gid_t run_gid = -1;
>      char *hugepagePath = NULL;
> +    char *memoryBackingPath = NULL;
>      size_t i;
>  
>      if (VIR_ALLOC(qemu_driver) < 0)
> @@ -889,6 +890,21 @@ qemuStateInitialize(bool privileged,
>          VIR_FREE(hugepagePath);
>      }
>  
> +    if (qemuGetMemoryBackingBasePath(cfg, &memoryBackingPath) < 0)
> +        goto error;
> +
> +    if (virFileMakePath(memoryBackingPath) < 0) {
> +        virReportSystemError(errno,
> +                             _("unable to create memory backing path %s"),
> +                             memoryBackingPath);
> +        goto error;
> +    }
> +
> +    if (privileged &&
> +        virFileUpdatePerm(memoryBackingPath,
> +                          0, S_IXGRP | S_IXOTH) < 0)
> +        goto error;
> +
>      if (!(qemu_driver->closeCallbacks = virCloseCallbacksNew()))
>          goto error;
>  
> @@ -946,6 +962,7 @@ qemuStateInitialize(bool privileged,
>      virObjectUnref(conn);
>      VIR_FREE(driverConf);
>      VIR_FREE(hugepagePath);
> +    VIR_FREE(memoryBackingPath);
>      qemuStateCleanup();
>      return -1;
>  }
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 8eef2794e..eca41261a 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -3324,6 +3324,36 @@ qemuProcessNeedHugepagesPath(virDomainDefPtr def,
>  }
>  
>  
> +static bool
> +qemuProcessNeedMemoryBackingPath(virDomainDefPtr def,
> +                                 virDomainMemoryDefPtr mem)
> +{
> +    size_t i;
> +    size_t numaNodes;
> +
> +    if (def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_FILE ||
> +        def->mem.access != VIR_DOMAIN_MEMORY_ACCESS_DEFAULT)
> +        return true;
> +
> +    numaNodes = virDomainNumaGetNodeCount(def->numa);
> +    for (i = 0; i < numaNodes; i++) {
> +        if (virDomainNumaGetNodeMemoryAccessMode(def->numa, i)
> +            != VIR_DOMAIN_MEMORY_ACCESS_DEFAULT)
> +            return true;
> +    }
> +
> +    if (mem &&
> +        mem->model == VIR_DOMAIN_MEMORY_MODEL_DIMM &&
> +        (mem->access != VIR_DOMAIN_MEMORY_ACCESS_DEFAULT ||
> +         (mem->targetNode >= 0 &&
> +          virDomainNumaGetNodeMemoryAccessMode(def->numa, mem->targetNode)
> +          != VIR_DOMAIN_MEMORY_ACCESS_DEFAULT)))
> +        return true;
> +
> +    return false;
> +}
> +
> +
>  static int
>  qemuProcessBuildDestroyMemoryPathsImpl(virQEMUDriverPtr driver,
>                                         virDomainDefPtr def,
> @@ -3365,33 +3395,46 @@ qemuProcessBuildDestroyMemoryPaths(virQEMUDriverPtr driver,
>                                     bool build)
>  {
>      virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> -    char *hugepagePath = NULL;
> +    char *path = NULL;
>      size_t i;
> -    bool shouldBuild = false;
> +    bool shouldBuildHP = false;
> +    bool shouldBuildMB = false;
>      int ret = -1;
>  
> -    if (build)
> -        shouldBuild = qemuProcessNeedHugepagesPath(vm->def, mem);
> +    if (build) {
> +        shouldBuildHP = qemuProcessNeedHugepagesPath(vm->def, mem);
> +        shouldBuildMB = qemuProcessNeedMemoryBackingPath(vm->def, mem);
> +    }
>  
> -    if (!build || shouldBuild) {
> +    if (!build || shouldBuildHP) {
>          for (i = 0; i < cfg->nhugetlbfs; i++) {
> -            VIR_FREE(hugepagePath);
> -            hugepagePath = qemuGetDomainHugepagePath(vm->def, &cfg->hugetlbfs[i]);
> +            path = qemuGetDomainHugepagePath(vm->def, &cfg->hugetlbfs[i]);
>  
> -            if (!hugepagePath)
> +            if (!path)
>                  goto cleanup;
>  
>              if (qemuProcessBuildDestroyMemoryPathsImpl(driver, vm->def,
> -                                                       hugepagePath, build) < 0)
> +                                                       path, build) < 0)
>                  goto cleanup;
>  
> -            VIR_FREE(hugepagePath);
> +            VIR_FREE(path);
>          }
>      }
>  
> +    if (!build || shouldBuildMB) {
> +        if (qemuGetMemoryBackingDomainPath(vm->def, cfg, &path) < 0)
> +            goto cleanup;
> +
> +        if (qemuProcessBuildDestroyMemoryPathsImpl(driver, vm->def,
> +                                                   path, build) < 0)
> +            goto cleanup;
> +
> +        VIR_FREE(path);
> +    }
> +
>      ret = 0;
>   cleanup:
> -    VIR_FREE(hugepagePath);
> +    VIR_FREE(path);
>      virObjectUnref(cfg);
>      return ret;
>  }
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args
> index 5700c3413..352819429 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-numa-memshared.args
> @@ -10,10 +10,12 @@ QEMU_AUDIO_DRV=none \
>  -M pc \
>  -m 214 \
>  -smp 16,sockets=2,cores=4,threads=2 \
> --object memory-backend-file,id=ram-node0,mem-path=/var/lib/libvirt/qemu/ram,\
> +-object memory-backend-file,id=ram-node0,\
> +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-QEMUGuest1/ram-node0,\
>  share=yes,size=112197632 \
>  -numa node,nodeid=0,cpus=0-7,memdev=ram-node0 \
> --object memory-backend-file,id=ram-node1,mem-path=/var/lib/libvirt/qemu/ram,\
> +-object memory-backend-file,id=ram-node1,\
> +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-QEMUGuest1/ram-node1,\
>  share=no,size=112197632 \
>  -numa node,nodeid=1,cpus=8-15,memdev=ram-node1 \
>  -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology.args b/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology.args
> index 12f3d8ab8..fa1353259 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology.args
> @@ -11,7 +11,8 @@ QEMU_AUDIO_DRV=none \
>  -m 14336 \
>  -mem-prealloc \
>  -smp 8,sockets=1,cores=8,threads=1 \
> --object memory-backend-file,id=ram-node0,mem-path=/var/lib/libvirt/qemu/ram,\
> +-object memory-backend-file,id=ram-node0,\
> +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-instance-00000092/ram-node0,\
>  share=yes,size=15032385536 \
>  -numa node,nodeid=0,cpus=0-7,memdev=ram-node0 \
>  -uuid 126f2720-6f8e-45ab-a886-ec9277079a67 \
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology2.args b/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology2.args
> index 585e4d506..6f73a1b99 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology2.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology2.args
> @@ -11,10 +11,12 @@ QEMU_AUDIO_DRV=none \
>  -m 28672 \
>  -mem-prealloc \
>  -smp 20,sockets=1,cores=8,threads=1 \
> --object memory-backend-file,id=ram-node0,mem-path=/var/lib/libvirt/qemu/ram,\
> +-object memory-backend-file,id=ram-node0,\
> +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-instance-00000092/ram-node0,\
>  share=no,size=15032385536 \
>  -numa node,nodeid=0,cpus=0-7,memdev=ram-node0 \
> --object memory-backend-file,id=ram-node1,mem-path=/var/lib/libvirt/qemu/ram,\
> +-object memory-backend-file,id=ram-node1,\
> +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-instance-00000092/ram-node1,\
>  share=yes,size=15032385536 \
>  -numa node,nodeid=1,cpus=8-15,memdev=ram-node1 \
>  -uuid 126f2720-6f8e-45ab-a886-ec9277079a67 \
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology3.args b/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology3.args
> index e9a57a69e..3c352fe03 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology3.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-fd-memory-numa-topology3.args
> @@ -11,13 +11,16 @@ QEMU_AUDIO_DRV=none \
>  -m 43008 \
>  -mem-prealloc \
>  -smp 32,sockets=1,cores=24,threads=1 \
> --object memory-backend-file,id=ram-node0,mem-path=/var/lib/libvirt/qemu/ram,\
> +-object memory-backend-file,id=ram-node0,\
> +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-instance-00000092/ram-node0,\
>  share=yes,size=15032385536 \
>  -numa node,nodeid=0,cpus=0-1,memdev=ram-node0 \
> --object memory-backend-file,id=ram-node1,mem-path=/var/lib/libvirt/qemu/ram,\
> +-object memory-backend-file,id=ram-node1,\
> +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-instance-00000092/ram-node1,\
>  share=yes,size=15032385536 \
>  -numa node,nodeid=1,cpus=2-3,memdev=ram-node1 \
> --object memory-backend-file,id=ram-node2,mem-path=/var/lib/libvirt/qemu/ram,\
> +-object memory-backend-file,id=ram-node2,\
> +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-instance-00000092/ram-node2,\
>  share=no,size=15032385536 \
>  -numa node,nodeid=2,cpus=4-5,memdev=ram-node2 \
>  -uuid 126f2720-6f8e-45ab-a886-ec9277079a67 \
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-memaccess2.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-memaccess2.args
> index 55db49171..d8e506c19 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-memaccess2.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-memaccess2.args
> @@ -10,17 +10,20 @@ QEMU_AUDIO_DRV=none \
>  -M pc \
>  -m size=4194304k,slots=16,maxmem=8388608k \
>  -smp 4,sockets=4,cores=1,threads=1 \
> --object memory-backend-file,id=ram-node0,mem-path=/var/lib/libvirt/qemu/ram,\
> +-object memory-backend-file,id=ram-node0,\
> +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-QEMUGuest1/ram-node0,\
>  share=no,size=1073741824,host-nodes=0-3,policy=bind \
>  -numa node,nodeid=0,cpus=0,memdev=ram-node0 \
>  -object memory-backend-file,id=ram-node1,prealloc=yes,\
>  mem-path=/dev/hugepages2M/libvirt/qemu/-1-QEMUGuest1,share=yes,size=1073741824,\
>  host-nodes=0-3,policy=bind \
>  -numa node,nodeid=1,cpus=1,memdev=ram-node1 \
> --object memory-backend-file,id=ram-node2,mem-path=/var/lib/libvirt/qemu/ram,\
> +-object memory-backend-file,id=ram-node2,\
> +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-QEMUGuest1/ram-node2,\
>  share=no,size=1073741824,host-nodes=0-3,policy=bind \
>  -numa node,nodeid=2,cpus=2,memdev=ram-node2 \
> --object memory-backend-file,id=ram-node3,mem-path=/var/lib/libvirt/qemu/ram,\
> +-object memory-backend-file,id=ram-node3,\
> +mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/-1-QEMUGuest1/ram-node3,\
>  share=no,size=1073741824,host-nodes=3,policy=bind \
>  -numa node,nodeid=3,cpus=3,memdev=ram-node3 \
>  -object memory-backend-file,id=memdimm0,prealloc=yes,\
> 

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