* [PATCH v4 00/11] tests/unit: Fix test-smp-parse
@ 2021-11-15 14:58 Philippe Mathieu-Daudé
2021-11-15 14:58 ` [PATCH-for-6.2 v4 01/11] tests/unit/test-smp-parse: Restore MachineClass fields after modifying Philippe Mathieu-Daudé
` (11 more replies)
0 siblings, 12 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-11-15 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Andrew Jones, Eduardo Habkost, Markus Armbruster, Yanan Wang,
Thomas Huth, Philippe Mathieu-Daudé
Missing review: patches #4 to #8 (new)
Yet another approach to fix test-smp-parse. v2 from Yanan Wang:
https://lore.kernel.org/qemu-devel/20211111024429.10568-1-wangyanan55@huawei.com/
Here we use the QOM class_init() to avoid having to deal with
object_unref() and deinit().
Since v3:
- Restore 'dies_supported' field in test_with_dies (Yanan)
- Add R-b tags
- QOM-ify the TYPE_MACHINE classes
Philippe Mathieu-Daudé (11):
tests/unit/test-smp-parse: Restore MachineClass fields after modifying
tests/unit/test-smp-parse: QOM'ify smp_machine_class_init()
tests/unit/test-smp-parse: Explicit MachineClass name
tests/unit/test-smp-parse: Pass machine type as argument to tests
tests/unit/test-smp-parse: Split the 'generic' test in valid / invalid
tests/unit/test-smp-parse: Add 'smp-with-dies' machine type
tests/unit/test-smp-parse: Add 'smp-without-dies-invalid' machine type
tests/unit/test-smp-parse: Add 'smp-without-dies-valid' machine type
tests/unit/test-smp-parse: Simplify pointer to compound literal use
tests/unit/test-smp-parse: Constify some pointer/struct
hw/core: Rename smp_parse() -> machine_parse_smp_config()
include/hw/boards.h | 3 +-
hw/core/machine-smp.c | 6 +-
hw/core/machine.c | 2 +-
tests/unit/test-smp-parse.c | 221 +++++++++++++++++++++++-------------
4 files changed, 148 insertions(+), 84 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH-for-6.2 v4 01/11] tests/unit/test-smp-parse: Restore MachineClass fields after modifying
2021-11-15 14:58 [PATCH v4 00/11] tests/unit: Fix test-smp-parse Philippe Mathieu-Daudé
@ 2021-11-15 14:58 ` Philippe Mathieu-Daudé
2021-11-15 14:58 ` [PATCH-for-6.2 v4 02/11] tests/unit/test-smp-parse: QOM'ify smp_machine_class_init() Philippe Mathieu-Daudé
` (10 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-11-15 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Andrew Jones, Eduardo Habkost, Richard Henderson,
Markus Armbruster, Yanan Wang, Thomas Huth,
Philippe Mathieu-Daudé
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.
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Yanan Wang <wangyanan55@huawei.com>
Tested-by: Yanan Wang <wangyanan55@huawei.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
tests/unit/test-smp-parse.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index cbe0c990494..47774c1ee2a 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);
}
@@ -536,6 +540,7 @@ static void test_with_dies(void)
int i;
smp_machine_class_init(mc);
+ /* Force the SMP compat properties */
mc->smp_props.dies_supported = true;
for (i = 0; i < ARRAY_SIZE(data_generic_valid); i++) {
@@ -575,6 +580,9 @@ static void test_with_dies(void)
smp_parse_test(ms, data, false);
}
+ /* Restore the SMP compat properties */
+ mc->smp_props.dies_supported = false;
+
object_unref(obj);
}
--
2.31.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH-for-6.2 v4 02/11] tests/unit/test-smp-parse: QOM'ify smp_machine_class_init()
2021-11-15 14:58 [PATCH v4 00/11] tests/unit: Fix test-smp-parse Philippe Mathieu-Daudé
2021-11-15 14:58 ` [PATCH-for-6.2 v4 01/11] tests/unit/test-smp-parse: Restore MachineClass fields after modifying Philippe Mathieu-Daudé
@ 2021-11-15 14:58 ` Philippe Mathieu-Daudé
2021-11-15 14:58 ` [PATCH-for-6.2 v4 03/11] tests/unit/test-smp-parse: Explicit MachineClass name Philippe Mathieu-Daudé
` (9 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-11-15 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Andrew Jones, Eduardo Habkost, Richard Henderson,
Markus Armbruster, Yanan Wang, Thomas Huth,
Philippe Mathieu-Daudé
smp_machine_class_init() is the actual TypeInfo::class_init().
Declare it as such in smp_machine_info, and avoid to call it
manually in each test. Move smp_machine_info definition just
before we register the type to avoid a forward declaration.
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Yanan Wang <wangyanan55@huawei.com>
Tested-by: Yanan Wang <wangyanan55@huawei.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
tests/unit/test-smp-parse.c | 34 ++++++++++++++++++----------------
1 file changed, 18 insertions(+), 16 deletions(-)
diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index 47774c1ee2a..3fff2af4e27 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -75,14 +75,6 @@ typedef struct SMPTestData {
const char *expect_error;
} SMPTestData;
-/* Type info of the tested machine */
-static const TypeInfo smp_machine_info = {
- .name = TYPE_MACHINE,
- .parent = TYPE_OBJECT,
- .class_size = sizeof(MachineClass),
- .instance_size = sizeof(MachineState),
-};
-
/*
* List all the possible valid sub-collections of the generic 5
* topology parameters (i.e. cpus/maxcpus/sockets/cores/threads),
@@ -480,9 +472,10 @@ static void unsupported_params_init(MachineClass *mc, SMPTestData *data)
}
}
-/* Reset the related machine properties before each sub-test */
-static void smp_machine_class_init(MachineClass *mc)
+static void machine_base_class_init(ObjectClass *oc, void *data)
{
+ MachineClass *mc = MACHINE_CLASS(oc);
+
mc->min_cpus = MIN_CPUS;
mc->max_cpus = MAX_CPUS;
@@ -498,8 +491,6 @@ static void test_generic(void)
SMPTestData *data = &(SMPTestData){{ }};
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);
@@ -539,7 +530,6 @@ static void test_with_dies(void)
unsigned int num_dies = 2;
int i;
- smp_machine_class_init(mc);
/* Force the SMP compat properties */
mc->smp_props.dies_supported = true;
@@ -586,12 +576,24 @@ static void test_with_dies(void)
object_unref(obj);
}
+/* Type info of the tested machine */
+static const TypeInfo smp_machine_types[] = {
+ {
+ .name = TYPE_MACHINE,
+ .parent = TYPE_OBJECT,
+ .class_init = machine_base_class_init,
+ .class_size = sizeof(MachineClass),
+ .instance_size = sizeof(MachineState),
+ }
+};
+
+DEFINE_TYPES(smp_machine_types)
+
int main(int argc, char *argv[])
{
- g_test_init(&argc, &argv, NULL);
-
module_call_init(MODULE_INIT_QOM);
- type_register_static(&smp_machine_info);
+
+ g_test_init(&argc, &argv, NULL);
g_test_add_func("/test-smp-parse/generic", test_generic);
g_test_add_func("/test-smp-parse/with_dies", test_with_dies);
--
2.31.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH-for-6.2 v4 03/11] tests/unit/test-smp-parse: Explicit MachineClass name
2021-11-15 14:58 [PATCH v4 00/11] tests/unit: Fix test-smp-parse Philippe Mathieu-Daudé
2021-11-15 14:58 ` [PATCH-for-6.2 v4 01/11] tests/unit/test-smp-parse: Restore MachineClass fields after modifying Philippe Mathieu-Daudé
2021-11-15 14:58 ` [PATCH-for-6.2 v4 02/11] tests/unit/test-smp-parse: QOM'ify smp_machine_class_init() Philippe Mathieu-Daudé
@ 2021-11-15 14:58 ` Philippe Mathieu-Daudé
2021-11-15 14:58 ` [PATCH-for-7.0 v4 04/11] tests/unit/test-smp-parse: Pass machine type as argument to tests Philippe Mathieu-Daudé
` (8 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-11-15 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Andrew Jones, Eduardo Habkost, Richard Henderson,
Markus Armbruster, Yanan Wang, Thomas Huth,
Philippe Mathieu-Daudé
If the MachineClass::name pointer is not explicitly set, it is NULL.
Per the C standard, passing a NULL pointer to printf "%s" format is
undefined. Some implementations display it as 'NULL', other as 'null'.
Since we are comparing the formatted output, we need a stable value.
The easiest is to explicit a machine name string.
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Yanan Wang <wangyanan55@huawei.com>
Tested-by: Yanan Wang <wangyanan55@huawei.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
tests/unit/test-smp-parse.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index 3fff2af4e27..b02450e25a3 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -23,6 +23,8 @@
#define MIN_CPUS 1 /* set the min CPUs supported by the machine as 1 */
#define MAX_CPUS 512 /* set the max CPUs supported by the machine as 512 */
+#define SMP_MACHINE_NAME "TEST-SMP"
+
/*
* Used to define the generic 3-level CPU topology hierarchy
* -sockets/cores/threads
@@ -307,13 +309,13 @@ static struct SMPTestData data_generic_invalid[] = {
* should tweak the supported min CPUs to 2 for testing */
.config = SMP_CONFIG_GENERIC(T, 1, F, 0, F, 0, F, 0, F, 0),
.expect_error = "Invalid SMP CPUs 1. The min CPUs supported "
- "by machine '(null)' is 2",
+ "by machine '" SMP_MACHINE_NAME "' is 2",
}, {
/* config: -smp 512
* should tweak the supported max CPUs to 511 for testing */
.config = SMP_CONFIG_GENERIC(T, 512, F, 0, F, 0, F, 0, F, 0),
.expect_error = "Invalid SMP CPUs 512. The max CPUs supported "
- "by machine '(null)' is 511",
+ "by machine '" SMP_MACHINE_NAME "' is 511",
},
};
@@ -481,6 +483,8 @@ static void machine_base_class_init(ObjectClass *oc, void *data)
mc->smp_props.prefer_sockets = true;
mc->smp_props.dies_supported = false;
+
+ mc->name = g_strdup(SMP_MACHINE_NAME);
}
static void test_generic(void)
--
2.31.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH-for-7.0 v4 04/11] tests/unit/test-smp-parse: Pass machine type as argument to tests
2021-11-15 14:58 [PATCH v4 00/11] tests/unit: Fix test-smp-parse Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2021-11-15 14:58 ` [PATCH-for-6.2 v4 03/11] tests/unit/test-smp-parse: Explicit MachineClass name Philippe Mathieu-Daudé
@ 2021-11-15 14:58 ` Philippe Mathieu-Daudé
2021-11-16 12:05 ` Richard Henderson
2021-11-16 13:57 ` wangyanan (Y)
2021-11-15 14:58 ` [PATCH-for-7.0 v4 05/11] tests/unit/test-smp-parse: Split the 'generic' test in valid / invalid Philippe Mathieu-Daudé
` (7 subsequent siblings)
11 siblings, 2 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-11-15 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Andrew Jones, Eduardo Habkost, Markus Armbruster, Yanan Wang,
Thomas Huth, Philippe Mathieu-Daudé
Use g_test_add_data_func() instead of g_test_add_func() so we can
pass the machine type to the tests (we will soon have different
machine types).
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
tests/unit/test-smp-parse.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index b02450e25a3..37c6b4981db 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -487,9 +487,10 @@ static void machine_base_class_init(ObjectClass *oc, void *data)
mc->name = g_strdup(SMP_MACHINE_NAME);
}
-static void test_generic(void)
+static void test_generic(const void *opaque)
{
- Object *obj = object_new(TYPE_MACHINE);
+ const char *machine_type = opaque;
+ Object *obj = object_new(machine_type);
MachineState *ms = MACHINE(obj);
MachineClass *mc = MACHINE_GET_CLASS(obj);
SMPTestData *data = &(SMPTestData){{ }};
@@ -525,9 +526,10 @@ static void test_generic(void)
object_unref(obj);
}
-static void test_with_dies(void)
+static void test_with_dies(const void *opaque)
{
- Object *obj = object_new(TYPE_MACHINE);
+ const char *machine_type = opaque;
+ Object *obj = object_new(machine_type);
MachineState *ms = MACHINE(obj);
MachineClass *mc = MACHINE_GET_CLASS(obj);
SMPTestData *data = &(SMPTestData){{ }};
@@ -599,8 +601,12 @@ int main(int argc, char *argv[])
g_test_init(&argc, &argv, NULL);
- g_test_add_func("/test-smp-parse/generic", test_generic);
- g_test_add_func("/test-smp-parse/with_dies", test_with_dies);
+ g_test_add_data_func("/test-smp-parse/generic",
+ TYPE_MACHINE,
+ test_generic);
+ g_test_add_data_func("/test-smp-parse/with_dies",
+ TYPE_MACHINE,
+ test_with_dies);
g_test_run();
--
2.31.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH-for-7.0 v4 05/11] tests/unit/test-smp-parse: Split the 'generic' test in valid / invalid
2021-11-15 14:58 [PATCH v4 00/11] tests/unit: Fix test-smp-parse Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2021-11-15 14:58 ` [PATCH-for-7.0 v4 04/11] tests/unit/test-smp-parse: Pass machine type as argument to tests Philippe Mathieu-Daudé
@ 2021-11-15 14:58 ` Philippe Mathieu-Daudé
2021-11-16 12:06 ` Richard Henderson
` (2 more replies)
2021-11-15 14:58 ` [PATCH-for-7.0 v4 06/11] tests/unit/test-smp-parse: Add 'smp-with-dies' machine type Philippe Mathieu-Daudé
` (6 subsequent siblings)
11 siblings, 3 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-11-15 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Andrew Jones, Eduardo Habkost, Markus Armbruster, Yanan Wang,
Thomas Huth, Philippe Mathieu-Daudé
Split the 'generic' test in two tests: 'valid' and 'invalid'.
This will allow us to remove the hack which modifies the
MachineClass internal state.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
tests/unit/test-smp-parse.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index 37c6b4981db..e27aedad354 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -487,7 +487,7 @@ static void machine_base_class_init(ObjectClass *oc, void *data)
mc->name = g_strdup(SMP_MACHINE_NAME);
}
-static void test_generic(const void *opaque)
+static void test_generic_valid(const void *opaque)
{
const char *machine_type = opaque;
Object *obj = object_new(machine_type);
@@ -508,6 +508,19 @@ static void test_generic(const void *opaque)
smp_parse_test(ms, data, true);
}
+ object_unref(obj);
+}
+
+static void test_generic_invalid(const void *opaque)
+{
+ const char *machine_type = opaque;
+ Object *obj = object_new(machine_type);
+ MachineState *ms = MACHINE(obj);
+ MachineClass *mc = MACHINE_GET_CLASS(obj);
+ SMPTestData *data = &(SMPTestData){};
+ int i;
+
+
/* Force invalid min CPUs and max CPUs */
mc->min_cpus = 2;
mc->max_cpus = 511;
@@ -601,9 +614,12 @@ int main(int argc, char *argv[])
g_test_init(&argc, &argv, NULL);
- g_test_add_data_func("/test-smp-parse/generic",
+ g_test_add_data_func("/test-smp-parse/generic/valid",
TYPE_MACHINE,
- test_generic);
+ test_generic_valid);
+ g_test_add_data_func("/test-smp-parse/generic/invalid",
+ TYPE_MACHINE,
+ test_generic_invalid);
g_test_add_data_func("/test-smp-parse/with_dies",
TYPE_MACHINE,
test_with_dies);
--
2.31.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH-for-7.0 v4 06/11] tests/unit/test-smp-parse: Add 'smp-with-dies' machine type
2021-11-15 14:58 [PATCH v4 00/11] tests/unit: Fix test-smp-parse Philippe Mathieu-Daudé
` (4 preceding siblings ...)
2021-11-15 14:58 ` [PATCH-for-7.0 v4 05/11] tests/unit/test-smp-parse: Split the 'generic' test in valid / invalid Philippe Mathieu-Daudé
@ 2021-11-15 14:58 ` Philippe Mathieu-Daudé
2021-11-16 12:07 ` Richard Henderson
2021-11-16 14:10 ` wangyanan (Y)
2021-11-15 14:58 ` [PATCH-for-7.0 v4 07/11] tests/unit/test-smp-parse: Add 'smp-without-dies-invalid' " Philippe Mathieu-Daudé
` (5 subsequent siblings)
11 siblings, 2 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-11-15 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Andrew Jones, Eduardo Habkost, Markus Armbruster, Yanan Wang,
Thomas Huth, Philippe Mathieu-Daudé
Avoid modifying the MachineClass internals by adding the
'smp-with-dies' machine, which inherits from TYPE_MACHINE.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
tests/unit/test-smp-parse.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index e27aedad354..ff61da06e3d 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -487,6 +487,16 @@ static void machine_base_class_init(ObjectClass *oc, void *data)
mc->name = g_strdup(SMP_MACHINE_NAME);
}
+static void machine_with_dies_class_init(ObjectClass *oc, void *data)
+{
+ MachineClass *mc = MACHINE_CLASS(oc);
+
+ mc->min_cpus = MIN_CPUS;
+ mc->max_cpus = MAX_CPUS;
+
+ mc->smp_props.dies_supported = true;
+}
+
static void test_generic_valid(const void *opaque)
{
const char *machine_type = opaque;
@@ -549,9 +559,6 @@ static void test_with_dies(const void *opaque)
unsigned int num_dies = 2;
int i;
- /* Force the SMP compat properties */
- 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);
@@ -589,9 +596,6 @@ static void test_with_dies(const void *opaque)
smp_parse_test(ms, data, false);
}
- /* Restore the SMP compat properties */
- mc->smp_props.dies_supported = false;
-
object_unref(obj);
}
@@ -603,6 +607,10 @@ static const TypeInfo smp_machine_types[] = {
.class_init = machine_base_class_init,
.class_size = sizeof(MachineClass),
.instance_size = sizeof(MachineState),
+ }, {
+ .name = MACHINE_TYPE_NAME("smp-with-dies"),
+ .parent = TYPE_MACHINE,
+ .class_init = machine_with_dies_class_init,
}
};
@@ -621,7 +629,7 @@ int main(int argc, char *argv[])
TYPE_MACHINE,
test_generic_invalid);
g_test_add_data_func("/test-smp-parse/with_dies",
- TYPE_MACHINE,
+ MACHINE_TYPE_NAME("smp-with-dies"),
test_with_dies);
g_test_run();
--
2.31.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH-for-7.0 v4 07/11] tests/unit/test-smp-parse: Add 'smp-without-dies-invalid' machine type
2021-11-15 14:58 [PATCH v4 00/11] tests/unit: Fix test-smp-parse Philippe Mathieu-Daudé
` (5 preceding siblings ...)
2021-11-15 14:58 ` [PATCH-for-7.0 v4 06/11] tests/unit/test-smp-parse: Add 'smp-with-dies' machine type Philippe Mathieu-Daudé
@ 2021-11-15 14:58 ` Philippe Mathieu-Daudé
2021-11-16 12:07 ` Richard Henderson
2021-11-15 14:58 ` [PATCH-for-7.0 v4 08/11] tests/unit/test-smp-parse: Add 'smp-without-dies-valid' " Philippe Mathieu-Daudé
` (4 subsequent siblings)
11 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-11-15 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Andrew Jones, Eduardo Habkost, Markus Armbruster, Yanan Wang,
Thomas Huth, Philippe Mathieu-Daudé
Avoid modifying the MachineClass internals by adding the
'smp-without-dies-invalid' machine, which inherits from TYPE_MACHINE.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
tests/unit/test-smp-parse.c | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index ff61da06e3d..dfe7f1313b0 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -487,6 +487,17 @@ static void machine_base_class_init(ObjectClass *oc, void *data)
mc->name = g_strdup(SMP_MACHINE_NAME);
}
+static void machine_without_dies_invalid_class_init(ObjectClass *oc, void *data)
+{
+ MachineClass *mc = MACHINE_CLASS(oc);
+
+ /* Force invalid min CPUs and max CPUs */
+ mc->min_cpus = 2;
+ mc->max_cpus = 511;
+
+ mc->smp_props.dies_supported = false;
+}
+
static void machine_with_dies_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
@@ -530,11 +541,6 @@ static void test_generic_invalid(const void *opaque)
SMPTestData *data = &(SMPTestData){};
int i;
-
- /* Force invalid min CPUs and max CPUs */
- mc->min_cpus = 2;
- mc->max_cpus = 511;
-
for (i = 0; i < ARRAY_SIZE(data_generic_invalid); i++) {
*data = data_generic_invalid[i];
unsupported_params_init(mc, data);
@@ -542,10 +548,6 @@ static void test_generic_invalid(const void *opaque)
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);
}
@@ -607,6 +609,10 @@ static const TypeInfo smp_machine_types[] = {
.class_init = machine_base_class_init,
.class_size = sizeof(MachineClass),
.instance_size = sizeof(MachineState),
+ }, {
+ .name = MACHINE_TYPE_NAME("smp-without-dies-invalid"),
+ .parent = TYPE_MACHINE,
+ .class_init = machine_without_dies_invalid_class_init,
}, {
.name = MACHINE_TYPE_NAME("smp-with-dies"),
.parent = TYPE_MACHINE,
@@ -626,7 +632,7 @@ int main(int argc, char *argv[])
TYPE_MACHINE,
test_generic_valid);
g_test_add_data_func("/test-smp-parse/generic/invalid",
- TYPE_MACHINE,
+ 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"),
--
2.31.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH-for-7.0 v4 08/11] tests/unit/test-smp-parse: Add 'smp-without-dies-valid' machine type
2021-11-15 14:58 [PATCH v4 00/11] tests/unit: Fix test-smp-parse Philippe Mathieu-Daudé
` (6 preceding siblings ...)
2021-11-15 14:58 ` [PATCH-for-7.0 v4 07/11] tests/unit/test-smp-parse: Add 'smp-without-dies-invalid' " Philippe Mathieu-Daudé
@ 2021-11-15 14:58 ` Philippe Mathieu-Daudé
2021-11-16 12:08 ` Richard Henderson
2021-11-17 7:37 ` wangyanan (Y)
2021-11-15 14:58 ` [PATCH-for-7.0 v4 09/11] tests/unit/test-smp-parse: Simplify pointer to compound literal use Philippe Mathieu-Daudé
` (3 subsequent siblings)
11 siblings, 2 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-11-15 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Andrew Jones, Eduardo Habkost, Markus Armbruster, Yanan Wang,
Thomas Huth, Philippe Mathieu-Daudé
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é <philmd@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"),
--
2.31.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH-for-7.0 v4 09/11] tests/unit/test-smp-parse: Simplify pointer to compound literal use
2021-11-15 14:58 [PATCH v4 00/11] tests/unit: Fix test-smp-parse Philippe Mathieu-Daudé
` (7 preceding siblings ...)
2021-11-15 14:58 ` [PATCH-for-7.0 v4 08/11] tests/unit/test-smp-parse: Add 'smp-without-dies-valid' " Philippe Mathieu-Daudé
@ 2021-11-15 14:58 ` Philippe Mathieu-Daudé
2021-11-15 14:58 ` [PATCH-for-7.0 v4 10/11] tests/unit/test-smp-parse: Constify some pointer/struct Philippe Mathieu-Daudé
` (2 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-11-15 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Andrew Jones, Eduardo Habkost, Richard Henderson,
Markus Armbruster, Yanan Wang, Thomas Huth,
Philippe Mathieu-Daudé
We can simply use a local variable (and pass its pointer) instead
of a pointer to a compound literal.
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Yanan Wang <wangyanan55@huawei.com>
Tested-by: Yanan Wang <wangyanan55@huawei.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
tests/unit/test-smp-parse.c | 66 ++++++++++++++++++-------------------
1 file changed, 33 insertions(+), 33 deletions(-)
diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index 90a249fe8c8..2f3bcf198a5 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -520,19 +520,19 @@ static void test_generic_valid(const void *opaque)
Object *obj = object_new(machine_type);
MachineState *ms = MACHINE(obj);
MachineClass *mc = MACHINE_GET_CLASS(obj);
- SMPTestData *data = &(SMPTestData){{ }};
+ SMPTestData data = {};
int i;
for (i = 0; i < ARRAY_SIZE(data_generic_valid); i++) {
- *data = data_generic_valid[i];
- unsupported_params_init(mc, data);
+ data = data_generic_valid[i];
+ unsupported_params_init(mc, &data);
- smp_parse_test(ms, data, true);
+ 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);
+ data.config.has_dies = true;
+ data.config.dies = 1;
+ smp_parse_test(ms, &data, true);
}
object_unref(obj);
@@ -544,14 +544,14 @@ static void test_generic_invalid(const void *opaque)
Object *obj = object_new(machine_type);
MachineState *ms = MACHINE(obj);
MachineClass *mc = MACHINE_GET_CLASS(obj);
- SMPTestData *data = &(SMPTestData){};
+ SMPTestData data = {};
int i;
for (i = 0; i < ARRAY_SIZE(data_generic_invalid); i++) {
- *data = data_generic_invalid[i];
- unsupported_params_init(mc, data);
+ data = data_generic_invalid[i];
+ unsupported_params_init(mc, &data);
- smp_parse_test(ms, data, false);
+ smp_parse_test(ms, &data, false);
}
object_unref(obj);
@@ -563,45 +563,45 @@ static void test_with_dies(const void *opaque)
Object *obj = object_new(machine_type);
MachineState *ms = MACHINE(obj);
MachineClass *mc = MACHINE_GET_CLASS(obj);
- SMPTestData *data = &(SMPTestData){{ }};
+ SMPTestData data = {};
unsigned int num_dies = 2;
int i;
for (i = 0; i < ARRAY_SIZE(data_generic_valid); i++) {
- *data = data_generic_valid[i];
- unsupported_params_init(mc, data);
+ 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;
+ data.expect_prefer_sockets.dies = 1;
+ data.expect_prefer_cores.dies = 1;
- smp_parse_test(ms, data, true);
+ smp_parse_test(ms, &data, true);
/* when dies parameter is specified */
- data->config.has_dies = true;
- data->config.dies = num_dies;
- if (data->config.has_cpus) {
- data->config.cpus *= num_dies;
+ data.config.has_dies = true;
+ data.config.dies = num_dies;
+ if (data.config.has_cpus) {
+ data.config.cpus *= num_dies;
}
- if (data->config.has_maxcpus) {
- data->config.maxcpus *= num_dies;
+ if (data.config.has_maxcpus) {
+ data.config.maxcpus *= num_dies;
}
- data->expect_prefer_sockets.dies = num_dies;
- data->expect_prefer_sockets.cpus *= num_dies;
- data->expect_prefer_sockets.max_cpus *= num_dies;
- data->expect_prefer_cores.dies = num_dies;
- data->expect_prefer_cores.cpus *= num_dies;
- data->expect_prefer_cores.max_cpus *= num_dies;
+ data.expect_prefer_sockets.dies = num_dies;
+ data.expect_prefer_sockets.cpus *= num_dies;
+ data.expect_prefer_sockets.max_cpus *= num_dies;
+ data.expect_prefer_cores.dies = num_dies;
+ data.expect_prefer_cores.cpus *= num_dies;
+ data.expect_prefer_cores.max_cpus *= num_dies;
- smp_parse_test(ms, data, true);
+ smp_parse_test(ms, &data, true);
}
for (i = 0; i < ARRAY_SIZE(data_with_dies_invalid); i++) {
- *data = data_with_dies_invalid[i];
- unsupported_params_init(mc, data);
+ data = data_with_dies_invalid[i];
+ unsupported_params_init(mc, &data);
- smp_parse_test(ms, data, false);
+ smp_parse_test(ms, &data, false);
}
object_unref(obj);
--
2.31.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH-for-7.0 v4 10/11] tests/unit/test-smp-parse: Constify some pointer/struct
2021-11-15 14:58 [PATCH v4 00/11] tests/unit: Fix test-smp-parse Philippe Mathieu-Daudé
` (8 preceding siblings ...)
2021-11-15 14:58 ` [PATCH-for-7.0 v4 09/11] tests/unit/test-smp-parse: Simplify pointer to compound literal use Philippe Mathieu-Daudé
@ 2021-11-15 14:58 ` Philippe Mathieu-Daudé
2021-11-15 14:59 ` [PATCH-for-7.0 v4 11/11] hw/core: Rename smp_parse() -> machine_parse_smp_config() Philippe Mathieu-Daudé
2021-11-15 22:49 ` [PATCH v4 00/11] tests/unit: Fix test-smp-parse Philippe Mathieu-Daudé
11 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-11-15 14:58 UTC (permalink / raw)
To: qemu-devel
Cc: Andrew Jones, Eduardo Habkost, Richard Henderson,
Markus Armbruster, Yanan Wang, Thomas Huth,
Philippe Mathieu-Daudé
Declare structures const when we don't need to modify
them at runtime.
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Yanan Wang <wangyanan55@huawei.com>
Tested-by: Yanan Wang <wangyanan55@huawei.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
tests/unit/test-smp-parse.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index 2f3bcf198a5..8f47a2e65f6 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -83,7 +83,7 @@ typedef struct SMPTestData {
* then test the automatic calculation algorithm of the missing
* values in the parser.
*/
-static struct SMPTestData data_generic_valid[] = {
+static const struct SMPTestData data_generic_valid[] = {
{
/* config: no configuration provided
* expect: cpus=1,sockets=1,cores=1,threads=1,maxcpus=1 */
@@ -285,7 +285,7 @@ static struct SMPTestData data_generic_valid[] = {
},
};
-static struct SMPTestData data_generic_invalid[] = {
+static const struct SMPTestData data_generic_invalid[] = {
{
/* config: -smp 2,dies=2 */
.config = SMP_CONFIG_WITH_DIES(T, 2, F, 0, T, 2, F, 0, F, 0, F, 0),
@@ -319,7 +319,7 @@ static struct SMPTestData data_generic_invalid[] = {
},
};
-static struct SMPTestData data_with_dies_invalid[] = {
+static const struct SMPTestData data_with_dies_invalid[] = {
{
/* config: -smp 16,sockets=2,dies=2,cores=4,threads=2,maxcpus=16 */
.config = SMP_CONFIG_WITH_DIES(T, 16, T, 2, T, 2, T, 4, T, 2, T, 16),
@@ -356,7 +356,7 @@ static char *smp_config_to_string(SMPConfiguration *config)
config->has_maxcpus ? "true" : "false", config->maxcpus);
}
-static char *cpu_topology_to_string(CpuTopology *topo)
+static char *cpu_topology_to_string(const CpuTopology *topo)
{
return g_strdup_printf(
"(CpuTopology) {\n"
@@ -372,7 +372,7 @@ static char *cpu_topology_to_string(CpuTopology *topo)
}
static void check_parse(MachineState *ms, SMPConfiguration *config,
- CpuTopology *expect_topo, const char *expect_err,
+ const CpuTopology *expect_topo, const char *expect_err,
bool is_valid)
{
g_autofree char *config_str = smp_config_to_string(config);
@@ -466,7 +466,7 @@ static void smp_parse_test(MachineState *ms, SMPTestData *data, bool is_valid)
}
/* The parsed results of the unsupported parameters should be 1 */
-static void unsupported_params_init(MachineClass *mc, SMPTestData *data)
+static void unsupported_params_init(const MachineClass *mc, SMPTestData *data)
{
if (!mc->smp_props.dies_supported) {
data->expect_prefer_sockets.dies = 1;
--
2.31.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH-for-7.0 v4 11/11] hw/core: Rename smp_parse() -> machine_parse_smp_config()
2021-11-15 14:58 [PATCH v4 00/11] tests/unit: Fix test-smp-parse Philippe Mathieu-Daudé
` (9 preceding siblings ...)
2021-11-15 14:58 ` [PATCH-for-7.0 v4 10/11] tests/unit/test-smp-parse: Constify some pointer/struct Philippe Mathieu-Daudé
@ 2021-11-15 14:59 ` Philippe Mathieu-Daudé
2021-11-15 22:49 ` [PATCH v4 00/11] tests/unit: Fix test-smp-parse Philippe Mathieu-Daudé
11 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-11-15 14:59 UTC (permalink / raw)
To: qemu-devel
Cc: Andrew Jones, Eduardo Habkost, Richard Henderson,
Markus Armbruster, Yanan Wang, Thomas Huth,
Philippe Mathieu-Daudé
All methods related to MachineState are prefixed with "machine_".
smp_parse() does not need to be an exception. Rename it and
const'ify the SMPConfiguration argument, since it doesn't need
to be modified.
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Yanan Wang <wangyanan55@huawei.com>
Tested-by: Yanan Wang <wangyanan55@huawei.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
include/hw/boards.h | 3 ++-
hw/core/machine-smp.c | 6 ++++--
hw/core/machine.c | 2 +-
tests/unit/test-smp-parse.c | 8 ++++----
4 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 9c1c1901046..7597cec4400 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -34,7 +34,8 @@ HotpluggableCPUList *machine_query_hotpluggable_cpus(MachineState *machine);
void machine_set_cpu_numa_node(MachineState *machine,
const CpuInstanceProperties *props,
Error **errp);
-void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp);
+void machine_parse_smp_config(MachineState *ms,
+ const SMPConfiguration *config, Error **errp);
/**
* machine_class_allow_dynamic_sysbus_dev: Add type to list of valid devices
diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c
index 116a0cbbfab..2cbfd574293 100644
--- a/hw/core/machine-smp.c
+++ b/hw/core/machine-smp.c
@@ -44,7 +44,8 @@ static char *cpu_hierarchy_to_string(MachineState *ms)
}
/*
- * smp_parse - Generic function used to parse the given SMP configuration
+ * machine_parse_smp_config: Generic function used to parse the given
+ * SMP configuration
*
* Any missing parameter in "cpus/maxcpus/sockets/cores/threads" will be
* automatically computed based on the provided ones.
@@ -63,7 +64,8 @@ static char *cpu_hierarchy_to_string(MachineState *ms)
* introduced topology members which are likely to be target specific should
* be directly set as 1 if they are omitted (e.g. dies for PC since 4.1).
*/
-void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
+void machine_parse_smp_config(MachineState *ms,
+ const SMPConfiguration *config, Error **errp)
{
MachineClass *mc = MACHINE_GET_CLASS(ms);
unsigned cpus = config->has_cpus ? config->cpus : 0;
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 26ec54e7261..a2d3c9969d9 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -760,7 +760,7 @@ static void machine_set_smp(Object *obj, Visitor *v, const char *name,
return;
}
- smp_parse(ms, config, errp);
+ machine_parse_smp_config(ms, config, errp);
}
static void machine_class_init(ObjectClass *oc, void *data)
diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index 8f47a2e65f6..8e488e95145 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -337,7 +337,7 @@ static const struct SMPTestData data_with_dies_invalid[] = {
},
};
-static char *smp_config_to_string(SMPConfiguration *config)
+static char *smp_config_to_string(const SMPConfiguration *config)
{
return g_strdup_printf(
"(SMPConfiguration) {\n"
@@ -371,7 +371,7 @@ static char *cpu_topology_to_string(const CpuTopology *topo)
topo->cores, topo->threads, topo->max_cpus);
}
-static void check_parse(MachineState *ms, SMPConfiguration *config,
+static void check_parse(MachineState *ms, const SMPConfiguration *config,
const CpuTopology *expect_topo, const char *expect_err,
bool is_valid)
{
@@ -380,8 +380,8 @@ static void check_parse(MachineState *ms, SMPConfiguration *config,
g_autofree char *output_topo_str = NULL;
Error *err = NULL;
- /* call the generic parser smp_parse() */
- smp_parse(ms, config, &err);
+ /* call the generic parser */
+ machine_parse_smp_config(ms, config, &err);
output_topo_str = cpu_topology_to_string(&ms->smp);
--
2.31.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v4 00/11] tests/unit: Fix test-smp-parse
2021-11-15 14:58 [PATCH v4 00/11] tests/unit: Fix test-smp-parse Philippe Mathieu-Daudé
` (10 preceding siblings ...)
2021-11-15 14:59 ` [PATCH-for-7.0 v4 11/11] hw/core: Rename smp_parse() -> machine_parse_smp_config() Philippe Mathieu-Daudé
@ 2021-11-15 22:49 ` Philippe Mathieu-Daudé
11 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-11-15 22:49 UTC (permalink / raw)
To: qemu-devel
Cc: Andrew Jones, Eduardo Habkost, Markus Armbruster, Yanan Wang,
Thomas Huth
On 11/15/21 15:58, Philippe Mathieu-Daudé wrote:
> Missing review: patches #4 to #8 (new)
>
> Yet another approach to fix test-smp-parse. v2 from Yanan Wang:
> https://lore.kernel.org/qemu-devel/20211111024429.10568-1-wangyanan55@huawei.com/
>
> Here we use the QOM class_init() to avoid having to deal with
> object_unref() and deinit().
>
> Since v3:
> - Restore 'dies_supported' field in test_with_dies (Yanan)
> - Add R-b tags
> - QOM-ify the TYPE_MACHINE classes
>
> Philippe Mathieu-Daudé (11):
> tests/unit/test-smp-parse: Restore MachineClass fields after modifying
> tests/unit/test-smp-parse: QOM'ify smp_machine_class_init()
> tests/unit/test-smp-parse: Explicit MachineClass name
Patches 1-3 queued.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH-for-7.0 v4 04/11] tests/unit/test-smp-parse: Pass machine type as argument to tests
2021-11-15 14:58 ` [PATCH-for-7.0 v4 04/11] tests/unit/test-smp-parse: Pass machine type as argument to tests Philippe Mathieu-Daudé
@ 2021-11-16 12:05 ` Richard Henderson
2021-11-16 13:57 ` wangyanan (Y)
1 sibling, 0 replies; 27+ messages in thread
From: Richard Henderson @ 2021-11-16 12:05 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Yanan Wang, Andrew Jones, Eduardo Habkost, Thomas Huth,
Markus Armbruster
On 11/15/21 3:58 PM, Philippe Mathieu-Daudé wrote:
> Use g_test_add_data_func() instead of g_test_add_func() so we can
> pass the machine type to the tests (we will soon have different
> machine types).
>
> Signed-off-by: Philippe Mathieu-Daudé<philmd@redhat.com>
> ---
> tests/unit/test-smp-parse.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH-for-7.0 v4 05/11] tests/unit/test-smp-parse: Split the 'generic' test in valid / invalid
2021-11-15 14:58 ` [PATCH-for-7.0 v4 05/11] tests/unit/test-smp-parse: Split the 'generic' test in valid / invalid Philippe Mathieu-Daudé
@ 2021-11-16 12:06 ` Richard Henderson
2021-11-16 13:58 ` wangyanan (Y)
2021-11-16 14:07 ` wangyanan (Y)
2 siblings, 0 replies; 27+ messages in thread
From: Richard Henderson @ 2021-11-16 12:06 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Yanan Wang, Andrew Jones, Eduardo Habkost, Thomas Huth,
Markus Armbruster
On 11/15/21 3:58 PM, Philippe Mathieu-Daudé wrote:
> Split the 'generic' test in two tests: 'valid' and 'invalid'.
> This will allow us to remove the hack which modifies the
> MachineClass internal state.
>
> Signed-off-by: Philippe Mathieu-Daudé<philmd@redhat.com>
> ---
> tests/unit/test-smp-parse.c | 22 +++++++++++++++++++---
> 1 file changed, 19 insertions(+), 3 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH-for-7.0 v4 06/11] tests/unit/test-smp-parse: Add 'smp-with-dies' machine type
2021-11-15 14:58 ` [PATCH-for-7.0 v4 06/11] tests/unit/test-smp-parse: Add 'smp-with-dies' machine type Philippe Mathieu-Daudé
@ 2021-11-16 12:07 ` Richard Henderson
2021-11-16 14:10 ` wangyanan (Y)
1 sibling, 0 replies; 27+ messages in thread
From: Richard Henderson @ 2021-11-16 12:07 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Yanan Wang, Andrew Jones, Eduardo Habkost, Thomas Huth,
Markus Armbruster
On 11/15/21 3:58 PM, Philippe Mathieu-Daudé wrote:
> Avoid modifying the MachineClass internals by adding the
> 'smp-with-dies' machine, which inherits from TYPE_MACHINE.
>
> Signed-off-by: Philippe Mathieu-Daudé<philmd@redhat.com>
> ---
> tests/unit/test-smp-parse.c | 22 +++++++++++++++-------
> 1 file changed, 15 insertions(+), 7 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH-for-7.0 v4 07/11] tests/unit/test-smp-parse: Add 'smp-without-dies-invalid' machine type
2021-11-15 14:58 ` [PATCH-for-7.0 v4 07/11] tests/unit/test-smp-parse: Add 'smp-without-dies-invalid' " Philippe Mathieu-Daudé
@ 2021-11-16 12:07 ` Richard Henderson
0 siblings, 0 replies; 27+ messages in thread
From: Richard Henderson @ 2021-11-16 12:07 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Yanan Wang, Andrew Jones, Eduardo Habkost, Thomas Huth,
Markus Armbruster
On 11/15/21 3:58 PM, Philippe Mathieu-Daudé wrote:
> Avoid modifying the MachineClass internals by adding the
> 'smp-without-dies-invalid' machine, which inherits from TYPE_MACHINE.
>
> Signed-off-by: Philippe Mathieu-Daudé<philmd@redhat.com>
> ---
> tests/unit/test-smp-parse.c | 26 ++++++++++++++++----------
> 1 file changed, 16 insertions(+), 10 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH-for-7.0 v4 08/11] tests/unit/test-smp-parse: Add 'smp-without-dies-valid' machine type
2021-11-15 14:58 ` [PATCH-for-7.0 v4 08/11] tests/unit/test-smp-parse: Add 'smp-without-dies-valid' " Philippe Mathieu-Daudé
@ 2021-11-16 12:08 ` Richard Henderson
2021-11-17 7:37 ` wangyanan (Y)
1 sibling, 0 replies; 27+ messages in thread
From: Richard Henderson @ 2021-11-16 12:08 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Yanan Wang, Andrew Jones, Eduardo Habkost, Thomas Huth,
Markus Armbruster
On 11/15/21 3:58 PM, 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é<philmd@redhat.com>
> ---
> tests/unit/test-smp-parse.c | 19 +++++++++++++++----
> 1 file changed, 15 insertions(+), 4 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH-for-7.0 v4 04/11] tests/unit/test-smp-parse: Pass machine type as argument to tests
2021-11-15 14:58 ` [PATCH-for-7.0 v4 04/11] tests/unit/test-smp-parse: Pass machine type as argument to tests Philippe Mathieu-Daudé
2021-11-16 12:05 ` Richard Henderson
@ 2021-11-16 13:57 ` wangyanan (Y)
1 sibling, 0 replies; 27+ messages in thread
From: wangyanan (Y) @ 2021-11-16 13:57 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Thomas Huth, Andrew Jones, Markus Armbruster, Eduardo Habkost
On 2021/11/15 22:58, Philippe Mathieu-Daudé wrote:
> Use g_test_add_data_func() instead of g_test_add_func() so we can
> pass the machine type to the tests (we will soon have different
> machine types).
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> tests/unit/test-smp-parse.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
Reviewed-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 b02450e25a3..37c6b4981db 100644
> --- a/tests/unit/test-smp-parse.c
> +++ b/tests/unit/test-smp-parse.c
> @@ -487,9 +487,10 @@ static void machine_base_class_init(ObjectClass *oc, void *data)
> mc->name = g_strdup(SMP_MACHINE_NAME);
> }
>
> -static void test_generic(void)
> +static void test_generic(const void *opaque)
> {
> - Object *obj = object_new(TYPE_MACHINE);
> + const char *machine_type = opaque;
> + Object *obj = object_new(machine_type);
> MachineState *ms = MACHINE(obj);
> MachineClass *mc = MACHINE_GET_CLASS(obj);
> SMPTestData *data = &(SMPTestData){{ }};
> @@ -525,9 +526,10 @@ static void test_generic(void)
> object_unref(obj);
> }
>
> -static void test_with_dies(void)
> +static void test_with_dies(const void *opaque)
> {
> - Object *obj = object_new(TYPE_MACHINE);
> + const char *machine_type = opaque;
> + Object *obj = object_new(machine_type);
> MachineState *ms = MACHINE(obj);
> MachineClass *mc = MACHINE_GET_CLASS(obj);
> SMPTestData *data = &(SMPTestData){{ }};
> @@ -599,8 +601,12 @@ int main(int argc, char *argv[])
>
> g_test_init(&argc, &argv, NULL);
>
> - g_test_add_func("/test-smp-parse/generic", test_generic);
> - g_test_add_func("/test-smp-parse/with_dies", test_with_dies);
> + g_test_add_data_func("/test-smp-parse/generic",
> + TYPE_MACHINE,
> + test_generic);
> + g_test_add_data_func("/test-smp-parse/with_dies",
> + TYPE_MACHINE,
> + test_with_dies);
>
> g_test_run();
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH-for-7.0 v4 05/11] tests/unit/test-smp-parse: Split the 'generic' test in valid / invalid
2021-11-15 14:58 ` [PATCH-for-7.0 v4 05/11] tests/unit/test-smp-parse: Split the 'generic' test in valid / invalid Philippe Mathieu-Daudé
2021-11-16 12:06 ` Richard Henderson
@ 2021-11-16 13:58 ` wangyanan (Y)
2021-11-16 14:07 ` wangyanan (Y)
2 siblings, 0 replies; 27+ messages in thread
From: wangyanan (Y) @ 2021-11-16 13:58 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Thomas Huth, Andrew Jones, Markus Armbruster, Eduardo Habkost
On 2021/11/15 22:58, Philippe Mathieu-Daudé wrote:
> Split the 'generic' test in two tests: 'valid' and 'invalid'.
> This will allow us to remove the hack which modifies the
> MachineClass internal state.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> tests/unit/test-smp-parse.c | 22 +++++++++++++++++++---
> 1 file changed, 19 insertions(+), 3 deletions(-)
Reviewed-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 37c6b4981db..e27aedad354 100644
> --- a/tests/unit/test-smp-parse.c
> +++ b/tests/unit/test-smp-parse.c
> @@ -487,7 +487,7 @@ static void machine_base_class_init(ObjectClass *oc, void *data)
> mc->name = g_strdup(SMP_MACHINE_NAME);
> }
>
> -static void test_generic(const void *opaque)
> +static void test_generic_valid(const void *opaque)
> {
> const char *machine_type = opaque;
> Object *obj = object_new(machine_type);
> @@ -508,6 +508,19 @@ static void test_generic(const void *opaque)
> smp_parse_test(ms, data, true);
> }
>
> + object_unref(obj);
> +}
> +
> +static void test_generic_invalid(const void *opaque)
> +{
> + const char *machine_type = opaque;
> + Object *obj = object_new(machine_type);
> + MachineState *ms = MACHINE(obj);
> + MachineClass *mc = MACHINE_GET_CLASS(obj);
> + SMPTestData *data = &(SMPTestData){};
> + int i;
> +
> +
> /* Force invalid min CPUs and max CPUs */
> mc->min_cpus = 2;
> mc->max_cpus = 511;
> @@ -601,9 +614,12 @@ int main(int argc, char *argv[])
>
> g_test_init(&argc, &argv, NULL);
>
> - g_test_add_data_func("/test-smp-parse/generic",
> + g_test_add_data_func("/test-smp-parse/generic/valid",
> TYPE_MACHINE,
> - test_generic);
> + test_generic_valid);
> + g_test_add_data_func("/test-smp-parse/generic/invalid",
> + TYPE_MACHINE,
> + test_generic_invalid);
> g_test_add_data_func("/test-smp-parse/with_dies",
> TYPE_MACHINE,
> test_with_dies);
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH-for-7.0 v4 05/11] tests/unit/test-smp-parse: Split the 'generic' test in valid / invalid
2021-11-15 14:58 ` [PATCH-for-7.0 v4 05/11] tests/unit/test-smp-parse: Split the 'generic' test in valid / invalid Philippe Mathieu-Daudé
2021-11-16 12:06 ` Richard Henderson
2021-11-16 13:58 ` wangyanan (Y)
@ 2021-11-16 14:07 ` wangyanan (Y)
2021-11-16 14:22 ` Richard Henderson
2 siblings, 1 reply; 27+ messages in thread
From: wangyanan (Y) @ 2021-11-16 14:07 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Thomas Huth, Andrew Jones, Markus Armbruster, Eduardo Habkost
On 2021/11/15 22:58, Philippe Mathieu-Daudé wrote:
> Split the 'generic' test in two tests: 'valid' and 'invalid'.
> This will allow us to remove the hack which modifies the
> MachineClass internal state.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> tests/unit/test-smp-parse.c | 22 +++++++++++++++++++---
> 1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
> index 37c6b4981db..e27aedad354 100644
> --- a/tests/unit/test-smp-parse.c
> +++ b/tests/unit/test-smp-parse.c
> @@ -487,7 +487,7 @@ static void machine_base_class_init(ObjectClass *oc, void *data)
> mc->name = g_strdup(SMP_MACHINE_NAME);
> }
>
> -static void test_generic(const void *opaque)
> +static void test_generic_valid(const void *opaque)
> {
> const char *machine_type = opaque;
> Object *obj = object_new(machine_type);
> @@ -508,6 +508,19 @@ static void test_generic(const void *opaque)
> smp_parse_test(ms, data, true);
> }
>
> + object_unref(obj);
> +}
> +
> +static void test_generic_invalid(const void *opaque)
> +{
> + const char *machine_type = opaque;
> + Object *obj = object_new(machine_type);
> + MachineState *ms = MACHINE(obj);
> + MachineClass *mc = MACHINE_GET_CLASS(obj);
> + SMPTestData *data = &(SMPTestData){};
> + int i;
> +
> +
Ah, there is an extra empty line which should be deleted.
Thanks,
Yanan
> /* Force invalid min CPUs and max CPUs */
> mc->min_cpus = 2;
> mc->max_cpus = 511;
> @@ -601,9 +614,12 @@ int main(int argc, char *argv[])
>
> g_test_init(&argc, &argv, NULL);
>
> - g_test_add_data_func("/test-smp-parse/generic",
> + g_test_add_data_func("/test-smp-parse/generic/valid",
> TYPE_MACHINE,
> - test_generic);
> + test_generic_valid);
> + g_test_add_data_func("/test-smp-parse/generic/invalid",
> + TYPE_MACHINE,
> + test_generic_invalid);
> g_test_add_data_func("/test-smp-parse/with_dies",
> TYPE_MACHINE,
> test_with_dies);
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH-for-7.0 v4 06/11] tests/unit/test-smp-parse: Add 'smp-with-dies' machine type
2021-11-15 14:58 ` [PATCH-for-7.0 v4 06/11] tests/unit/test-smp-parse: Add 'smp-with-dies' machine type Philippe Mathieu-Daudé
2021-11-16 12:07 ` Richard Henderson
@ 2021-11-16 14:10 ` wangyanan (Y)
1 sibling, 0 replies; 27+ messages in thread
From: wangyanan (Y) @ 2021-11-16 14:10 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Thomas Huth, Andrew Jones, Markus Armbruster, Eduardo Habkost
On 2021/11/15 22:58, Philippe Mathieu-Daudé wrote:
> Avoid modifying the MachineClass internals by adding the
> 'smp-with-dies' machine, which inherits from TYPE_MACHINE.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> tests/unit/test-smp-parse.c | 22 +++++++++++++++-------
> 1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
> index e27aedad354..ff61da06e3d 100644
> --- a/tests/unit/test-smp-parse.c
> +++ b/tests/unit/test-smp-parse.c
> @@ -487,6 +487,16 @@ static void machine_base_class_init(ObjectClass *oc, void *data)
> mc->name = g_strdup(SMP_MACHINE_NAME);
> }
>
> +static void machine_with_dies_class_init(ObjectClass *oc, void *data)
> +{
> + MachineClass *mc = MACHINE_CLASS(oc);
> +
> + mc->min_cpus = MIN_CPUS;
> + mc->max_cpus = MAX_CPUS;
> +
> + mc->smp_props.dies_supported = true;
> +}
> +
> static void test_generic_valid(const void *opaque)
> {
> const char *machine_type = opaque;
> @@ -549,9 +559,6 @@ static void test_with_dies(const void *opaque)
> unsigned int num_dies = 2;
> int i;
>
> - /* Force the SMP compat properties */
> - 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);
> @@ -589,9 +596,6 @@ static void test_with_dies(const void *opaque)
> smp_parse_test(ms, data, false);
> }
>
> - /* Restore the SMP compat properties */
> - mc->smp_props.dies_supported = false;
> -
> object_unref(obj);
> }
>
> @@ -603,6 +607,10 @@ static const TypeInfo smp_machine_types[] = {
> .class_init = machine_base_class_init,
> .class_size = sizeof(MachineClass),
> .instance_size = sizeof(MachineState),
> + }, {
> + .name = MACHINE_TYPE_NAME("smp-with-dies"),
> + .parent = TYPE_MACHINE,
> + .class_init = machine_with_dies_class_init,
> }
> };
>
> @@ -621,7 +629,7 @@ int main(int argc, char *argv[])
> TYPE_MACHINE,
> test_generic_invalid);
> g_test_add_data_func("/test-smp-parse/with_dies",
> - TYPE_MACHINE,
> + MACHINE_TYPE_NAME("smp-with-dies"),
> test_with_dies);
>
> g_test_run();
I have tested this patch locally and all works as expected:
Reviewed-by: Yanan Wang <wangyanan55@huawei.com>
Tested-by: Yanan Wang <wangyanan55@huawei.com>
Thanks,
Yanan
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH-for-7.0 v4 05/11] tests/unit/test-smp-parse: Split the 'generic' test in valid / invalid
2021-11-16 14:07 ` wangyanan (Y)
@ 2021-11-16 14:22 ` Richard Henderson
2021-11-16 15:15 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 27+ messages in thread
From: Richard Henderson @ 2021-11-16 14:22 UTC (permalink / raw)
To: wangyanan (Y), Philippe Mathieu-Daudé, qemu-devel
Cc: Thomas Huth, Andrew Jones, Eduardo Habkost, Markus Armbruster
On 11/16/21 3:07 PM, wangyanan (Y) wrote:
>> + int i;
>> +
>> +
> Ah, there is an extra empty line which should be deleted.
I noticed that too, but it gets deleted in patch 7, so I let it be. ;-)
r~
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH-for-7.0 v4 05/11] tests/unit/test-smp-parse: Split the 'generic' test in valid / invalid
2021-11-16 14:22 ` Richard Henderson
@ 2021-11-16 15:15 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-11-16 15:15 UTC (permalink / raw)
To: Richard Henderson, wangyanan (Y), qemu-devel
Cc: Thomas Huth, Andrew Jones, Eduardo Habkost, Markus Armbruster
On 11/16/21 15:22, Richard Henderson wrote:
> On 11/16/21 3:07 PM, wangyanan (Y) wrote:
>>> + int i;
>>> +
>>> +
>> Ah, there is an extra empty line which should be deleted.
>
> I noticed that too, but it gets deleted in patch 7, so I let it be. ;-)
Oops sorry, I'll clean that once v7.0 is released.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH-for-7.0 v4 08/11] tests/unit/test-smp-parse: Add 'smp-without-dies-valid' machine type
2021-11-15 14:58 ` [PATCH-for-7.0 v4 08/11] tests/unit/test-smp-parse: Add 'smp-without-dies-valid' " Philippe Mathieu-Daudé
2021-11-16 12:08 ` Richard Henderson
@ 2021-11-17 7:37 ` wangyanan (Y)
2021-11-17 8:08 ` Philippe Mathieu-Daudé
1 sibling, 1 reply; 27+ messages in thread
From: wangyanan (Y) @ 2021-11-17 7:37 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Thomas Huth, Andrew Jones, Markus Armbruster, Eduardo Habkost
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é <philmd@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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH-for-7.0 v4 08/11] tests/unit/test-smp-parse: Add 'smp-without-dies-valid' machine type
2021-11-17 7:37 ` wangyanan (Y)
@ 2021-11-17 8:08 ` Philippe Mathieu-Daudé
2021-11-17 10:51 ` wangyanan (Y)
0 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-11-17 8:08 UTC (permalink / raw)
To: wangyanan (Y), qemu-devel
Cc: Thomas Huth, Andrew Jones, Markus Armbruster, Eduardo Habkost
Hi Yanan,
On 11/17/21 08:37, wangyanan (Y) wrote:
> 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é <philmd@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.
OK I modified it and will respin once v6.2 is released.
Also test_with_dies() should be split in 2 tests: valid/invalid;
then smp_parse_test() should split tests further regarding the
socket preference. But I'll let that to you, I wanted to 1/ fix
our Windows CI and 2/ show you how to structure the tests.
Regards,
Phil.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH-for-7.0 v4 08/11] tests/unit/test-smp-parse: Add 'smp-without-dies-valid' machine type
2021-11-17 8:08 ` Philippe Mathieu-Daudé
@ 2021-11-17 10:51 ` wangyanan (Y)
0 siblings, 0 replies; 27+ messages in thread
From: wangyanan (Y) @ 2021-11-17 10:51 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Thomas Huth, Andrew Jones, Markus Armbruster, Eduardo Habkost
On 2021/11/17 16:08, Philippe Mathieu-Daudé wrote:
> Hi Yanan,
>
> On 11/17/21 08:37, wangyanan (Y) wrote:
>> 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é <philmd@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.
> OK I modified it and will respin once v6.2 is released.
>
> Also test_with_dies() should be split in 2 tests: valid/invalid;
> then smp_parse_test() should split tests further regarding the
> socket preference. But I'll let that to you,
Sure, I can do this in an appropriate time. But IMHO, I don't see a
strong necessity to split test_with_dies for now, as the valid/invalid
scenarios share the same class properties. We can probably keep it
as is until we have to split it.
As for socket preference, I can foresee code duplication if we split
all the tests into two parts just regarding the socket preference.
Isn't it clear enough to use current smp_parse_test() for each
test data sample? Do we have other concern here?
> I wanted to 1/ fix
> our Windows CI and 2/ show you how to structure the tests.
Understood. The test architecture is indeed improved a lot.
Thanks for your education. 😉
Thanks,
Yanan
> Regards,
>
> Phil.
>
> .
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2021-11-17 10:54 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-15 14:58 [PATCH v4 00/11] tests/unit: Fix test-smp-parse Philippe Mathieu-Daudé
2021-11-15 14:58 ` [PATCH-for-6.2 v4 01/11] tests/unit/test-smp-parse: Restore MachineClass fields after modifying Philippe Mathieu-Daudé
2021-11-15 14:58 ` [PATCH-for-6.2 v4 02/11] tests/unit/test-smp-parse: QOM'ify smp_machine_class_init() Philippe Mathieu-Daudé
2021-11-15 14:58 ` [PATCH-for-6.2 v4 03/11] tests/unit/test-smp-parse: Explicit MachineClass name Philippe Mathieu-Daudé
2021-11-15 14:58 ` [PATCH-for-7.0 v4 04/11] tests/unit/test-smp-parse: Pass machine type as argument to tests Philippe Mathieu-Daudé
2021-11-16 12:05 ` Richard Henderson
2021-11-16 13:57 ` wangyanan (Y)
2021-11-15 14:58 ` [PATCH-for-7.0 v4 05/11] tests/unit/test-smp-parse: Split the 'generic' test in valid / invalid Philippe Mathieu-Daudé
2021-11-16 12:06 ` Richard Henderson
2021-11-16 13:58 ` wangyanan (Y)
2021-11-16 14:07 ` wangyanan (Y)
2021-11-16 14:22 ` Richard Henderson
2021-11-16 15:15 ` Philippe Mathieu-Daudé
2021-11-15 14:58 ` [PATCH-for-7.0 v4 06/11] tests/unit/test-smp-parse: Add 'smp-with-dies' machine type Philippe Mathieu-Daudé
2021-11-16 12:07 ` Richard Henderson
2021-11-16 14:10 ` wangyanan (Y)
2021-11-15 14:58 ` [PATCH-for-7.0 v4 07/11] tests/unit/test-smp-parse: Add 'smp-without-dies-invalid' " Philippe Mathieu-Daudé
2021-11-16 12:07 ` Richard Henderson
2021-11-15 14:58 ` [PATCH-for-7.0 v4 08/11] tests/unit/test-smp-parse: Add 'smp-without-dies-valid' " Philippe Mathieu-Daudé
2021-11-16 12:08 ` Richard Henderson
2021-11-17 7:37 ` wangyanan (Y)
2021-11-17 8:08 ` Philippe Mathieu-Daudé
2021-11-17 10:51 ` wangyanan (Y)
2021-11-15 14:58 ` [PATCH-for-7.0 v4 09/11] tests/unit/test-smp-parse: Simplify pointer to compound literal use Philippe Mathieu-Daudé
2021-11-15 14:58 ` [PATCH-for-7.0 v4 10/11] tests/unit/test-smp-parse: Constify some pointer/struct Philippe Mathieu-Daudé
2021-11-15 14:59 ` [PATCH-for-7.0 v4 11/11] hw/core: Rename smp_parse() -> machine_parse_smp_config() Philippe Mathieu-Daudé
2021-11-15 22:49 ` [PATCH v4 00/11] tests/unit: Fix test-smp-parse Philippe Mathieu-Daudé
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.