Re: [Qemu-devel] [PATCH 14/31] numa: Fix QMP command set-numa-node error handling

[Eduardo Habkost]

On Mon, Oct 08, 2018 at 07:31:08PM +0200, Markus Armbruster wrote:
> Calling error_report() in a function that takes an Error ** argument
> is suspicious.  parse_numa_node() does that, and then exit()s.  It
> also passes &error_fatal to machine_set_cpu_numa_node().  Both wrong.
> Attempting to configure numa when the machine doesn't support it kills
> the VM:
> 
>     $ qemu-system-x86_64 -nodefaults -S -display none -M none -preconfig -qmp 
> stdio
>     {"QMP": {"version": {"qemu": {"micro": 50, "minor": 0, "major": 3}, 
> "package": "v3.0.0-837-gc5e4e49258"}, "capabilities": []}}
>     {"execute": "qmp_capabilities"}
>     {"return": {}}
>     {"execute": "set-numa-node", "arguments": {"type": "node"}}
>     NUMA is not supported by this machine-type
>     $ echo $?
>     1
> 
> Messed up when commit 64c2a8f6d3f and 7c88e65d9e9 (v2.10.0) added
> incorrect error handling right next to correct examples.  Latent bug
> until commit f3be67812c2 (v3.0.0) made it accessible via QMP.  Fairly
> harmless in practice, because it's limited to RUN_STATE_PRECONFIG.
> The fix is obvious: replace error_report(); exit() by error_setg();
> return.
> 
> This affects parse_numa_node()'s other caller
> numa_complete_configuration(): since it ignores errors, the "NUMA is
> not supported by this machine-type" is now ignored, too.  But that
> error is as unexpected there as any other.  Change it to abort on
> error instead.
> 
> Fixes: f3be67812c226162f86ce92634bd913714445420
> Cc: Igor Mammedov <address@hidden>
> Signed-off-by: Markus Armbruster <address@hidden>
> 
> fixup! numa: Fix QMP command set-numa-node error handling

I assume this last line was kept by mistake, and I'm removing it
while committing.

-- 
Eduardo