Commit a22528b9 authored by Markus Armbruster's avatar Markus Armbruster
Browse files

numa: Fix QMP command set-numa-node error handling 



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 64c2a8f6 and 7c88e65d (v2.10.0) added
incorrect error handling right next to correct examples.  Latent bug
until commit f3be6781 (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: f3be6781
Cc: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
Reviewed-by: default avatarIgor Mammedov <imammedo@redhat.com>
Message-Id: <20181017082702.5581-16-armbru@redhat.com>
parent 8c42dbe3

numa.c

+9 −4
Original line number Diff line number Diff line
@@ -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);