[Qemu-devel] [PATCH for-4.0 0/5] Make more tests independent of global_qtest

[Qemu-devel] [PATCH for-4.0 0/5] Make more tests independent of global_qtest

[Thomas Huth]

We should avoid and finally get rid of the global_qtest variable in
our tests, since it causes trouble when running tests in parallel,
e.g. migration tests. Here are some more small patches to make some
tests independent of this variable.

Thomas Huth (5):
  tests/boot-serial: Get rid of global_qtest variable
  tests/test-filter: Make tests independent of global_qtest
  tests/machine-none: Make test independent of global_qtest
  tests/prom-env: Make test independent of global_qtest
  tests/pxe: Make test independent from global_qtest

 tests/boot-serial-test.c       | 13 +++++++------
 tests/machine-none-test.c      |  7 ++++---
 tests/prom-env-test.c          | 17 ++++++++---------
 tests/pxe-test.c               |  7 ++++---
 tests/test-filter-mirror.c     |  9 +++++----
 tests/test-filter-redirector.c | 16 +++++++++-------
 6 files changed, 37 insertions(+), 32 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH for-4.0 1/5] tests/boot-serial: Get rid of global_qtest variable

[Thomas Huth]

The test does not use any of the functions that require global_qtest,
so we can simply get rid of this global variable here.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/boot-serial-test.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c
index 8ec6aed..f085165 100644
--- a/tests/boot-serial-test.c
+++ b/tests/boot-serial-test.c
@@ -161,6 +161,7 @@ static void test_machine(const void *data)
     char codetmp[] = "/tmp/qtest-boot-serial-cXXXXXX";
     const char *codeparam = "";
     const uint8_t *code = NULL;
+    QTestState *qts;
     int ser_fd;
 
     ser_fd = mkstemp(serialtmp);
@@ -189,11 +190,11 @@ static void test_machine(const void *data)
      * Make sure that this test uses tcg if available: It is used as a
      * fast-enough smoketest for that.
      */
-    global_qtest = qtest_initf("%s %s -M %s,accel=tcg:kvm "
-                               "-chardev file,id=serial0,path=%s "
-                               "-no-shutdown -serial chardev:serial0 %s",
-                               codeparam, code ? codetmp : "",
-                               test->machine, serialtmp, test->extra);
+    qts = qtest_initf("%s %s -M %s,accel=tcg:kvm -no-shutdown "
+                      "-chardev file,id=serial0,path=%s "
+                      " -serial chardev:serial0 %s",
+                      codeparam, code ? codetmp : "", test->machine,
+                      serialtmp, test->extra);
     if (code) {
         unlink(codetmp);
     }
@@ -204,7 +205,7 @@ static void test_machine(const void *data)
     }
     unlink(serialtmp);
 
-    qtest_quit(global_qtest);
+    qtest_quit(qts);
 
     close(ser_fd);
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH for-4.0 2/5] tests/test-filter: Make tests independent of global_qtest

[Thomas Huth]

Apart from using qmp() in the qmp_discard_response() macro, these
tests do not have any dependencies to the global_qtest variable,
so we can simply get rid of it here by replacing the qmp() with
qtest_qmp() in the macro.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/test-filter-mirror.c     |  9 +++++----
 tests/test-filter-redirector.c | 16 +++++++++-------
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/tests/test-filter-mirror.c b/tests/test-filter-mirror.c
index d15917e..7ab2aed 100644
--- a/tests/test-filter-mirror.c
+++ b/tests/test-filter-mirror.c
@@ -17,7 +17,7 @@
 #include "qemu/main-loop.h"
 
 /* TODO actually test the results and get rid of this */
-#define qmp_discard_response(...) qobject_unref(qmp(__VA_ARGS__))
+#define qmp_discard_response(qs, ...) qobject_unref(qtest_qmp(qs, __VA_ARGS__))
 
 static void test_mirror(void)
 {
@@ -29,6 +29,7 @@ static void test_mirror(void)
     uint32_t size = sizeof(send_buf);
     size = htonl(size);
     const char *devstr = "e1000";
+    QTestState *qts;
 
     if (g_str_equal(qtest_get_arch(), "s390x")) {
         devstr = "virtio-net-ccw";
@@ -40,7 +41,7 @@ static void test_mirror(void)
     ret = mkstemp(sock_path);
     g_assert_cmpint(ret, !=, -1);
 
-    global_qtest = qtest_initf(
+    qts = qtest_initf(
         "-netdev socket,id=qtest-bn0,fd=%d "
         "-device %s,netdev=qtest-bn0,id=qtest-e0 "
         "-chardev socket,id=mirror0,path=%s,server,nowait "
@@ -61,7 +62,7 @@ static void test_mirror(void)
     };
 
     /* send a qmp command to guarantee that 'connected' is setting to true. */
-    qmp_discard_response("{ 'execute' : 'query-status'}");
+    qmp_discard_response(qts, "{ 'execute' : 'query-status'}");
     ret = iov_send(send_sock[0], iov, 2, 0, sizeof(size) + sizeof(send_buf));
     g_assert_cmpint(ret, ==, sizeof(send_buf) + sizeof(size));
     close(send_sock[0]);
@@ -78,6 +79,7 @@ static void test_mirror(void)
     g_free(recv_buf);
     close(recv_sock);
     unlink(sock_path);
+    qtest_quit(qts);
 }
 
 int main(int argc, char **argv)
@@ -88,7 +90,6 @@ int main(int argc, char **argv)
 
     qtest_add_func("/netfilter/mirror", test_mirror);
     ret = g_test_run();
-    qtest_end();
 
     return ret;
 }
diff --git a/tests/test-filter-redirector.c b/tests/test-filter-redirector.c
index 615ff5c..9ca9fea 100644
--- a/tests/test-filter-redirector.c
+++ b/tests/test-filter-redirector.c
@@ -59,7 +59,7 @@
 #include "qemu/main-loop.h"
 
 /* TODO actually test the results and get rid of this */
-#define qmp_discard_response(...) qobject_unref(qmp(__VA_ARGS__))
+#define qmp_discard_response(qs, ...) qobject_unref(qtest_qmp(qs, __VA_ARGS__))
 
 static const char *get_devstr(void)
 {
@@ -81,6 +81,7 @@ static void test_redirector_tx(void)
     char *recv_buf;
     uint32_t size = sizeof(send_buf);
     size = htonl(size);
+    QTestState *qts;
 
     ret = socketpair(PF_UNIX, SOCK_STREAM, 0, backend_sock);
     g_assert_cmpint(ret, !=, -1);
@@ -90,7 +91,7 @@ static void test_redirector_tx(void)
     ret = mkstemp(sock_path1);
     g_assert_cmpint(ret, !=, -1);
 
-    global_qtest = qtest_initf(
+    qts = qtest_initf(
         "-netdev socket,id=qtest-bn0,fd=%d "
         "-device %s,netdev=qtest-bn0,id=qtest-e0 "
         "-chardev socket,id=redirector0,path=%s,server,nowait "
@@ -108,7 +109,7 @@ static void test_redirector_tx(void)
     g_assert_cmpint(recv_sock, !=, -1);
 
     /* send a qmp command to guarantee that 'connected' is setting to true. */
-    qmp_discard_response("{ 'execute' : 'query-status'}");
+    qmp_discard_response(qts, "{ 'execute' : 'query-status'}");
 
     struct iovec iov[] = {
         {
@@ -137,7 +138,7 @@ static void test_redirector_tx(void)
     close(recv_sock);
     unlink(sock_path0);
     unlink(sock_path1);
-    qtest_end();
+    qtest_quit(qts);
 }
 
 static void test_redirector_rx(void)
@@ -150,6 +151,7 @@ static void test_redirector_rx(void)
     char *recv_buf;
     uint32_t size = sizeof(send_buf);
     size = htonl(size);
+    QTestState *qts;
 
     ret = socketpair(PF_UNIX, SOCK_STREAM, 0, backend_sock);
     g_assert_cmpint(ret, !=, -1);
@@ -159,7 +161,7 @@ static void test_redirector_rx(void)
     ret = mkstemp(sock_path1);
     g_assert_cmpint(ret, !=, -1);
 
-    global_qtest = qtest_initf(
+    qts = qtest_initf(
         "-netdev socket,id=qtest-bn0,fd=%d "
         "-device %s,netdev=qtest-bn0,id=qtest-e0 "
         "-chardev socket,id=redirector0,path=%s,server,nowait "
@@ -186,7 +188,7 @@ static void test_redirector_rx(void)
     send_sock = unix_connect(sock_path1, NULL);
     g_assert_cmpint(send_sock, !=, -1);
     /* send a qmp command to guarantee that 'connected' is setting to true. */
-    qmp_discard_response("{ 'execute' : 'query-status'}");
+    qmp_discard_response(qts, "{ 'execute' : 'query-status'}");
 
     ret = iov_send(send_sock, iov, 2, 0, sizeof(size) + sizeof(send_buf));
     g_assert_cmpint(ret, ==, sizeof(send_buf) + sizeof(size));
@@ -204,7 +206,7 @@ static void test_redirector_rx(void)
     g_free(recv_buf);
     unlink(sock_path0);
     unlink(sock_path1);
-    qtest_end();
+    qtest_quit(qts);
 }
 
 int main(int argc, char **argv)
-- 
1.8.3.1

[Qemu-devel] [PATCH for-4.0 3/5] tests/machine-none: Make test independent of global_qtest

[Thomas Huth]

Apart from using qmp() in one spot, this test does not have any
dependencies to the global_qtest variable, so we can simply get
rid of it here by replacing the qmp() with qtest_qmp().

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/machine-none-test.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tests/machine-none-test.c b/tests/machine-none-test.c
index 2b3b750..4c6d470 100644
--- a/tests/machine-none-test.c
+++ b/tests/machine-none-test.c
@@ -75,6 +75,7 @@ static void test_machine_cpu_cli(void)
     QDict *response;
     const char *arch = qtest_get_arch();
     const char *cpu_model = get_cpu_model_by_arch(arch);
+    QTestState *qts;
 
     if (!cpu_model) {
         if (!(!strcmp(arch, "microblaze") || !strcmp(arch, "microblazeel"))) {
@@ -83,13 +84,13 @@ static void test_machine_cpu_cli(void)
         }
         return; /* TODO: die here to force all targets have a test */
     }
-    global_qtest = qtest_initf("-machine none -cpu '%s'", cpu_model);
+    qts = qtest_initf("-machine none -cpu '%s'", cpu_model);
 
-    response = qmp("{ 'execute': 'quit' }");
+    response = qtest_qmp(qts, "{ 'execute': 'quit' }");
     g_assert(qdict_haskey(response, "return"));
     qobject_unref(response);
 
-    qtest_quit(global_qtest);
+    qtest_quit(qts);
 }
 
 int main(int argc, char **argv)
-- 
1.8.3.1

[Qemu-devel] [PATCH for-4.0 4/5] tests/prom-env: Make test independent of global_qtest

[Thomas Huth]

global_qtest is only needed here for one readl(). Let's replace it
with qtest_readl() and we can remove the global_qtest variable here.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/prom-env-test.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/tests/prom-env-test.c b/tests/prom-env-test.c
index 198d007..4821254 100644
--- a/tests/prom-env-test.c
+++ b/tests/prom-env-test.c
@@ -25,14 +25,14 @@
 #define MAGIC   0xcafec0de
 #define ADDRESS 0x4000
 
-static void check_guest_memory(void)
+static void check_guest_memory(QTestState *qts)
 {
     uint32_t signature;
     int i;
 
     /* Poll until code has run and modified memory. Wait at most 600 seconds */
     for (i = 0; i < 60000; ++i) {
-        signature = readl(ADDRESS);
+        signature = qtest_readl(qts, ADDRESS);
         if (signature == MAGIC) {
             break;
         }
@@ -45,17 +45,16 @@ static void check_guest_memory(void)
 static void test_machine(const void *machine)
 {
     const char *extra_args;
+    QTestState *qts;
 
     /* The pseries firmware boots much faster without the default devices */
     extra_args = strcmp(machine, "pseries") == 0 ? "-nodefaults" : "";
 
-    global_qtest = qtest_initf("-M %s,accel=tcg %s "
-                               "-prom-env 'use-nvramrc?=true' "
-                               "-prom-env 'nvramrc=%x %x l!' ",
-                               (const char *)machine, extra_args,
-                               MAGIC, ADDRESS);
-    check_guest_memory();
-    qtest_quit(global_qtest);
+    qts = qtest_initf("-M %s,accel=tcg %s -prom-env 'use-nvramrc?=true' "
+                      "-prom-env 'nvramrc=%x %x l!' ", (const char *)machine,
+                      extra_args, MAGIC, ADDRESS);
+    check_guest_memory(qts);
+    qtest_quit(qts);
 }
 
 static void add_tests(const char *machines[])
-- 
1.8.3.1

[Qemu-devel] [PATCH for-4.0 5/5] tests/pxe: Make test independent from global_qtest

[Thomas Huth]

global_qtest is not really required here, since boot_sector_test()
is already independent from that global variable.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/pxe-test.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tests/pxe-test.c b/tests/pxe-test.c
index 6e36796..73ac1d1 100644
--- a/tests/pxe-test.c
+++ b/tests/pxe-test.c
@@ -61,6 +61,7 @@ static testdef_t s390x_tests[] = {
 
 static void test_pxe_one(const testdef_t *test, bool ipv6)
 {
+    QTestState *qts;
     char *args;
 
     args = g_strdup_printf(
@@ -70,9 +71,9 @@ static void test_pxe_one(const testdef_t *test, bool ipv6)
         test->machine, disk, ipv6 ? "off" : "on", ipv6 ? "on" : "off",
         test->model);
 
-    qtest_start(args);
-    boot_sector_test(global_qtest);
-    qtest_quit(global_qtest);
+    qts = qtest_init(args);
+    boot_sector_test(qts);
+    qtest_quit(qts);
     g_free(args);
 }
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH for-4.0 1/5] tests/boot-serial: Get rid of global_qtest variable

[Eric Blake]

On 12/6/18 10:49 AM, Thomas Huth wrote:
> The test does not use any of the functions that require global_qtest,
> so we can simply get rid of this global variable here.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   tests/boot-serial-test.c | 13 +++++++------
>   1 file changed, 7 insertions(+), 6 deletions(-)
> 

> -    global_qtest = qtest_initf("%s %s -M %s,accel=tcg:kvm "
> -                               "-chardev file,id=serial0,path=%s "
> -                               "-no-shutdown -serial chardev:serial0 %s",
> -                               codeparam, code ? codetmp : "",
> -                               test->machine, serialtmp, test->extra);
> +    qts = qtest_initf("%s %s -M %s,accel=tcg:kvm -no-shutdown "
> +                      "-chardev file,id=serial0,path=%s "
> +                      " -serial chardev:serial0 %s",

What happened to -no-shutdown?

> +                      codeparam, code ? codetmp : "", test->machine,
> +                      serialtmp, test->extra);
>       if (code) {
-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH for-4.0 2/5] tests/test-filter: Make tests independent of global_qtest

[Eric Blake]

On 12/6/18 10:49 AM, Thomas Huth wrote:
> Apart from using qmp() in the qmp_discard_response() macro, these
> tests do not have any dependencies to the global_qtest variable,
> so we can simply get rid of it here by replacing the qmp() with
> qtest_qmp() in the macro.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   tests/test-filter-mirror.c     |  9 +++++----
>   tests/test-filter-redirector.c | 16 +++++++++-------
>   2 files changed, 14 insertions(+), 11 deletions(-)
> 
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH for-4.0 3/5] tests/machine-none: Make test independent of global_qtest

[Eric Blake]

On 12/6/18 10:49 AM, Thomas Huth wrote:
> Apart from using qmp() in one spot, this test does not have any
> dependencies to the global_qtest variable, so we can simply get
> rid of it here by replacing the qmp() with qtest_qmp().
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   tests/machine-none-test.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH for-4.0 4/5] tests/prom-env: Make test independent of global_qtest

[Eric Blake]

On 12/6/18 10:49 AM, Thomas Huth wrote:
> global_qtest is only needed here for one readl(). Let's replace it
> with qtest_readl() and we can remove the global_qtest variable here.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   tests/prom-env-test.c | 17 ++++++++---------
>   1 file changed, 8 insertions(+), 9 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH for-4.0 5/5] tests/pxe: Make test independent from global_qtest

[Eric Blake]

On 12/6/18 10:49 AM, Thomas Huth wrote:
> global_qtest is not really required here, since boot_sector_test()
> is already independent from that global variable.

Inconsistency in the use of 'independent of' and 'independent from' 
across the series. I think both work grammatically (English is screwy 
that way), but I prefer 'independent of'; it also matches your existing 
commits such as 7cbe423c.

> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   tests/pxe-test.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH for-4.0 1/5] tests/boot-serial: Get rid of global_qtest variable

[Thomas Huth]

On 2018-12-06 19:42, Eric Blake wrote:
> On 12/6/18 10:49 AM, Thomas Huth wrote:
>> The test does not use any of the functions that require global_qtest,
>> so we can simply get rid of this global variable here.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   tests/boot-serial-test.c | 13 +++++++------
>>   1 file changed, 7 insertions(+), 6 deletions(-)
>>
> 
>> -    global_qtest = qtest_initf("%s %s -M %s,accel=tcg:kvm "
>> -                               "-chardev file,id=serial0,path=%s "
>> -                               "-no-shutdown -serial chardev:serial0
>> %s",
>> -                               codeparam, code ? codetmp : "",
>> -                               test->machine, serialtmp, test->extra);
>> +    qts = qtest_initf("%s %s -M %s,accel=tcg:kvm -no-shutdown "
>> +                      "-chardev file,id=serial0,path=%s "
>> +                      " -serial chardev:serial0 %s",
> 
> What happened to -no-shutdown?

Moved to the first line (since there was some more space there now).

 Thomas

Re: [Qemu-devel] [PATCH for-4.0 1/5] tests/boot-serial: Get rid of global_qtest variable

[Eric Blake]

On 12/6/18 1:34 PM, Thomas Huth wrote:
> On 2018-12-06 19:42, Eric Blake wrote:
>> On 12/6/18 10:49 AM, Thomas Huth wrote:
>>> The test does not use any of the functions that require global_qtest,
>>> so we can simply get rid of this global variable here.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>    tests/boot-serial-test.c | 13 +++++++------
>>>    1 file changed, 7 insertions(+), 6 deletions(-)
>>>
>>
>>> -    global_qtest = qtest_initf("%s %s -M %s,accel=tcg:kvm "
>>> -                               "-chardev file,id=serial0,path=%s "
>>> -                               "-no-shutdown -serial chardev:serial0
>>> %s",
>>> -                               codeparam, code ? codetmp : "",
>>> -                               test->machine, serialtmp, test->extra);
>>> +    qts = qtest_initf("%s %s -M %s,accel=tcg:kvm -no-shutdown "
>>> +                      "-chardev file,id=serial0,path=%s "
>>> +                      " -serial chardev:serial0 %s",
>>
>> What happened to -no-shutdown?
> 
> Moved to the first line (since there was some more space there now).

Aha - serves me right for thinking you just reindented, rather than 
spotting that you also rearranged in order to reflow the breaks.

You still have a minor issue of a double space (one to end the -chardev 
fragment, another to start the -serial fragment); with that fixed,

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org