On 11/12/21 03:04, wangyanan (Y) wrote: > On 2021/11/11 18:03, Philippe Mathieu-Daudé wrote: >> There is a single MachineClass object, registered with >> type_register_static(&smp_machine_info). Since the same >> object is used multiple times (an MachineState object >> is instantiated in both test_generic and test_with_dies), >> we should restore its internal state after modifying for >> the test purpose. >> >> Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com> >> --- >> tests/unit/test-smp-parse.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c >> index cbe0c990494..bd11fbe91de 100644 >> --- a/tests/unit/test-smp-parse.c >> +++ b/tests/unit/test-smp-parse.c >> @@ -512,7 +512,7 @@ static void test_generic(void) >> smp_parse_test(ms, data, true); >> } >> - /* Reset the supported min CPUs and max CPUs */ >> + /* Force invalid min CPUs and max CPUs */ >> mc->min_cpus = 2; >> mc->max_cpus = 511; >> @@ -523,6 +523,10 @@ static void test_generic(void) >> smp_parse_test(ms, data, false); >> } >> + /* Reset the supported min CPUs and max CPUs */ >> + mc->min_cpus = MIN_CPUS; >> + mc->max_cpus = MAX_CPUS; >> + >> object_unref(obj); >> } >> > Just want to have a note: > Besides the supported min/max CPUs, mc->smp_props is dirtied > too for test purpose in each sub-test function. But for now, it is > not functionally necessary to also restore them at the final of each > sub-test function. We need to do this when new specific parameters > are tested in separate tests.
What we ought do is have an abstract TestMachineClass and have a specific TestCaseMachineClass for each of your test cases. This way we don't need to modify the class internal state at runtime. I chose to not do it now because this is a more invasive change past hard-freeze, and I just want to fix the Cirrus-CI jobs here. > At that time, for example, we will need > to at least add: > > /* Restore the SMP compat properties */ > mc->smp_props.dies_supported = false; > > at the bottom of test_with_dies() OK, I'll add that. > Reviewed-by: Yanan Wang <wangyana...@huawei.com> > Tested-by: Yanan Wang <wangyana...@huawei.com> > > Thanks, > Yanan >
