On 10/28/21 17:09, Philippe Mathieu-Daudé wrote: > From: Yanan Wang <wangyana...@huawei.com> > > Now that we have a generic parser smp_parse(), let's add an unit > test for the code. All possible valid/invalid SMP configurations > that the user can specify are covered. > > Signed-off-by: Yanan Wang <wangyana...@huawei.com> > Reviewed-by: Andrew Jones <drjo...@redhat.com> > Tested-by: Philippe Mathieu-Daudé <phi...@redhat.com> > Message-Id: <20211026034659.22040-3-wangyana...@huawei.com> > Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com> > --- > tests/unit/test-smp-parse.c | 594 ++++++++++++++++++++++++++++++++++++ > MAINTAINERS | 1 + > tests/unit/meson.build | 1 + > 3 files changed, 596 insertions(+) > create mode 100644 tests/unit/test-smp-parse.c
> +static struct SMPTestData data_generic_valid[] = { > + { > + /* config: no configuration provided > + * expect: cpus=1,sockets=1,cores=1,threads=1,maxcpus=1 */ [1] > + .config = SMP_CONFIG_GENERIC(F, 0, F, 0, F, 0, F, 0, F, 0), > + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(1, 1, 1, 1, 1), > + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(1, 1, 1, 1, 1), > + }, { > +static void test_generic(void) > +{ > + Object *obj = object_new(TYPE_MACHINE); > + MachineState *ms = MACHINE(obj); > + MachineClass *mc = MACHINE_GET_CLASS(obj); Watch out, while you create a machine instance in each test, there is only one machine class registered (see type_register_static(&smp_machine_info) below in [2]), ... > + SMPTestData *data = &(SMPTestData){0}; > + int i; > + > + smp_machine_class_init(mc); > + > + for (i = 0; i < ARRAY_SIZE(data_generic_valid); i++) { > + *data = data_generic_valid[i]; > + unsupported_params_init(mc, data); > + > + smp_parse_test(ms, data, true); > + > + /* Unsupported parameters can be provided with their values as 1 */ > + data->config.has_dies = true; > + data->config.dies = 1; > + smp_parse_test(ms, data, true); > + } > + > + /* Reset the supported min CPUs and max CPUs */ > + mc->min_cpus = 2; > + mc->max_cpus = 511; ... and here you are modifying the single machine class state, ... > + > + for (i = 0; i < ARRAY_SIZE(data_generic_invalid); i++) { > + *data = data_generic_invalid[i]; > + unsupported_params_init(mc, data); > + > + smp_parse_test(ms, data, false); > + } > + > + object_unref(obj); > +} > + > +static void test_with_dies(void) > +{ > + Object *obj = object_new(TYPE_MACHINE); > + MachineState *ms = MACHINE(obj); > + MachineClass *mc = MACHINE_GET_CLASS(obj); ... so here the machine class state is inconsistent, ... > + SMPTestData *data = &(SMPTestData){0}; > + unsigned int num_dies = 2; > + int i; > + > + smp_machine_class_init(mc); > + mc->smp_props.dies_supported = true; > + > + for (i = 0; i < ARRAY_SIZE(data_generic_valid); i++) { > + *data = data_generic_valid[i]; > + unsupported_params_init(mc, data); > + > + /* when dies parameter is omitted, it will be set as 1 */ > + data->expect_prefer_sockets.dies = 1; > + data->expect_prefer_cores.dies = 1; > + > + smp_parse_test(ms, data, true); ... in particular the first test [1] is tested with mc->min_cpus = 2. I wonder why you are not getting: Output error report: Invalid SMP CPUs 1. The min CPUs supported by machine '(null)' is 2 for [1]. > + > + /* when dies parameter is specified */ > + data->config.has_dies = true; > + data->config.dies = num_dies; > + if (data->config.has_cpus) { > + data->config.cpus *= num_dies; > + } > + if (data->config.has_maxcpus) { > + data->config.maxcpus *= num_dies; > + } > + > + data->expect_prefer_sockets.dies = num_dies; > + data->expect_prefer_sockets.cpus *= num_dies; > + data->expect_prefer_sockets.max_cpus *= num_dies; > + data->expect_prefer_cores.dies = num_dies; > + data->expect_prefer_cores.cpus *= num_dies; > + data->expect_prefer_cores.max_cpus *= num_dies; > + > + smp_parse_test(ms, data, true); > + } > + > + for (i = 0; i < ARRAY_SIZE(data_with_dies_invalid); i++) { > + *data = data_with_dies_invalid[i]; > + unsupported_params_init(mc, data); > + > + smp_parse_test(ms, data, false); > + } > + > + object_unref(obj); > +} > + > +int main(int argc, char *argv[]) > +{ > + g_test_init(&argc, &argv, NULL); > + > + module_call_init(MODULE_INIT_QOM); > + type_register_static(&smp_machine_info); [2]