[PATCH 0/2] make -M memory-backend and -numa memdev mutually exclusive

[PATCH 0/2] make -M memory-backend and -numa memdev mutually exclusive

[Igor Mammedov]

The options can't be used together (1st provides RAM for non-numa and fake-numa,
while 2nd provides RAM for each numa node).
If used together it might lead to crashes, so add a check to prevent simultaneous
usage.

Igor Mammedov (2):
  vl.c: run preconfig loop before creating default RAM backend
  numa: prevent usage of -M memory-backend and -numa memdev at the same
    time

 hw/core/numa.c | 5 +++++
 softmmu/vl.c   | 5 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)

-- 
2.18.4

[PATCH 1/2] vl.c: run preconfig loop before creating default RAM backend

[Igor Mammedov]

Default RAM backend depends on numa_uses_legacy_mem(), which is
infulenced by -numa options on CLI or set-numa-node QMP command
at preconfig time. If QEMU is started with  '-preconfig'
without -numa, it will lead to creating default RAM backend
even if later set-numa-node is used to assing RAM to NUMA nodes
using 'memdev' NUMA option.
That at best will waste RAM object created by default and with
next patch adding a check to prevent usage of conflicting
 '-M memory-backend' and '-numa memdev'
options, it will make QEMU error out if user tries to configure
NUMA at preconfig time with memdev option, making set-numa-node
unusable.

To fix issue, move preconfig loop before default RAM backend is
created, so that numa_uses_legacy_mem() would take into account
effects of set-numa-node commands executed at preconfig time.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 softmmu/vl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/softmmu/vl.c b/softmmu/vl.c
index afd2615fb3..8e806c9b25 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -4333,12 +4333,13 @@ void qemu_init(int argc, char **argv, char **envp)
 
     parse_numa_opts(current_machine);
 
+    /* do monitor/qmp handling at preconfig state if requested */
+    qemu_main_loop();
+
     if (machine_class->default_ram_id && current_machine->ram_size &&
         numa_uses_legacy_mem() && !current_machine->ram_memdev_id) {
         create_default_memdev(current_machine, mem_path);
     }
-    /* do monitor/qmp handling at preconfig state if requested */
-    qemu_main_loop();
 
     audio_init_audiodevs();
 
-- 
2.18.4

[PATCH 2/2] numa: prevent usage of -M memory-backend and -numa memdev at the same time

[Igor Mammedov]

Options -M memory-backend and -numa memdev are mutually exclusive,
and if used together, it might lead to a crash in the worst case.
For example when the same backend is used with these options together:
  -m 4G \
  -object memory-backend-ram,id=mem0,size=4G \
  -M pc,memory-backend=mem0 \
  -numa node,memdev=mem0
QEMU will abort with:
   exec.c:2006: qemu_ram_set_idstr: Assertion `!new_block->idstr[0]' failed.

and following backtrace:
    abort ()
    qemu_ram_set_idstr ()
    vmstate_register_ram ()
    vmstate_register_ram_global ()
    machine_consume_memdev ()
    numa_init_memdev_container ()
    numa_complete_configuration ()
    machine_run_board_init ()

add a check to error out in case the user tries to use both options at
the same time.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/core/numa.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/core/numa.c b/hw/core/numa.c
index 316bc50d75..5f81900f88 100644
--- a/hw/core/numa.c
+++ b/hw/core/numa.c
@@ -757,6 +757,11 @@ void numa_complete_configuration(MachineState *ms)
         }
 
         if (!numa_uses_legacy_mem() && mc->default_ram_id) {
+            if (ms->ram_memdev_id) {
+                error_report("'-machine memory-backend' and '-numa memdev'"
+                             " properties are mutually exclusive");
+                exit(1);
+            }
             ms->ram = g_new(MemoryRegion, 1);
             memory_region_init(ms->ram, OBJECT(ms), mc->default_ram_id,
                                ram_size);
-- 
2.18.4

Re: [PATCH 0/2] make -M memory-backend and -numa memdev mutually exclusive

[Igor Mammedov]

On Mon, 11 May 2020 10:11:01 -0400
Igor Mammedov <imammedo@redhat.com> wrote:

> The options can't be used together (1st provides RAM for non-numa and fake-numa,
> while 2nd provides RAM for each numa node).
> If used together it might lead to crashes, so add a check to prevent simultaneous
> usage.
> 
> Igor Mammedov (2):
>   vl.c: run preconfig loop before creating default RAM backend
>   numa: prevent usage of -M memory-backend and -numa memdev at the same
>     time
> 
>  hw/core/numa.c | 5 +++++
>  softmmu/vl.c   | 5 +++--
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 

gentle ping

Re: [PATCH 2/2] numa: prevent usage of -M memory-backend and -numa memdev at the same time

[Paolo Bonzini]

On 11/05/20 16:11, Igor Mammedov wrote:
> Options -M memory-backend and -numa memdev are mutually exclusive,
> and if used together, it might lead to a crash in the worst case.
> For example when the same backend is used with these options together:
>   -m 4G \
>   -object memory-backend-ram,id=mem0,size=4G \
>   -M pc,memory-backend=mem0 \
>   -numa node,memdev=mem0
> QEMU will abort with:
>    exec.c:2006: qemu_ram_set_idstr: Assertion `!new_block->idstr[0]' failed.
> 
> and following backtrace:
>     abort ()
>     qemu_ram_set_idstr ()
>     vmstate_register_ram ()
>     vmstate_register_ram_global ()
>     machine_consume_memdev ()
>     numa_init_memdev_container ()
>     numa_complete_configuration ()
>     machine_run_board_init ()
> 
> add a check to error out in case the user tries to use both options at
> the same time.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  hw/core/numa.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/core/numa.c b/hw/core/numa.c
> index 316bc50d75..5f81900f88 100644
> --- a/hw/core/numa.c
> +++ b/hw/core/numa.c
> @@ -757,6 +757,11 @@ void numa_complete_configuration(MachineState *ms)
>          }
>  
>          if (!numa_uses_legacy_mem() && mc->default_ram_id) {
> +            if (ms->ram_memdev_id) {
> +                error_report("'-machine memory-backend' and '-numa memdev'"
> +                             " properties are mutually exclusive");
> +                exit(1);
> +            }
>              ms->ram = g_new(MemoryRegion, 1);
>              memory_region_init(ms->ram, OBJECT(ms), mc->default_ram_id,
>                                 ram_size);
> 

Queued, thanks.

Paolo