[PATCH] tests/qtest: Unify the test for the xenfv and xenpv machines

Thomas Huth posted 1 patch 3 years, 9 months ago
Test FreeBSD passed
Test asan passed
Test docker-quick@centos7 passed
Test checkpatch passed
Test docker-mingw@fedora passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200622104339.21000-1-thuth@redhat.com
Maintainers: Laurent Vivier <lvivier@redhat.com>, Thomas Huth <thuth@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Paolo Bonzini <pbonzini@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(-)
[PATCH] tests/qtest: Unify the test for the xenfv and xenpv machines
Posted by Thomas Huth 3 years, 9 months ago
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;
+        }
         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);
-- 
2.18.1


Re: [PATCH] tests/qtest: Unify the test for the xenfv and xenpv machines
Posted by Philippe Mathieu-Daudé 3 years, 9 months ago
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);
>