On 2021/11/11 17:37, Philippe Mathieu-Daudé wrote:
On 11/11/21 10:31, wangyanan (Y) wrote:
On 2021/11/11 17:14, Philippe Mathieu-Daudé wrote:
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]),
...
Yes, I noticed this. So on the top of each sub-test function, the
properties
of the single machine class is re-initialized by
smp_machine_class_init(mc).
See [*] below.
+ 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);
And here [*].
+ 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].
So as I have explained above, we won't get an output error report like
this here. :)
I see. IMHO this is bad practice example, so I'll send a cleanup patch.
.
Sure! I'm very happy that we can have a better solution. Thank you for
doing that.
Thanks,
Yanan
