[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] tests/qtest: Unify the test for the xenfv and xenpv machines
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH] tests/qtest: Unify the test for the xenfv and xenpv machines |
Date: |
Mon, 22 Jun 2020 13:23:03 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
On 6/22/20 12:43 PM, Thomas Huth wrote:
> We have the same check in three places. Let's unify it in a central
> place instead.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> tests/qtest/device-introspect-test.c | 5 -----
> tests/qtest/libqtest.c | 4 ++++
> tests/qtest/qom-test.c | 5 -----
> tests/qtest/test-hmp.c | 5 -----
> 4 files changed, 4 insertions(+), 15 deletions(-)
>
> diff --git a/tests/qtest/device-introspect-test.c
> b/tests/qtest/device-introspect-test.c
> index f2c1576cae..9abb5ec889 100644
> --- a/tests/qtest/device-introspect-test.c
> +++ b/tests/qtest/device-introspect-test.c
> @@ -287,11 +287,6 @@ static void add_machine_test_case(const char *mname)
> {
> char *path, *args;
>
> - /* Ignore blacklisted machines */
> - if (!memcmp("xenfv", mname, 5) || g_str_equal("xenpv", mname)) {
> - return;
> - }
> -
> path = g_strdup_printf("device/introspect/concrete/defaults/%s", mname);
> args = g_strdup_printf("-M %s", mname);
> qtest_add_data_func(path, args, test_device_intro_concrete);
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index 49075b55a1..fd4680590d 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -1232,6 +1232,10 @@ void qtest_cb_for_every_machine(void (*cb)(const char
> *machine),
> qstr = qobject_to(QString, qobj);
> g_assert(qstr);
> mname = qstring_get_str(qstr);
> + /* Ignore machines that cannot be used for qtests */
> + if (!memcmp("xenfv", mname, 5) || g_str_equal("xenpv", mname)) {
> + continue;
> + }
The patch is correct.
Since qtest uses QMP introspection, it would be cleaner to have
a MachineClass::qtest_allowed property (default to true), set
it to false in the xenfv/pv machines, and either check the property
here or have a query-qtest-machines QMP command.
Just thinking loudly.
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> if (!skip_old_versioned || !qtest_is_old_versioned_machine(mname)) {
> cb(mname);
> }
> diff --git a/tests/qtest/qom-test.c b/tests/qtest/qom-test.c
> index e338a41194..1acf0d7369 100644
> --- a/tests/qtest/qom-test.c
> +++ b/tests/qtest/qom-test.c
> @@ -81,11 +81,6 @@ static void add_machine_test_case(const char *mname)
> {
> char *path;
>
> - /* Ignore blacklisted machines that have known problems */
> - if (!memcmp("xenfv", mname, 5) || g_str_equal("xenpv", mname)) {
> - return;
> - }
> -
> path = g_strdup_printf("qom/%s", mname);
> qtest_add_data_func(path, g_strdup(mname), test_machine);
> g_free(path);
> diff --git a/tests/qtest/test-hmp.c b/tests/qtest/test-hmp.c
> index b8b1271b9e..d5e7ebd176 100644
> --- a/tests/qtest/test-hmp.c
> +++ b/tests/qtest/test-hmp.c
> @@ -143,11 +143,6 @@ static void add_machine_test_case(const char *mname)
> {
> char *path;
>
> - /* Ignore blacklisted machines that have known problems */
> - if (!memcmp("xenfv", mname, 5) || g_str_equal("xenpv", mname)) {
> - return;
> - }
> -
> path = g_strdup_printf("hmp/%s", mname);
> qtest_add_data_func(path, g_strdup(mname), test_machine);
> g_free(path);
>