Re: [PATCH-for-7.0 v4 08/11] tests/unit/test-smp-parse: Add 'smp-without-dies-valid' machine type

[wangyanan (Y)]

Hi Philippe,

On 2021/11/15 22:58, Philippe Mathieu-Daudé wrote:
Keep the common TYPE_MACHINE class initialization in
machine_base_class_init(), make it abstract, and move
the non-common code to a new class: "smp-without-dies-valid".
Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com>
---
  tests/unit/test-smp-parse.c | 19 +++++++++++++++----
  1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index dfe7f1313b0..90a249fe8c8 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -478,13 +478,19 @@ static void machine_base_class_init(ObjectClass *oc, void 
*data)
  {
      MachineClass *mc = MACHINE_CLASS(oc);
  
+    mc->smp_props.prefer_sockets = true;
+
+    mc->name = g_strdup(SMP_MACHINE_NAME);
+}
+
+static void machine_without_dies_valid_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
      mc->min_cpus = MIN_CPUS;
      mc->max_cpus = MAX_CPUS;
  
-    mc->smp_props.prefer_sockets = true;
      mc->smp_props.dies_supported = false;
-
-    mc->name = g_strdup(SMP_MACHINE_NAME);
  }
  
  static void machine_without_dies_invalid_class_init(ObjectClass *oc, void *data)
@@ -606,9 +612,14 @@ static const TypeInfo smp_machine_types[] = {
      {
          .name           = TYPE_MACHINE,
          .parent         = TYPE_OBJECT,
+        .abstract       = true,
          .class_init     = machine_base_class_init,
          .class_size     = sizeof(MachineClass),
          .instance_size  = sizeof(MachineState),
+    }, {
+        .name           = MACHINE_TYPE_NAME("smp-without-dies-valid"),
+        .parent         = TYPE_MACHINE,
+        .class_init     = machine_without_dies_valid_class_init,
      }, {
          .name           = MACHINE_TYPE_NAME("smp-without-dies-invalid"),
          .parent         = TYPE_MACHINE,
@@ -629,7 +640,7 @@ int main(int argc, char *argv[])
      g_test_init(&argc, &argv, NULL);
  
      g_test_add_data_func("/test-smp-parse/generic/valid",
-                         TYPE_MACHINE,
+                         MACHINE_TYPE_NAME("smp-without-dies-valid"),
                           test_generic_valid);
      g_test_add_data_func("/test-smp-parse/generic/invalid",
                           MACHINE_TYPE_NAME("smp-without-dies-invalid"),
After patch #7 and #8, we will have sub-tests as below. It looks nice, 
but it will
probably be better to tweak "smp-without-dies-valid" to "smp-generic-valid",
and "smp-without-dies-invalid" to "smp-generic-invalid", which will be more
consistent with the corresponding sub-test name.

It's Ok now as we only have dies currently besides generic 
sockets/cores/threads,
but "smp-without-dies-xxx" will become a bit confusing when new topology
members are introduced and tested here.

int main(int argc, char *argv[])
{
module_call_init(MODULE_INIT_QOM);

    g_test_init(&argc, &argv, NULL);

g_test_add_data_func("/test-smp-parse/generic/valid",
MACHINE_TYPE_NAME("smp-without-dies-valid"),
test_generic_valid);
g_test_add_data_func("/test-smp-parse/generic/invalid",
MACHINE_TYPE_NAME("smp-without-dies-invalid"),
test_generic_invalid);
g_test_add_data_func("/test-smp-parse/with_dies",
MACHINE_TYPE_NAME("smp-with-dies"),
test_with_dies);

g_test_run();

    return 0;
}

Otherwise, #7 and #8 look nice. Thanks for your work!

Thanks,
Yanan