| Message ID | 20211111100351.2153662-2-philmd@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tests/unit: Fix test-smp-parse | expand |
On 11/11/21 11:03 AM, 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(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
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. 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() Reviewed-by: Yanan Wang <wangyanan55@huawei.com> Tested-by: Yanan Wang <wangyanan55@huawei.com> Thanks, Yanan
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 >
On 2021/11/15 18:24, Philippe Mathieu-Daudé wrote: > 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, Yes, we can do that as an optimization for 7.0. I also have noticed those for-7.0 patches submitted. I will have a look. > and I just want to fix the Cirrus-CI > jobs here. And keep the fix for 6.2. >> 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. Thanks! Yanan > >> Reviewed-by: Yanan Wang <wangyanan55@huawei.com> >> Tested-by: Yanan Wang <wangyanan55@huawei.com> >> >> Thanks, >> Yanan >> > .
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); }
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(-)