On Thu, Oct 18, 2018 at 05:03:41PM -0300, Eduardo Habkost wrote:
> From: Markus Armbruster <armbru@xxxxxxxxxx>
>
> 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 <imammedo@xxxxxxxxxx>
> Signed-off-by: Markus Armbruster <armbru@xxxxxxxxxx>
> Message-Id: <20181008173125.19678-15-armbru@xxxxxxxxxx>
> Reviewed-by: Eduardo Habkost <ehabkost@xxxxxxxxxx>
> Reviewed-by: Igor Mammedov <imammedo@xxxxxxxxxx>
> Signed-off-by: Eduardo Habkost <ehabkost@xxxxxxxxxx>
Reviewed-by: David Gibson <david@xxxxxxxxxxxxxxxxxxxxx>
> ---
> numa.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/numa.c b/numa.c
> index 81542d4ebb..1d7c49ad43 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -60,6 +60,7 @@ NodeInfo numa_info[MAX_NODES];
> static void parse_numa_node(MachineState *ms, NumaNodeOptions *node,
> Error **errp)
> {
> + Error *err = NULL;
> uint16_t nodenr;
> uint16List *cpus = NULL;
> MachineClass *mc = MACHINE_GET_CLASS(ms);
> @@ -82,8 +83,8 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions *node,
> }
>
> if (!mc->cpu_index_to_instance_props || !mc->get_default_cpu_node_id) {
> - error_report("NUMA is not supported by this machine-type");
> - exit(1);
> + error_setg(errp, "NUMA is not supported by this machine-type");
> + return;
> }
> for (cpus = node->cpus; cpus; cpus = cpus->next) {
> CpuInstanceProperties props;
> @@ -97,7 +98,11 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions *node,
> props = mc->cpu_index_to_instance_props(ms, cpus->value);
> props.node_id = nodenr;
> props.has_node_id = true;
> - machine_set_cpu_numa_node(ms, &props, &error_fatal);
> + machine_set_cpu_numa_node(ms, &props, &err);
> + if (err) {
> + error_propagate(errp, err);
> + return;
> + }
> }
>
> if (node->has_mem && node->has_memdev) {
> @@ -367,7 +372,7 @@ void numa_complete_configuration(MachineState *ms)
> if (ms->ram_slots > 0 && nb_numa_nodes == 0 &&
> mc->auto_enable_numa_with_memhp) {
> NumaNodeOptions node = { };
> - parse_numa_node(ms, &node, NULL);
> + parse_numa_node(ms, &node, &error_abort);
> }
>
> assert(max_numa_nodeid <= MAX_NODES);
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
