[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 14/31] numa: Fix QMP command set-numa-node error
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 14/31] numa: Fix QMP command set-numa-node error handling |
Date: |
Thu, 11 Oct 2018 19:38:08 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
Eduardo Habkost <address@hidden> writes:
> 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.
Oops.
- [Qemu-devel] [PATCH 29/31] vl: Assert drive_new() does not fail in default_drive(), (continued)
- [Qemu-devel] [PATCH 29/31] vl: Assert drive_new() does not fail in default_drive(), Markus Armbruster, 2018/10/08
- [Qemu-devel] [PATCH 03/31] cpus hw target: Use warn_report() & friends to report warnings, Markus Armbruster, 2018/10/08
- [Qemu-devel] [PATCH 31/31] vl: Simplify call of parse_name(), Markus Armbruster, 2018/10/08
- [Qemu-devel] [PATCH 14/31] numa: Fix QMP command set-numa-node error handling, Markus Armbruster, 2018/10/08
- [Qemu-devel] [PATCH 25/31] numa: Clean up error reporting in parse_numa(), Markus Armbruster, 2018/10/08
- [Qemu-devel] [PATCH 01/31] Use error_fatal to simplify obvious fatal errors (again), Markus Armbruster, 2018/10/08
- [Qemu-devel] [PATCH 05/31] vfio: Clean up error reporting after previous commit, Markus Armbruster, 2018/10/08