Re: [PATCH-for-6.2 v3 1/6] tests/unit/test-smp-parse: Restore MachineClass fields after modifying

[Philippe Mathieu-Daudé]

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é <philmd@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 <wangyanan55@huawei.com>
> Tested-by: Yanan Wang <wangyanan55@huawei.com>
> 
> Thanks,
> Yanan
>