[PATCH] qemuBuildMemoryBackendProps: Use boolean type for 'pmem' property

Peter Krempa posted 1 patch 3 years, 9 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/05b329b0f90da6c5ac0ba15c7a22782bb820be23.1594192755.git.pkrempa@redhat.com
src/qemu/qemu_command.c                                         | 2 +-
.../memory-hotplug-nvdimm-pmem.x86_64-latest.args               | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
[PATCH] qemuBuildMemoryBackendProps: Use boolean type for 'pmem' property
Posted by Peter Krempa 3 years, 9 months ago
Commit 82576d8f35e used a string "on" to enable the 'pmem' property.
This is okay for the command line visitor, but the property is declared
as boolean in qemu and thus it will not work when using QMP.

Modify the type to boolean. This changes the command line, but
fortunately the command line visitor in qemu parses both 'yes' and 'on'
as true for the property.

https://bugzilla.redhat.com/show_bug.cgi?id=1854684

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_command.c                                         | 2 +-
 .../memory-hotplug-nvdimm-pmem.x86_64-latest.args               | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 0c4c77cf8c..c32db06e34 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3187,7 +3187,7 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
                              "with this QEMU binary"));
             return -1;
         }
-        if (virJSONValueObjectAdd(props, "s:pmem", "on", NULL) < 0)
+        if (virJSONValueObjectAdd(props, "b:pmem", true, NULL) < 0)
             return -1;
     }

diff --git a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-pmem.x86_64-latest.args b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-pmem.x86_64-latest.args
index 5dfba9b50a..00a78baa92 100644
--- a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-pmem.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-pmem.x86_64-latest.args
@@ -19,7 +19,7 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
 -smp 2,sockets=2,dies=1,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,pmem=on \
+share=no,size=536870912,pmem=yes \
 -device nvdimm,node=0,memdev=memnvdimm0,id=nvdimm0,slot=0 \
 -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
 -display none \
-- 
2.26.2

Re: [PATCH] qemuBuildMemoryBackendProps: Use boolean type for 'pmem' property
Posted by Ján Tomko 3 years, 9 months ago
On a Wednesday in 2020, Peter Krempa wrote:
>Commit 82576d8f35e used a string "on" to enable the 'pmem' property.
>This is okay for the command line visitor, but the property is declared
>as boolean in qemu and thus it will not work when using QMP.
>
>Modify the type to boolean. This changes the command line, but
>fortunately the command line visitor in qemu parses both 'yes' and 'on'
>as true for the property.
>
>https://bugzilla.redhat.com/show_bug.cgi?id=1854684
>
>Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>---
> src/qemu/qemu_command.c                                         | 2 +-
> .../memory-hotplug-nvdimm-pmem.x86_64-latest.args               | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>index 0c4c77cf8c..c32db06e34 100644
>--- a/src/qemu/qemu_command.c
>+++ b/src/qemu/qemu_command.c
>@@ -3187,7 +3187,7 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
>                              "with this QEMU binary"));
>             return -1;
>         }
>-        if (virJSONValueObjectAdd(props, "s:pmem", "on", NULL) < 0)
>+        if (virJSONValueObjectAdd(props, "b:pmem", true, NULL) < 0)
>             return -1;
>     }
>
>diff --git a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-pmem.x86_64-latest.args b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-pmem.x86_64-latest.args
>index 5dfba9b50a..00a78baa92 100644
>--- a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-pmem.x86_64-latest.args
>+++ b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-pmem.x86_64-latest.args
>@@ -19,7 +19,7 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
> -smp 2,sockets=2,dies=1,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,pmem=on \
>+share=no,size=536870912,pmem=yes \
> -device nvdimm,node=0,memdev=memnvdimm0,id=nvdimm0,slot=0 \
> -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
> -display none \
>-- 
>2.26.2
>
Re: [PATCH] qemuBuildMemoryBackendProps: Use boolean type for 'pmem' property
Posted by Ján Tomko 3 years, 9 months ago
On a Wednesday in 2020, Ján Tomko wrote:
>On a Wednesday in 2020, Peter Krempa wrote:
>>Commit 82576d8f35e used a string "on" to enable the 'pmem' property.
>>This is okay for the command line visitor, but the property is declared
>>as boolean in qemu and thus it will not work when using QMP.
>>
>>Modify the type to boolean. This changes the command line, but
>>fortunately the command line visitor in qemu parses both 'yes' and 'on'
>>as true for the property.
>>
>>https://bugzilla.redhat.com/show_bug.cgi?id=1854684
>>
>>Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>>---
>>src/qemu/qemu_command.c                                         | 2 +-
>>.../memory-hotplug-nvdimm-pmem.x86_64-latest.args               | 2 +-
>>2 files changed, 2 insertions(+), 2 deletions(-)
>>

I meant to paste:

Reviewed-by: Ján Tomko <jtomko@redhat.com>

somewhere in here.

Jano