[libvirt] [PATCH v2 06/14] qemu: Implement @access for <memory/> banks

Michal Privoznik posted 14 patches 8 years, 11 months ago
There is a newer version of this series
[libvirt] [PATCH v2 06/14] qemu: Implement @access for <memory/> banks
Posted by Michal Privoznik 8 years, 11 months ago
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_command.c                            | 11 +++++++--
 src/qemu/qemu_command.h                            |  1 +
 src/qemu/qemu_hotplug.c                            |  2 +-
 .../qemuxml2argv-memory-hotplug-nvdimm-access.args | 26 ++++++++++++++++++++++
 tests/qemuxml2argvtest.c                           |  2 ++
 5 files changed, 39 insertions(+), 3 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-access.args

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 46a6ca9f0..287387055 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3090,6 +3090,7 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
  *               for default
  * @autoNodeset: fallback nodeset in case of automatic NUMA placement
  * @memPathReq: request memory-backend-file with specific mem-path
+ * @memAccessReq: specifically requested memAccess mode
  * @force: forcibly use one of the backends
  *
  * Creates a configuration object that represents memory backend of given guest
@@ -3122,6 +3123,7 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
                           virBitmapPtr userNodeset,
                           virBitmapPtr autoNodeset,
                           const char *memPathReq,
+                          virDomainMemoryAccess memAccessReq,
                           bool force)
 {
     virDomainHugePagePtr master_hugepage = NULL;
@@ -3154,6 +3156,9 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
         memAccess = virDomainNumaGetNodeMemoryAccessMode(def->numa, guestNode);
     }
 
+    if (memAccessReq)
+        memAccess = memAccessReq;
+
     if (virDomainNumatuneGetMode(def->numa, guestNode, &mode) < 0 &&
         virDomainNumatuneGetMode(def->numa, -1, &mode) < 0)
         mode = VIR_DOMAIN_NUMATUNE_MEM_STRICT;
@@ -3352,7 +3357,9 @@ qemuBuildMemoryCellBackendStr(virDomainDefPtr def,
 
     if ((rc = qemuBuildMemoryBackendStr(&props, &backendType, cfg, qemuCaps,
                                         def, cell, memsize, 0, NULL,
-                                        auto_nodeset, NULL, false)) < 0)
+                                        auto_nodeset, NULL,
+                                        VIR_DOMAIN_MEMORY_ACCESS_DEFAULT,
+                                        false)) < 0)
         goto cleanup;
 
     if (!(*backendStr = virQEMUBuildObjectCommandlineFromJSON(backendType,
@@ -3394,7 +3401,7 @@ qemuBuildMemoryDimmBackendStr(virDomainMemoryDefPtr mem,
     if (qemuBuildMemoryBackendStr(&props, &backendType, cfg, qemuCaps, def,
                                   mem->targetNode, mem->size, mem->pagesize,
                                   mem->sourceNodes, auto_nodeset, mem->path,
-                                  true) < 0)
+                                  mem->access, true) < 0)
         goto cleanup;
 
     ret = virQEMUBuildObjectCommandlineFromJSON(backendType, alias, props);
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index e23930255..e32e1718d 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -135,6 +135,7 @@ int qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
                               virBitmapPtr userNodeset,
                               virBitmapPtr autoNodeset,
                               const char *memPath,
+                              virDomainMemoryAccess memAccessReq,
                               bool force);
 
 char *qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 51b87804d..f2299f66e 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2219,7 +2219,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
     if (qemuBuildMemoryBackendStr(&props, &backendType, cfg, priv->qemuCaps,
                                   vm->def, mem->targetNode, mem->size,
                                   mem->pagesize, mem->sourceNodes, NULL,
-                                  mem->path, true) < 0)
+                                  mem->path, mem->access, true) < 0)
         goto cleanup;
 
     if (virDomainMemoryInsert(vm->def, mem) < 0) {
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-access.args b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-access.args
new file mode 100644
index 000000000..d398b2294
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-access.args
@@ -0,0 +1,26 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-name QEMUGuest1 \
+-S \
+-machine pc,accel=tcg,nvdimm=on \
+-m size=219136k,slots=16,maxmem=1099511627776k \
+-smp 2,sockets=2,cores=1,threads=1 \
+-numa node,nodeid=0,cpus=0-1,mem=214 \
+-object memory-backend-file,id=memnvdimm0,prealloc=yes,mem-path=/tmp/nvdimm,\
+share=no,size=536870912 \
+-device nvdimm,node=0,memdev=memnvdimm0,id=nvdimm0,slot=0 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
+-no-acpi \
+-boot c \
+-usb \
+-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
+-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 05a51a389..97a2c55cd 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2335,6 +2335,8 @@ mymain(void)
             QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE);
     DO_TEST("memory-hotplug-nvdimm", QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_DEVICE_NVDIMM,
             QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE);
+    DO_TEST("memory-hotplug-nvdimm-access", QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_DEVICE_NVDIMM,
+            QEMU_CAPS_NUMA, 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.11.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 06/14] qemu: Implement @access for <memory/> banks
Posted by John Ferlan 8 years, 11 months ago

On 02/27/2017 08:19 AM, Michal Privoznik wrote:
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/qemu/qemu_command.c                            | 11 +++++++--
>  src/qemu/qemu_command.h                            |  1 +
>  src/qemu/qemu_hotplug.c                            |  2 +-
>  .../qemuxml2argv-memory-hotplug-nvdimm-access.args | 26 ++++++++++++++++++++++
>  tests/qemuxml2argvtest.c                           |  2 ++
>  5 files changed, 39 insertions(+), 3 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-access.args
> 

Umm - even more reason to pass 'mem' to qemuBuildMemoryBackendStr


> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 46a6ca9f0..287387055 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -3090,6 +3090,7 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
>   *               for default
>   * @autoNodeset: fallback nodeset in case of automatic NUMA placement
>   * @memPathReq: request memory-backend-file with specific mem-path
> + * @memAccessReq: specifically requested memAccess mode
>   * @force: forcibly use one of the backends
>   *
>   * Creates a configuration object that represents memory backend of given guest
> @@ -3122,6 +3123,7 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
>                            virBitmapPtr userNodeset,
>                            virBitmapPtr autoNodeset,
>                            const char *memPathReq,
> +                          virDomainMemoryAccess memAccessReq,
>                            bool force)
>  {
>      virDomainHugePagePtr master_hugepage = NULL;
> @@ -3154,6 +3156,9 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
>          memAccess = virDomainNumaGetNodeMemoryAccessMode(def->numa, guestNode);
>      }
>  
> +    if (memAccessReq)
> +        memAccess = memAccessReq;
> +

Does or could this overwrite what we just got if guestNode >= 0? and
virDomainNumaGetNodeMemoryAccessMode returns something?  Which takes
precedence?  Does the first check even matter if the memAccessReq is set?

Could we be more specific on the condition, too... e.g.

    if (memAccessReq > VIR_DOMAIN_MEMORY_ACCESS_DEFAULT)

John

>      if (virDomainNumatuneGetMode(def->numa, guestNode, &mode) < 0 &&
>          virDomainNumatuneGetMode(def->numa, -1, &mode) < 0)
>          mode = VIR_DOMAIN_NUMATUNE_MEM_STRICT;
> @@ -3352,7 +3357,9 @@ qemuBuildMemoryCellBackendStr(virDomainDefPtr def,
>  
>      if ((rc = qemuBuildMemoryBackendStr(&props, &backendType, cfg, qemuCaps,
>                                          def, cell, memsize, 0, NULL,
> -                                        auto_nodeset, NULL, false)) < 0)
> +                                        auto_nodeset, NULL,
> +                                        VIR_DOMAIN_MEMORY_ACCESS_DEFAULT,
> +                                        false)) < 0)
>          goto cleanup;
>  
>      if (!(*backendStr = virQEMUBuildObjectCommandlineFromJSON(backendType,
> @@ -3394,7 +3401,7 @@ qemuBuildMemoryDimmBackendStr(virDomainMemoryDefPtr mem,
>      if (qemuBuildMemoryBackendStr(&props, &backendType, cfg, qemuCaps, def,
>                                    mem->targetNode, mem->size, mem->pagesize,
>                                    mem->sourceNodes, auto_nodeset, mem->path,
> -                                  true) < 0)
> +                                  mem->access, true) < 0)
>          goto cleanup;
>  
>      ret = virQEMUBuildObjectCommandlineFromJSON(backendType, alias, props);
> diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
> index e23930255..e32e1718d 100644
> --- a/src/qemu/qemu_command.h
> +++ b/src/qemu/qemu_command.h
> @@ -135,6 +135,7 @@ int qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
>                                virBitmapPtr userNodeset,
>                                virBitmapPtr autoNodeset,
>                                const char *memPath,
> +                              virDomainMemoryAccess memAccessReq,
>                                bool force);
>  
>  char *qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem);
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 51b87804d..f2299f66e 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -2219,7 +2219,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
>      if (qemuBuildMemoryBackendStr(&props, &backendType, cfg, priv->qemuCaps,
>                                    vm->def, mem->targetNode, mem->size,
>                                    mem->pagesize, mem->sourceNodes, NULL,
> -                                  mem->path, true) < 0)
> +                                  mem->path, mem->access, true) < 0)
>          goto cleanup;
>  
>      if (virDomainMemoryInsert(vm->def, mem) < 0) {
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-access.args b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-access.args
> new file mode 100644
> index 000000000..d398b2294
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-access.args
> @@ -0,0 +1,26 @@
> +LC_ALL=C \
> +PATH=/bin \
> +HOME=/home/test \
> +USER=test \
> +LOGNAME=test \
> +QEMU_AUDIO_DRV=none \
> +/usr/bin/qemu \
> +-name QEMUGuest1 \
> +-S \
> +-machine pc,accel=tcg,nvdimm=on \
> +-m size=219136k,slots=16,maxmem=1099511627776k \
> +-smp 2,sockets=2,cores=1,threads=1 \
> +-numa node,nodeid=0,cpus=0-1,mem=214 \
> +-object memory-backend-file,id=memnvdimm0,prealloc=yes,mem-path=/tmp/nvdimm,\
> +share=no,size=536870912 \
> +-device nvdimm,node=0,memdev=memnvdimm0,id=nvdimm0,slot=0 \
> +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
> +-nographic \
> +-nodefaults \
> +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
> +-no-acpi \
> +-boot c \
> +-usb \
> +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
> +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
> +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index 05a51a389..97a2c55cd 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -2335,6 +2335,8 @@ mymain(void)
>              QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE);
>      DO_TEST("memory-hotplug-nvdimm", QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_DEVICE_NVDIMM,
>              QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE);
> +    DO_TEST("memory-hotplug-nvdimm-access", QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_DEVICE_NVDIMM,
> +            QEMU_CAPS_NUMA, 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,
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 06/14] qemu: Implement @access for <memory/> banks
Posted by Michal Privoznik 8 years, 11 months ago
On 03/07/2017 05:31 PM, John Ferlan wrote:
> 
> 
> On 02/27/2017 08:19 AM, Michal Privoznik wrote:
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  src/qemu/qemu_command.c                            | 11 +++++++--
>>  src/qemu/qemu_command.h                            |  1 +
>>  src/qemu/qemu_hotplug.c                            |  2 +-
>>  .../qemuxml2argv-memory-hotplug-nvdimm-access.args | 26 ++++++++++++++++++++++
>>  tests/qemuxml2argvtest.c                           |  2 ++
>>  5 files changed, 39 insertions(+), 3 deletions(-)
>>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-nvdimm-access.args
>>
> 
> Umm - even more reason to pass 'mem' to qemuBuildMemoryBackendStr

Yup. It makes patches more clean and readable now that I've switched to
that. Thank you for "forcing" me to overcome my laziness, roll up
sleeves and get the work done.

> 
> 
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index 46a6ca9f0..287387055 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -3090,6 +3090,7 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
>>   *               for default
>>   * @autoNodeset: fallback nodeset in case of automatic NUMA placement
>>   * @memPathReq: request memory-backend-file with specific mem-path
>> + * @memAccessReq: specifically requested memAccess mode
>>   * @force: forcibly use one of the backends
>>   *
>>   * Creates a configuration object that represents memory backend of given guest
>> @@ -3122,6 +3123,7 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
>>                            virBitmapPtr userNodeset,
>>                            virBitmapPtr autoNodeset,
>>                            const char *memPathReq,
>> +                          virDomainMemoryAccess memAccessReq,
>>                            bool force)
>>  {
>>      virDomainHugePagePtr master_hugepage = NULL;
>> @@ -3154,6 +3156,9 @@ qemuBuildMemoryBackendStr(virJSONValuePtr *backendProps,
>>          memAccess = virDomainNumaGetNodeMemoryAccessMode(def->numa, guestNode);
>>      }
>>  
>> +    if (memAccessReq)
>> +        memAccess = memAccessReq;
>> +
> 
> Does or could this overwrite what we just got if guestNode >= 0? and
> virDomainNumaGetNodeMemoryAccessMode returns something?  Which takes
> precedence?  Does the first check even matter if the memAccessReq is set?
> 

I think memAccessReq is the most specific, thus should have the highest
level of importance. IOW, you can have a numa node in say 'private' mode
but one specific NVDIMM module in it in 'shared' mode.

Michal

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