Preliminary libqtest cleanups

Right now, tests/multiboot is not run by default during 'make check'; but when it is run in-tree, it was leaving some junk behind that could accidentally get checked in with a careless 'git add tests'.

Signed-off-by Eric Blake <[email protected]>

---
 tests/.gitignore | 2 ++
 1 file changed, 2 insertions(+)

tests/.gitignore +0 -0

index fed0189a5a..64ecd6683b 100644

Quoted 9 lines · Eric Blake

Right now, tests/multiboot is not run by default during 'make check'; but when it is run in-tree, it was leaving some junk behind that could accidentally get checked in with a careless 'git add tests'.

Signed-off-by Eric Blake <[email protected]>

---
 tests/.gitignore | 2 ++
 1 file changed, 2 insertions(+)

tests/.gitignore check-qnull · line 13

Quoted 9 lines

tests/.gitignore +1 -0

index fed0189a5a..64ecd6683b 100644
@@ -10,6 +10,8 @@ check-qnull
 check-qstring
 check-qom-interface
 check-qom-proplist
+multiboot/test.out

Your patch is of course a good idea ... but that test.out smells quite bad ... I guess it should (a) use mktemp for a better file name instead so that we can run this in parallel and (b) clean up the temporary file once the test has been done?

It doesn't matter if things are unsorted, but finding stuff in a list is easier when it is sorted. (Sorted under LC_ALL=C rules, rather than en_US.UTF-8).

Signed-off-by Eric Blake <[email protected]>

---
 tests/.gitignore | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

tests/.gitignore +14 -15

 atomic_add-bench
 benchmark-crypto-cipher
 benchmark-crypto-hash
 benchmark-crypto-hmac
 check-qdict
-check-qnum
 check-qjson
 check-qlist
 check-qnull
-check-qstring
+check-qnum
 check-qom-interface
 check-qom-proplist
-multiboot/test.out
+check-qstring
 multiboot/*.elf
+multiboot/test.out
+qapi-schema/*.test.*
 qht-bench
 rcutorture
 test-aio
 test-aio-multithread
 test-arm-mptimer
 test-base64
-test-bitops
 test-bitcnt
+test-bitops
 test-blockjob
 test-blockjob-txn
 test-bufferiszero

@@ -35,18 +37,19 @@ test-crypto-ivgen
 test-crypto-pbkdf
 test-crypto-secret
 test-crypto-tlscredsx509
-test-crypto-tlscredsx509-work/
 test-crypto-tlscredsx509-certs/
+test-crypto-tlscredsx509-work/
 test-crypto-tlssession
-test-crypto-tlssession-work/
 test-crypto-tlssession-client/
 test-crypto-tlssession-server/
+test-crypto-tlssession-work/
 test-crypto-xts
 test-cutils
+test-filter-mirror
+test-filter-redirector
 test-hbitmap
 test-hmp
 test-int128
-test-iov
 test-io-channel-buffer
 test-io-channel-command
 test-io-channel-command.fifo

@@ -55,27 +58,29 @@ test-io-channel-file.txt
 test-io-channel-socket
 test-io-channel-tls
 test-io-task
+test-iov
 test-keyval
 test-logging
 test-mul64
+test-netfilter
 test-opts-visitor
 test-qapi-event.[ch]
 test-qapi-types.[ch]
 test-qapi-util
 test-qapi-visit.[ch]
 test-qdev-global-props
-test-qemu-opts
 test-qdist
+test-qemu-opts
 test-qga
 test-qht
 test-qht-par
 test-qmp-commands
 test-qmp-commands.h
 test-qmp-event
-test-qobject-input-strict
-test-qobject-input-visitor
 test-qmp-introspect.[ch]
 test-qmp-marshal.c
+test-qobject-input-strict
+test-qobject-input-visitor
 test-qobject-output-visitor
 test-rcu-list
 test-replication

@@ -92,8 +97,3 @@ test-write-threshold
 test-x86-cpuid
 test-x86-cpuid-compat
 test-xbzrle
-test-netfilter
-test-filter-mirror
-test-filter-redirector
-*-test
-qapi-schema/*.test.*

Quoted 8 lines · Eric Blake

It doesn't matter if things are unsorted, but finding stuff in a list is easier when it is sorted. (Sorted under LC_ALL=C rules, rather than en_US.UTF-8).

Signed-off-by Eric Blake <[email protected]>

---
 tests/.gitignore | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

Reviewed-by Thomas Huth <[email protected]>

BTW, did you note the patch "build-sys: generate tests/.gitignore" from Marc-André ? Maybe the list can be generated automatically, too...

tests/.gitignore line 1

Quoted 6 lines

tests/.gitignore +1 -0

index 64ecd6683b..8d35a58751 100644
@@ -1,25 +1,27 @@
+*-test

... but looking at this wildcard, I wonder if we just named most of our tests in a bad way. If we name the files something-test instead of test-something, we maybe do not need the automatic generation from Marc-André (which is kind of ugly because it writes to the source directory - and this should not happen when doing out of tree builds).

So what do you think about renaming our tests, so that the above wildcard applies instead?

Quoted 28 lines · Thomas Huth

Quoted 8 lines · Eric Blake

It doesn't matter if things are unsorted, but finding stuff in a list is easier when it is sorted. (Sorted under LC_ALL=C rules, rather than en_US.UTF-8).

Signed-off-by Eric Blake <[email protected]>

---
 tests/.gitignore | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

Reviewed-by Thomas Huth <[email protected]>

BTW, did you note the patch "build-sys: generate tests/.gitignore" from Marc-André ? Maybe the list can be generated automatically, too...

tests/.gitignore line 1

Quoted 6 lines

tests/.gitignore +1 -0

index 64ecd6683b..8d35a58751 100644
@@ -1,25 +1,27 @@
+*-test

... but looking at this wildcard, I wonder if we just named most of our tests in a bad way. If we name the files something-test instead of test-something, we maybe do not need the automatic generation from Marc-André (which is kind of ugly because it writes to the source directory - and this should not happen when doing out of tree builds).

Quoted 2 lines

So what do you think about renaming our tests, so that the above wildcard applies instead?

Apropos naming tests: there's an (unspoken) convention to name unit tests check-FOO or test-FOO and tests using libqtest FOO-test, but it's not really honored anymore, probably because people creating tests didn't know about it.

Quoted 28 lines · Thomas Huth

Quoted 8 lines · Eric Blake

It doesn't matter if things are unsorted, but finding stuff in a list is easier when it is sorted. (Sorted under LC_ALL=C rules, rather than en_US.UTF-8).

Signed-off-by Eric Blake <[email protected]>

---
 tests/.gitignore | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

Reviewed-by Thomas Huth <[email protected]>

BTW, did you note the patch "build-sys: generate tests/.gitignore" from Marc-André ? Maybe the list can be generated automatically, too...

tests/.gitignore line 1

Quoted 6 lines

tests/.gitignore +1 -0

index 64ecd6683b..8d35a58751 100644
@@ -1,25 +1,27 @@
+*-test

... but looking at this wildcard, I wonder if we just named most of our tests in a bad way. If we name the files something-test instead of test-something, we maybe do not need the automatic generation from Marc-André (which is kind of ugly because it writes to the source directory - and this should not happen when doing out of tree builds).

Quoted 2 lines

So what do you think about renaming our tests, so that the above wildcard applies instead?

Apropos naming tests: there's an (unspoken) convention to name unit tests check-FOO or test-FOO and tests using libqtest FOO-test, but it's not really honored anymore, probably because people creating tests didn't know about it.

Quoted 30 lines · Markus Armbruster

Quoted 28 lines · Thomas Huth

Quoted 8 lines · Eric Blake

It doesn't matter if things are unsorted, but finding stuff in a list is easier when it is sorted. (Sorted under LC_ALL=C rules, rather than en_US.UTF-8).

Signed-off-by Eric Blake <[email protected]>

---
 tests/.gitignore | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

Reviewed-by Thomas Huth <[email protected]>

BTW, did you note the patch "build-sys: generate tests/.gitignore" from Marc-André ? Maybe the list can be generated automatically, too...

tests/.gitignore line 1

Quoted 6 lines

tests/.gitignore +1 -0

index 64ecd6683b..8d35a58751 100644
@@ -1,25 +1,27 @@
+*-test

... but looking at this wildcard, I wonder if we just named most of our tests in a bad way. If we name the files something-test instead of test-something, we maybe do not need the automatic generation from Marc-André (which is kind of ugly because it writes to the source directory - and this should not happen when doing out of tree builds).

Quoted 7 lines

Quoted 2 lines

So what do you think about renaming our tests, so that the above wildcard applies instead?

Apropos naming tests: there's an (unspoken) convention to name unit tests check-FOO or test-FOO and tests using libqtest FOO-test, but it's not really honored anymore, probably because people creating tests didn't know about it.

$ ls *test.c | wc -l
67
$ ls test*.c | wc -l
73
$ ls check*.c | wc -l
8

Fairly even split between 'test' as a prefix vs suffix. 'check' as a prefix should clearly be killed as a minority pattern. Any appetite for standardizing naming of everything else ?

A further idea might be to actually have separate sub-directories for unit tests vs qtests. eg go for

tests

    +- unit
    +- qtest
    +- iotests

Fairly even split between 'test' as a prefix vs suffix. 'check' as a prefix should clearly be killed as a minority pattern. Any appetite for standardizing naming of everything else ?

A further idea might be to actually have separate sub-directories for unit tests vs qtests. eg go for

tests

    +- unit
    +- qtest
    +- iotests

Back when the test was introduced, in commit 62c39b307, the test was set up to run qemu-ga directly on the host performing the test, and defaults to limiting itself to safe commands. At the time, it was envisioned that setting QGA_TEST_SIDE_EFFECTING in the environment could cover a few more commands, while noting the potential danger of those side effects running in the host.

But this has NEVER been tested: if you enable the environment variable, the test WILL fail. One obvious reason: if you are not running as root, you'll probably get a permission failure when trying to freeze the file systems, or when changing system time. Less obvious: if you run the test as root (wow, you're brave), you could end up hanging if the test tries to log things to a temporarily frozen filesystem. But the cutest reason of all: if you get past the above hurdles, the test uses invalid JSON in test_qga_fstrim() (missing '' around the dictionary key 'minimum'), and will thus fail an assertion in qmp_fd().

Rather than leave this untested time-bomb in place, rip it out. Hopefully, as originally envisioned, we can find an opportunity to test an actual sandboxed guest where the guest-agent has full permissions and will not unduly affect the host running the test - if so, 'git revert' can be used if desired, for salvaging any useful parts of this attempt.

Signed-off-by Eric Blake <[email protected]>

Reviewed-by Marc-André Lureau <[email protected]>

---
 tests/test-qga.c | 90 --------------------------------------------------------
 1 file changed, 90 deletions(-)

Assertions should be separate from the side effects, since in theory, g_assert() can be disabled (in practice, we can't really ever do that).

Signed-off-by Eric Blake <[email protected]>

---
 qtest.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 57 insertions(+), 23 deletions(-)

Don't open-code something that has a convenient helper available.

Signed-off-by Eric Blake <[email protected]>

Reviewed-by Markus Armbruster <[email protected]>

Reviewed-by Philippe Mathieu-Daudé <[email protected]>

---
 tests/numa-test.c | 21 +++------------------
 1 file changed, 3 insertions(+), 18 deletions(-)

Prior to commit 063c23d9, we were tracking a list of parallel qtest objects, in order to safely clean up a SIGABRT handler only after the last connection quits. But when we switched to more of glib's infrastructure, the list became dead code that is never assigned to.

Signed-off-by Eric Blake <[email protected]>

Reviewed-by Philippe Mathieu-Daudé <[email protected]>

---
 tests/libqtest.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

Rather than make multiple callers call strlen(), it's easier if socket_send() itself can compute a length via strlen() if none was provided (caller passes -1). Callers that can get at the length more efficiently are left that way.

Signed-off-by Eric Blake <[email protected]>

---
 tests/libqtest.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Quoted 5 lines · Eric Blake

Rather than make multiple callers call strlen(), it's easier if socket_send() itself can compute a length via strlen() if none was provided (caller passes -1). Callers that can get at the length more efficiently are left that way.

Quoted 4 lines

Signed-off-by Eric Blake <[email protected]>

---
 tests/libqtest.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

I have to say that I don't like this idea very much. socket_send() should IMHO not know about the type of the data that should be sent, i.e. it should not assume that the content is a zero-terminated string.

This also could lead to some hard to detect bugs later in case somebody is calling the function like this:

  size = someotherfunction();
  socket_send(fd, buf, size);

... and the someotherfunction() returned a negative error code instead of a correct size.

So I'd like to suggest to simply drop this patch.

Quoted 11 lines · Thomas Huth

Quoted 9 lines · Eric Blake

Rather than make multiple callers call strlen(), it's easier if socket_send() itself can compute a length via strlen() if none was provided (caller passes -1). Callers that can get at the length more efficiently are left that way.

Signed-off-by Eric Blake <[email protected]>

---
 tests/libqtest.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Quoted 3 lines

I have to say that I don't like this idea very much. socket_send() should IMHO not know about the type of the data that should be sent, i.e. it should not assume that the content is a zero-terminated string.

I agree.

Quoted 7 lines

This also could lead to some hard to detect bugs later in case somebody is calling the function like this:

  size = someotherfunction();
  socket_send(fd, buf, size);

Quoted 4 lines

... and the someotherfunction() returned a negative error code instead of a correct size.

So I'd like to suggest to simply drop this patch.

A separate wrapper function for sending zero-terminated strings would be fine with me.

This will keep checkpatch happy when the next patch does code motion. Fix the include order to match HACKING when adding the needed header.

Signed-off-by Eric Blake <[email protected]>

Reviewed-by Philippe Mathieu-Daudé <[email protected]>

---
 tests/libqtest.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

Put static functions prior to public ones, in part so that improvements to qtest_start() can benefit from the static helpers without needing forward references. Code motion, with no semantic change.

Signed-off-by Eric Blake <[email protected]>

---
 tests/libqtest.c | 263 +++++++++++++++++++++++++++----------------------------
 1 file changed, 131 insertions(+), 132 deletions(-)

tests/libqtest.c +131 -125

index 438a22678d..5d16351e24 100644
@@ -49,7 +49,6 @@ static struct sigaction sigact_old;
     g_assert_cmpint(ret, !=, -1); \
 } while (0)
 
-static int qtest_query_target_endianness(QTestState *s);
 
 static int init_socket(const char *socket_path)
 {

@@ -128,6 +127,137 @@ static void setup_sigabrt_handler(void)
     sigaction(SIGABRT, &sigact, &sigact_old);
 }
 
+static void socket_send(int fd, const char *buf, ssize_t size)
+{
+    size_t offset;
+
+    if (size < 0) {
+        size = strlen(buf);
+    }
+    offset = 0;
+    while (offset < size) {
+        ssize_t len;
+
+        len = write(fd, buf + offset, size - offset);
+        if (len == -1 && errno == EINTR) {
+            continue;
+        }
+
+        g_assert_no_errno(len);
+        g_assert_cmpint(len, >, 0);
+
+        offset += len;
+    }
+}
+
+static void socket_sendf(int fd, const char *fmt, va_list ap)
+{
+    gchar *str = g_strdup_vprintf(fmt, ap);
+
+    socket_send(fd, str, -1);
+    g_free(str);
+}
+
+static void GCC_FMT_ATTR(2, 3) qtest_sendf(QTestState *s, const char *fmt, ...)
+{
+    va_list ap;
+
+    va_start(ap, fmt);
+    socket_sendf(s->fd, fmt, ap);
+    va_end(ap);
+}
+
+static GString *qtest_recv_line(QTestState *s)
+{
+    GString *line;
+    size_t offset;
+    char *eol;
+
+    while ((eol = strchr(s->rx->str, '\n')) == NULL) {
+        ssize_t len;
+        char buffer[1024];
+
+        len = read(s->fd, buffer, sizeof(buffer));
+        if (len == -1 && errno == EINTR) {
+            continue;
+        }
+
+        if (len == -1 || len == 0) {
+            fprintf(stderr, "Broken pipe\n");
+            exit(1);
+        }
+
+        g_string_append_len(s->rx, buffer, len);
+    }
+
+    offset = eol - s->rx->str;
+    line = g_string_new_len(s->rx->str, offset);
+    g_string_erase(s->rx, 0, offset + 1);
+
+    return line;
+}
+
+static gchar **qtest_rsp(QTestState *s, int expected_args)
+{
+    GString *line;
+    gchar **words;
+    int i;
+
+redo:
+    line = qtest_recv_line(s);
+    words = g_strsplit(line->str, " ", 0);
+    g_string_free(line, TRUE);
+
+    if (strcmp(words[0], "IRQ") == 0) {
+        long irq;
+        int ret;
+
+        g_assert(words[1] != NULL);
+        g_assert(words[2] != NULL);
+
+        ret = qemu_strtol(words[2], NULL, 0, &irq);
+        g_assert(!ret);
+        g_assert_cmpint(irq, >=, 0);
+        g_assert_cmpint(irq, <, MAX_IRQ);
+
+        if (strcmp(words[1], "raise") == 0) {
+            s->irq_level[irq] = true;
+        } else {
+            s->irq_level[irq] = false;
+        }
+
+        g_strfreev(words);
+        goto redo;
+    }
+
+    g_assert(words[0] != NULL);
+    g_assert_cmpstr(words[0], ==, "OK");
+
+    if (expected_args) {
+        for (i = 0; i < expected_args; i++) {
+            g_assert(words[i] != NULL);
+        }
+    } else {
+        g_strfreev(words);
+    }
+
+    return words;
+}
+
+static int qtest_query_target_endianness(QTestState *s)
+{
+    gchar **args;
+    int big_endian;
+
+    qtest_sendf(s, "endianness\n");
+    args = qtest_rsp(s, 1);
+    g_assert(strcmp(args[1], "big") == 0 || strcmp(args[1], "little") == 0);
+    big_endian = strcmp(args[1], "big") == 0;
+    g_strfreev(args);
+
+    return big_endian;
+}
+
 static void cleanup_sigabrt_handler(void)
 {
     sigaction(SIGABRT, &sigact_old, NULL);

@@ -252,137 +382,6 @@ void qtest_quit(QTestState *s)
     g_free(s);
 }
 
-static void socket_send(int fd, const char *buf, ssize_t size)
-{
-    size_t offset;
-
-    if (size < 0) {
-        size = strlen(buf);
-    }
-    offset = 0;
-    while (offset < size) {
-        ssize_t len;
-
-        len = write(fd, buf + offset, size - offset);
-        if (len == -1 && errno == EINTR) {
-            continue;
-        }
-
-        g_assert_no_errno(len);
-        g_assert_cmpint(len, >, 0);
-
-        offset += len;
-    }
-}
-
-static void socket_sendf(int fd, const char *fmt, va_list ap)
-{
-    gchar *str = g_strdup_vprintf(fmt, ap);
-
-    socket_send(fd, str, -1);
-    g_free(str);
-}
-
-static void GCC_FMT_ATTR(2, 3) qtest_sendf(QTestState *s, const char *fmt, ...)
-{
-    va_list ap;
-
-    va_start(ap, fmt);
-    socket_sendf(s->fd, fmt, ap);
-    va_end(ap);
-}
-
-static GString *qtest_recv_line(QTestState *s)
-{
-    GString *line;
-    size_t offset;
-    char *eol;
-
-    while ((eol = strchr(s->rx->str, '\n')) == NULL) {
-        ssize_t len;
-        char buffer[1024];
-
-        len = read(s->fd, buffer, sizeof(buffer));
-        if (len == -1 && errno == EINTR) {
-            continue;
-        }
-
-        if (len == -1 || len == 0) {
-            fprintf(stderr, "Broken pipe\n");
-            exit(1);
-        }
-
-        g_string_append_len(s->rx, buffer, len);
-    }
-
-    offset = eol - s->rx->str;
-    line = g_string_new_len(s->rx->str, offset);
-    g_string_erase(s->rx, 0, offset + 1);
-
-    return line;
-}
-
-static gchar **qtest_rsp(QTestState *s, int expected_args)
-{
-    GString *line;
-    gchar **words;
-    int i;
-
-redo:
-    line = qtest_recv_line(s);
-    words = g_strsplit(line->str, " ", 0);
-    g_string_free(line, TRUE);
-
-    if (strcmp(words[0], "IRQ") == 0) {
-        long irq;
-        int ret;
-
-        g_assert(words[1] != NULL);
-        g_assert(words[2] != NULL);
-
-        ret = qemu_strtol(words[2], NULL, 0, &irq);
-        g_assert(!ret);
-        g_assert_cmpint(irq, >=, 0);
-        g_assert_cmpint(irq, <, MAX_IRQ);
-
-        if (strcmp(words[1], "raise") == 0) {
-            s->irq_level[irq] = true;
-        } else {
-            s->irq_level[irq] = false;
-        }
-
-        g_strfreev(words);
-        goto redo;
-    }
-
-    g_assert(words[0] != NULL);
-    g_assert_cmpstr(words[0], ==, "OK");
-
-    if (expected_args) {
-        for (i = 0; i < expected_args; i++) {
-            g_assert(words[i] != NULL);
-        }
-    } else {
-        g_strfreev(words);
-    }
-
-    return words;
-}
-
-static int qtest_query_target_endianness(QTestState *s)
-{
-    gchar **args;
-    int big_endian;
-
-    qtest_sendf(s, "endianness\n");
-    args = qtest_rsp(s, 1);

Quoted 9 lines · Eric Blake

Put static functions prior to public ones, in part so that improvements to qtest_start() can benefit from the static helpers without needing forward references. Code motion, with no semantic change.

Signed-off-by Eric Blake <[email protected]>

---
 tests/libqtest.c | 263 +++++++++++++++++++++++++++----------------------------
 1 file changed, 131 insertions(+), 132 deletions(-)

That's a *lot* of code motion - just to get rid of one forward declaration. IMHO getting rid of just one forward declaration does not justify this code churn. But if we really, really want to get rid of the forward declaration here, it's maybe better to move the qtest_init_without_qmp_handshake() and qtest_init() function to the end of the file instead?

There was only one caller; it's easier to inline things.

Signed-off-by Eric Blake <[email protected]>

---
  tests/libqtest.c | 22 ++++++----------------
  1 file changed, 6 insertions(+), 16 deletions(-)

tests/libqtest.c +6 -16

index 5d16351e24..b6dd26e54a 100644
@@ -244,20 +244,6 @@ redo:
      return words;
  }
 
-static int qtest_query_target_endianness(QTestState *s)
-{
-    gchar **args;
-    int big_endian;
-
-    qtest_sendf(s, "endianness\n");
-    args = qtest_rsp(s, 1);
-    g_assert(strcmp(args[1], "big") == 0 || strcmp(args[1], "little") == 0);
-    big_endian = strcmp(args[1], "big") == 0;
-    g_strfreev(args);
-
-    return big_endian;
-}
-
  static void cleanup_sigabrt_handler(void)
  {
      sigaction(SIGABRT, &sigact_old, NULL);

@@ -288,6 +274,7 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
      gchar *qmp_socket_path;
      gchar *command;
      const char *qemu_binary;
+    gchar **args;
 
      qemu_binary = getenv("QTEST_QEMU_BINARY");
      if (!qemu_binary) {

@@ -351,8 +338,11 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
      }
 
      /* ask endianness of the target */
-
-    s->big_endian = qtest_query_target_endianness(s);
+    qtest_sendf(s, "endianness\n");
+    args = qtest_rsp(s, 1);
+    g_assert(strcmp(args[1], "big") == 0 || strcmp(args[1], "little") == 0);
+    s->big_endian = strcmp(args[1], "big") == 0;
+    g_strfreev(args);
 
      return s;
  }

There was only one caller; it's easier to inline things.

Signed-off-by Eric Blake <[email protected]>

---
 tests/libqtest.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

tests/libqtest.c +1 -14

index 5d16351e24..b6dd26e54a 100644
@@ -244,20 +244,6 @@ redo:
     return words;
 }
 
-static int qtest_query_target_endianness(QTestState *s)
-{
-    gchar **args;
-    int big_endian;
-
-    qtest_sendf(s, "endianness\n");
-    args = qtest_rsp(s, 1);
-    g_assert(strcmp(args[1], "big") == 0 || strcmp(args[1], "little") == 0);
-    big_endian = strcmp(args[1], "big") == 0;
-    g_strfreev(args);
-
-    return big_endian;
-}
-
 static void cleanup_sigabrt_handler(void)
 {
     sigaction(SIGABRT, &sigact_old, NULL);

@@ -288,6 +274,7 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
     gchar *qmp_socket_path;
     gchar *command;
     const char *qemu_binary;
+    gchar **args;
 
     qemu_binary = getenv("QTEST_QEMU_BINARY");
     if (!qemu_binary) {

@@ -351,8 +338,11 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
     }
 
     /* ask endianness of the target */

Quoted 7 lines · Eric Blake

There was only one caller; it's easier to inline things.

Signed-off-by Eric Blake <[email protected]>

---
 tests/libqtest.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

Ah, well, now that's another way of getting rid of the forward declaration in the previous patch - so may I suggest to drop the previous patch, and simply do the code inlining here instead?

When initializing a QPCIBus, track which QTestState the bus is associated with (so that a later patch can then explicitly use that test state for all communication on the bus, rather than blindly relying on global_qtest). Update the initialization functions to take another parameter, and update all callers to pass in state (for now, most callers get away with passing the current global_qtest as the current state, although this required fixing the order of initialization to ensure qtest_start() is called before qpci_init*() in rtl8139-test, and provided an opportunity to pass in the allocator in e1000e-test).

Signed-off-by Eric Blake <[email protected]>

---
  tests/libqos/ahci.h       | 2 +-
  tests/libqos/libqos.h     | 2 +-
  tests/libqos/pci-pc.h     | 2 +-
  tests/libqos/pci-spapr.h  | 2 +-
  tests/libqos/pci.h        | 1 +
  tests/ahci-test.c         | 2 +-
  tests/e1000e-test.c       | 6 +++---
  tests/i440fx-test.c       | 2 +-
  tests/ide-test.c          | 2 +-
  tests/libqos/ahci.c       | 4 ++--
  tests/libqos/libqos.c     | 4 ++--
  tests/libqos/pci-pc.c     | 5 ++++-
  tests/libqos/pci-spapr.c  | 6 +++++-
  tests/q35-test.c          | 4 ++--
  tests/rtl8139-test.c      | 5 +++--
  tests/tco-test.c          | 2 +-
  tests/usb-hcd-ehci-test.c | 2 +-
  tests/vhost-user-test.c   | 4 ++--
  18 files changed, 33 insertions(+), 24 deletions(-)

tests/libqos/ahci.h +1 -1

index 5f9627bb0f..715ca1e226 100644
@@ -571,7 +571,7 @@ void ahci_free(AHCIQState *ahci, uint64_t addr);
  void ahci_clean_mem(AHCIQState *ahci);
 
  /* Device management */
-QPCIDevice *get_ahci_device(uint32_t *fingerprint);
+QPCIDevice *get_ahci_device(QTestState *qts, uint32_t *fingerprint);
  void free_ahci_device(QPCIDevice *dev);
  void ahci_pci_enable(AHCIQState *ahci);
  void start_ahci_device(AHCIQState *ahci);

tests/libqos/libqos.h +1 -1

index 231969766f..78e5c044a0 100644
@@ -10,7 +10,7 @@ typedef struct QOSState QOSState;
  typedef struct QOSOps {
      QGuestAllocator *(*init_allocator)(QAllocOpts);
      void (*uninit_allocator)(QGuestAllocator *);
-    QPCIBus *(*qpci_init)(QGuestAllocator *alloc);
+    QPCIBus *(*qpci_init)(QTestState *qts, QGuestAllocator *alloc);
      void (*qpci_free)(QPCIBus *bus);
      void (*shutdown)(QOSState *);
  } QOSOps;

tests/libqos/pci-pc.h +1 -1

index 9479b51642..491eeac756 100644
@@ -16,7 +16,7 @@
  #include "libqos/pci.h"
  #include "libqos/malloc.h"
 
-QPCIBus *qpci_init_pc(QGuestAllocator *alloc);
+QPCIBus *qpci_init_pc(QTestState *qts, QGuestAllocator *alloc);
  void     qpci_free_pc(QPCIBus *bus);
 
  #endif

tests/libqos/pci-spapr.h +1 -1

index 4192126d86..387686dfc8 100644
@@ -11,7 +11,7 @@
  #include "libqos/malloc.h"
  #include "libqos/pci.h"
 
-QPCIBus *qpci_init_spapr(QGuestAllocator *alloc);
+QPCIBus *qpci_init_spapr(QTestState *qts, QGuestAllocator *alloc);
  void     qpci_free_spapr(QPCIBus *bus);
 
  #endif

tests/libqos/pci.h +1 -0

index ed480614ff..429c382282 100644
@@ -48,6 +48,7 @@ struct QPCIBus {
      void (*config_writel)(QPCIBus *bus, int devfn,
                            uint8_t offset, uint32_t value);
 
+    QTestState *qts;
      uint16_t pio_alloc_ptr;
      uint64_t mmio_alloc_ptr, mmio_limit;
  };

tests/ahci-test.c +1 -1

index 999121bb7c..c94d1bd712 100644
@@ -160,7 +160,7 @@ static AHCIQState *ahci_vboot(const char *cli, va_list ap)
      alloc_set_flags(s->parent->alloc, ALLOC_LEAK_ASSERT);
 
      /* Verify that we have an AHCI device present. */
-    s->dev = get_ahci_device(&s->fingerprint);
+    s->dev = get_ahci_device(s->parent->qts, &s->fingerprint);
 
      return s;
  }

tests/e1000e-test.c +3 -3

index c612dc64ec..d8085d944e 100644
@@ -392,12 +392,12 @@ static void data_test_init(e1000e_device *d)
      qtest_start(cmdline);
      g_free(cmdline);
 
-    test_bus = qpci_init_pc(NULL);
-    g_assert_nonnull(test_bus);
-
      test_alloc = pc_alloc_init();
      g_assert_nonnull(test_alloc);
 
+    test_bus = qpci_init_pc(global_qtest, test_alloc);
+    g_assert_nonnull(test_bus);
+
      e1000e_device_init(test_bus, d);
  }
 

tests/i440fx-test.c +1 -1

index e9d05c87d1..4390e5591e 100644
@@ -38,7 +38,7 @@ static QPCIBus *test_start_get_bus(const TestData *s)
      cmdline = g_strdup_printf("-smp %d", s->num_cpus);
      qtest_start(cmdline);
      g_free(cmdline);
-    return qpci_init_pc(NULL);
+    return qpci_init_pc(global_qtest, NULL);
  }
 
  static void test_i440fx_defaults(gconstpointer opaque)

tests/ide-test.c +1 -1

index aa9de065fc..b2237b6158 100644
@@ -143,7 +143,7 @@ static QPCIDevice *get_pci_device(QPCIBar *bmdma_bar, QPCIBar *ide_bar)
      uint16_t vendor_id, device_id;
 
      if (!pcibus) {
-        pcibus = qpci_init_pc(NULL);
+        pcibus = qpci_init_pc(global_qtest, NULL);
      }
 
      /* Find PCI device and verify it's the right one */

tests/libqos/ahci.c +2 -2

index 1ca7f456b5..790ef991b3 100644
@@ -123,13 +123,13 @@ bool is_atapi(AHCIQState *ahci, uint8_t port)
  /**
   * Locate, verify, and return a handle to the AHCI device.
   */
-QPCIDevice *get_ahci_device(uint32_t *fingerprint)
+QPCIDevice *get_ahci_device(QTestState *qts, uint32_t *fingerprint)
  {
      QPCIDevice *ahci;
      uint32_t ahci_fingerprint;
      QPCIBus *pcibus;
 
-    pcibus = qpci_init_pc(NULL);
+    pcibus = qpci_init_pc(qts, NULL);
 
      /* Find the AHCI PCI device and verify it's the right one. */
      ahci = qpci_device_find(pcibus, QPCI_DEVFN(0x1F, 0x02));

tests/libqos/libqos.c +2 -2

index 6226546c28..c95428e1cb 100644
@@ -26,8 +26,8 @@ QOSState *qtest_vboot(QOSOps *ops, const char *cmdline_fmt, va_list ap)
          if (ops->init_allocator) {
              qs->alloc = ops->init_allocator(ALLOC_NO_FLAGS);
          }
-        if (ops->qpci_init && qs->alloc) {
-            qs->pcibus = ops->qpci_init(qs->alloc);
+        if (ops->qpci_init) {
+            qs->pcibus = ops->qpci_init(qs->qts, qs->alloc);
          }
      }
 

tests/libqos/pci-pc.c +0 -0

index 02ce49927a..85b34c6d13 100644
@@ -115,11 +115,14 @@ static void qpci_pc_config_writel(QPCIBus *bus, int devfn, uint8_t offset, uint3
      outl(0xcfc, value);

tests/libqos/pci-spapr.c +0 -0

index 2043f1e123..cd9b8f52d2 100644
@@ -154,11 +154,14 @@ static void qpci_spapr_config_writel(QPCIBus *bus, int devfn, uint8_t offset,
  #define SPAPR_PCI_MMIO32_WIN_SIZE    0x80000000 /* 2 GiB */
  #define SPAPR_PCI_IO_WIN_SIZE        0x10000

tests/libqos/pci-spapr.c +1 -0

      ret->alloc = alloc;
 

@@ -201,6 +204,7 @@ QPCIBus *qpci_init_spapr(QGuestAllocator *alloc)
      ret->bus.mmio_alloc_ptr = ret->mmio32.pci_base;
      ret->bus.mmio_limit = ret->mmio32.pci_base + ret->mmio32.size;
 
+
      return &ret->bus;
  }
 

tests/q35-test.c +2 -2

index f98bed7a2d..e149c4c51d 100644
@@ -86,7 +86,7 @@ static void test_smram_lock(void)
 
      qtest_start("-M q35");
 
-    pcibus = qpci_init_pc(NULL);
+    pcibus = qpci_init_pc(global_qtest, NULL);
      g_assert(pcibus != NULL);
 
      pcidev = qpci_device_find(pcibus, 0);

@@ -145,7 +145,7 @@ static void test_tseg_size(const void *data)
      g_free(cmdline);
 
      /* locate the DRAM controller */
-    pcibus = qpci_init_pc(NULL);
+    pcibus = qpci_init_pc(global_qtest, NULL);
      g_assert(pcibus != NULL);
      pcidev = qpci_device_find(pcibus, 0);
      g_assert(pcidev != NULL);

tests/rtl8139-test.c +3 -2

index 7de7dc45ae..68bfc42178 100644
@@ -35,7 +35,7 @@ static QPCIDevice *get_device(void)
  {
      QPCIDevice *dev;
 
-    pcibus = qpci_init_pc(NULL);
+    pcibus = qpci_init_pc(global_qtest, NULL);
      qpci_device_foreach(pcibus, 0x10ec, 0x8139, save_fn, &dev);
      g_assert(dev != NULL);
 

@@ -197,11 +197,12 @@ int main(int argc, char **argv)
  {
      int ret;
 
+    qtest_start("-device rtl8139");
+
      g_test_init(&argc, &argv, NULL);
      qtest_add_func("/rtl8139/nop", nop);
      qtest_add_func("/rtl8139/timer", test_init);
 
-    qtest_start("-device rtl8139");
      ret = g_test_run();
 
      qtest_end();

tests/tco-test.c +1 -1

index f2ed6ed91c..0387971953 100644
@@ -64,7 +64,7 @@ static void test_init(TestData *d)
      qtest_irq_intercept_in(qs, "ioapic");
      g_free(s);
 
-    d->bus = qpci_init_pc(NULL);
+    d->bus = qpci_init_pc(qs, NULL);
      d->dev = qpci_device_find(d->bus, QPCI_DEVFN(0x1f, 0x00));
      g_assert(d->dev != NULL);
 

tests/usb-hcd-ehci-test.c +1 -1

index 944eb1c088..55d4743a2a 100644
@@ -52,7 +52,7 @@ static void ehci_port_test(struct qhc *hc, int port, uint32_t expect)
 
  static void test_init(void)
  {
-    pcibus = qpci_init_pc(NULL);
+    pcibus = qpci_init_pc(global_qtest, NULL);
      g_assert(pcibus != NULL);
 
      qusb_pci_init_one(pcibus, &uhci1, QPCI_DEVFN(0x1d, 0), 4);

tests/vhost-user-test.c +2 -2

index d4da09f147..ea7d38ea44 100644
@@ -164,7 +164,7 @@ static void init_virtio_dev(TestServer *s)
      QVirtioPCIDevice *dev;
      uint32_t features;
 
-    s->bus = qpci_init_pc(NULL);
+    s->bus = qpci_init_pc(global_qtest, NULL);
      g_assert_nonnull(s->bus);
 
      dev = qvirtio_pci_device_find(s->bus, VIRTIO_ID_NET);

@@ -891,7 +891,7 @@ static void test_multiqueue(void)
      qtest_start(cmd);
      g_free(cmd);
 
-    bus = qpci_init_pc(NULL);
+    bus = qpci_init_pc(global_qtest, NULL);
      dev = virtio_net_pci_init(bus, PCI_SLOT);
 
      alloc = pc_alloc_init();

Hi Eric,

Quoted 23 lines · Eric Blake

When initializing a QPCIBus, track which QTestState the bus is associated with (so that a later patch can then explicitly use that test state for all communication on the bus, rather than blindly relying on global_qtest).  Update the initialization functions to take another parameter, and update all callers to pass in state (for now, most callers get away with passing the current global_qtest as the current state, although this required fixing the order of initialization to ensure qtest_start() is called before qpci_init*() in rtl8139-test, and provided an opportunity to pass in the allocator in e1000e-test).

+++ b/tests/libqos/pci-pc.c

(quoted diff) +0 -0

@@ -115,11 +115,14 @@ static void qpci_pc_config_writel(QPCIBus *bus,

int devfn, uint8_t offset, uint3
      outl(0xcfc, value);
  }

tests/libqos/pci-pc.c +1 -1

-QPCIBus *qpci_init_pc(QGuestAllocator *alloc)
+QPCIBus *qpci_init_pc(QTestState *qts, QGuestAllocator *alloc)

  {
      QPCIBusPC *ret;

Quoted 3 lines

+    assert(qts);
+
      ret = g_malloc(sizeof(*ret));

I'd rather use g_malloc0() here (safer!)

Quoted 5 lines

+    ret->bus.qts = qts;

      ret->bus.pio_readb = qpci_pc_pio_readb;
      ret->bus.pio_readw = qpci_pc_pio_readw;

or init qts field in same order than struct:

Either ways:

Reviewed-by Philippe Mathieu-Daudé <[email protected]>

When initializing a QPCIBus, track which QTestState the bus is associated with (so that a later patch can then explicitly use that test state for all communication on the bus, rather than blindly relying on global_qtest). Update the initialization functions to take another parameter, and update all callers to pass in state (for now, most callers get away with passing the current global_qtest as the current state, although this required fixing the order of initialization to ensure qtest_start() is called before qpci_init*() in rtl8139-test, and provided an opportunity to pass in the allocator in e1000e-test).

Signed-off-by Eric Blake <[email protected]>

When initializing a QPCIBus, track which QTestState the bus is associated with (so that a later patch can then explicitly use that test state for all communication on the bus, rather than blindly relying on global_qtest). Update the initialization functions to take another parameter, and update all callers to pass in state (for now, most callers get away with passing the current global_qtest as the current state, although this required fixing the order of initialization to ensure qtest_start() is called before qpci_init*() in rtl8139-test, and provided an opportunity to pass in the allocator in e1000e-test).

Signed-off-by Eric Blake <[email protected]>

---

tests/libqos/libqos.c +1 -0

+            qs->pcibus = ops->qpci_init(qs->qts, qs->alloc);
         }
     }
 

tests/libqos/pci-pc.c +4 -1

index 02ce49927a..85b34c6d13 100644
@@ -115,11 +115,14 @@ static void qpci_pc_config_writel(QPCIBus *bus, int devfn, uint8_t offset, uint3
     outl(0xcfc, value);
 }
 
-QPCIBus *qpci_init_pc(QGuestAllocator *alloc)
+QPCIBus *qpci_init_pc(QTestState *qts, QGuestAllocator *alloc)
 {
     QPCIBusPC *ret;
 
+    assert(qts);
+
     ret = g_malloc(sizeof(*ret));
+    ret->bus.qts = qts;
 
     ret->bus.pio_readb = qpci_pc_pio_readb;
     ret->bus.pio_readw = qpci_pc_pio_readw;

tests/libqos/pci-spapr.c +0 -0

index 2043f1e123..cd9b8f52d2 100644
@@ -154,11 +154,14 @@ static void qpci_spapr_config_writel(QPCIBus *bus, int devfn, uint8_t offset,
 #define SPAPR_PCI_MMIO32_WIN_SIZE    0x80000000 /* 2 GiB */

Quoted 13 lines · Eric Blake

When initializing a QPCIBus, track which QTestState the bus is associated with (so that a later patch can then explicitly use that test state for all communication on the bus, rather than blindly relying on global_qtest). Update the initialization functions to take another parameter, and update all callers to pass in state (for now, most callers get away with passing the current global_qtest as the current state, although this required fixing the order of initialization to ensure qtest_start() is called before qpci_init*() in rtl8139-test, and provided an opportunity to pass in the allocator in e1000e-test).

Signed-off-by Eric Blake <[email protected]>

---

[...]

tests/libqos/libqos.c QOSState *qtest_vboot(QOSOps *ops, const char *cmdline_fmt, va_list ap) · line 29

Quoted 11 lines

tests/libqos/libqos.c +1 -2

index 6226546c28..c95428e1cb 100644
@@ -26,8 +26,8 @@ QOSState *qtest_vboot(QOSOps *ops, const char *cmdline_fmt, va_list ap)
         if (ops->init_allocator) {
             qs->alloc = ops->init_allocator(ALLOC_NO_FLAGS);
         }
-        if (ops->qpci_init && qs->alloc) {
-            qs->pcibus = ops->qpci_init(qs->alloc);
+        if (ops->qpci_init) {

Why did you remove the check for qs->alloc?

Quoted 1 line

tests/libqos/libqos.c +1 -0

+            qs->pcibus = ops->qpci_init(qs->qts, qs->alloc);

Quoted 15 lines · Thomas Huth

Quoted 13 lines · Eric Blake

When initializing a QPCIBus, track which QTestState the bus is associated with (so that a later patch can then explicitly use that test state for all communication on the bus, rather than blindly relying on global_qtest). Update the initialization functions to take another parameter, and update all callers to pass in state (for now, most callers get away with passing the current global_qtest as the current state, although this required fixing the order of initialization to ensure qtest_start() is called before qpci_init*() in rtl8139-test, and provided an opportunity to pass in the allocator in e1000e-test).

Signed-off-by Eric Blake <[email protected]>

---

[...]

Quoted 15 lines

tests/libqos/libqos.c QOSState *qtest_vboot(QOSOps *ops, const char *cmdline_fmt, va_list ap) · line 29

Quoted 11 lines

tests/libqos/libqos.c +1 -2

index 6226546c28..c95428e1cb 100644
@@ -26,8 +26,8 @@ QOSState *qtest_vboot(QOSOps *ops, const char *cmdline_fmt, va_list ap)
         if (ops->init_allocator) {
             qs->alloc = ops->init_allocator(ALLOC_NO_FLAGS);
         }
-        if (ops->qpci_init && qs->alloc) {
-            qs->pcibus = ops->qpci_init(qs->alloc);
+        if (ops->qpci_init) {

Why did you remove the check for qs->alloc?

Quoted 1 line

tests/libqos/libqos.c +1 -0

+            qs->pcibus = ops->qpci_init(qs->qts, qs->alloc);

Because we want to ensure qpci_init() is called to set qs->qts (presumably, whether or not qs->alloc is set). Furthermore, only two files declare a 'static QOSOps' structure in the first place (libqos-pc.c and libqos-spapr.c); where both files set both the .init_allocator and .qpci_init callbacks; a little bit of auditing shows that the .init_allocator() never returns NULL (although that requires browsing yet more files for malloc-{pc,spapr}.c).

Quoted 19 lines · Eric Blake

Quoted 14 lines · Thomas Huth

Quoted 11 lines · Eric Blake

When initializing a QPCIBus, track which QTestState the bus is associated with (so that a later patch can then explicitly use that test state for all communication on the bus, rather than blindly relying on global_qtest). Update the initialization functions to take another parameter, and update all callers to pass in state (for now, most callers get away with passing the current global_qtest as the current state, although this required fixing the order of initialization to ensure qtest_start() is called before qpci_init*() in rtl8139-test, and provided an opportunity to pass in the allocator in e1000e-test).

Quoted 3 lines

Why did you remove the check for qs->alloc?

Quoted 1 line

+ qs->pcibus = ops->qpci_init(qs->qts, qs->alloc);

Quoted 7 lines

Because we want to ensure qpci_init() is called to set qs->qts (presumably, whether or not qs->alloc is set). Furthermore, only two files declare a 'static QOSOps' structure in the first place (libqos-pc.c and libqos-spapr.c); where both files set both the .init_allocator and .qpci_init callbacks; a little bit of auditing shows that the .init_allocator() never returns NULL (although that requires browsing yet more files for malloc-{pc,spapr}.c).

OK, thanks for the explanation! ... but maybe we should g_assert(gs->alloc) somewhere instead? ... just an idea, I'm also fine if you leave it away.

Now that the QPCIBus tracks QTestState, use that state instead of an implicit reliance on global_qtest.

Signed-off-by Eric Blake <[email protected]>

---
  tests/libqos/pci-pc.c    | 47 ++++++++++++++++++++++++-----------------------
  tests/libqos/pci-spapr.c | 20 ++++++++++----------
  2 files changed, 34 insertions(+), 33 deletions(-)

tests/libqos/pci-pc.c +24 -23

index 85b34c6d13..9545f881ee 100644
@@ -29,90 +29,91 @@ typedef struct QPCIBusPC
 
  static uint8_t qpci_pc_pio_readb(QPCIBus *bus, uint32_t addr)
  {
-    return inb(addr);
+    return qtest_inb(bus->qts, addr);
  }
 
  static void qpci_pc_pio_writeb(QPCIBus *bus, uint32_t addr, uint8_t val)
  {
-    outb(addr, val);
+    qtest_outb(bus->qts, addr, val);
  }
 
  static uint16_t qpci_pc_pio_readw(QPCIBus *bus, uint32_t addr)
  {
-    return inw(addr);
+    return qtest_inw(bus->qts, addr);
  }
 
  static void qpci_pc_pio_writew(QPCIBus *bus, uint32_t addr, uint16_t val)
  {
-    outw(addr, val);
+    qtest_outw(bus->qts, addr, val);
  }
 
  static uint32_t qpci_pc_pio_readl(QPCIBus *bus, uint32_t addr)
  {
-    return inl(addr);
+    return qtest_inl(bus->qts, addr);
  }
 
  static void qpci_pc_pio_writel(QPCIBus *bus, uint32_t addr, uint32_t val)
  {
-    outl(addr, val);
+    qtest_outl(bus->qts, addr, val);
  }
 
  static uint64_t qpci_pc_pio_readq(QPCIBus *bus, uint32_t addr)
  {
-    return (uint64_t)inl(addr) + ((uint64_t)inl(addr + 4) << 32);
+    return (uint64_t)qtest_inl(bus->qts, addr) +
+        ((uint64_t)qtest_inl(bus->qts, addr + 4) << 32);
  }
 
  static void qpci_pc_pio_writeq(QPCIBus *bus, uint32_t addr, uint64_t val)
  {
-    outl(addr, val & 0xffffffff);
-    outl(addr + 4, val >> 32);
+    qtest_outl(bus->qts, addr, val & 0xffffffff);
+    qtest_outl(bus->qts, addr + 4, val >> 32);
  }
 
  static void qpci_pc_memread(QPCIBus *bus, uint32_t addr, void *buf, size_t len)
  {
-    memread(addr, buf, len);
+    qtest_memread(bus->qts, addr, buf, len);
  }
 
  static void qpci_pc_memwrite(QPCIBus *bus, uint32_t addr,
                               const void *buf, size_t len)
  {
-    memwrite(addr, buf, len);
+    qtest_memwrite(bus->qts, addr, buf, len);
  }
 
  static uint8_t qpci_pc_config_readb(QPCIBus *bus, int devfn, uint8_t offset)
  {
-    outl(0xcf8, (1U << 31) | (devfn << 8) | offset);
-    return inb(0xcfc);
+    qtest_outl(bus->qts, 0xcf8, (1U << 31) | (devfn << 8) | offset);
+    return qtest_inb(bus->qts, 0xcfc);
  }
 
  static uint16_t qpci_pc_config_readw(QPCIBus *bus, int devfn, uint8_t offset)
  {
-    outl(0xcf8, (1U << 31) | (devfn << 8) | offset);
-    return inw(0xcfc);
+    qtest_outl(bus->qts, 0xcf8, (1U << 31) | (devfn << 8) | offset);
+    return qtest_inw(bus->qts, 0xcfc);
  }
 
  static uint32_t qpci_pc_config_readl(QPCIBus *bus, int devfn, uint8_t offset)
  {
-    outl(0xcf8, (1U << 31) | (devfn << 8) | offset);
-    return inl(0xcfc);
+    qtest_outl(bus->qts, 0xcf8, (1U << 31) | (devfn << 8) | offset);
+    return qtest_inl(bus->qts, 0xcfc);
  }
 
  static void qpci_pc_config_writeb(QPCIBus *bus, int devfn, uint8_t offset, uint8_t value)
  {
-    outl(0xcf8, (1U << 31) | (devfn << 8) | offset);
-    outb(0xcfc, value);
+    qtest_outl(bus->qts, 0xcf8, (1U << 31) | (devfn << 8) | offset);
+    qtest_outb(bus->qts, 0xcfc, value);
  }
 
  static void qpci_pc_config_writew(QPCIBus *bus, int devfn, uint8_t offset, uint16_t value)
  {
-    outl(0xcf8, (1U << 31) | (devfn << 8) | offset);
-    outw(0xcfc, value);
+    qtest_outl(bus->qts, 0xcf8, (1U << 31) | (devfn << 8) | offset);
+    qtest_outw(bus->qts, 0xcfc, value);
  }
 
  static void qpci_pc_config_writel(QPCIBus *bus, int devfn, uint8_t offset, uint32_t value)
  {
-    outl(0xcf8, (1U << 31) | (devfn << 8) | offset);
-    outl(0xcfc, value);
+    qtest_outl(bus->qts, 0xcf8, (1U << 31) | (devfn << 8) | offset);
+    qtest_outl(bus->qts, 0xcfc, value);
  }
 
  QPCIBus *qpci_init_pc(QTestState *qts, QGuestAllocator *alloc)

tests/libqos/pci-spapr.c +10 -10

index cd9b8f52d2..d7b2aa75e6 100644
@@ -45,63 +45,63 @@ typedef struct QPCIBusSPAPR {
  static uint8_t qpci_spapr_pio_readb(QPCIBus *bus, uint32_t addr)
  {
      QPCIBusSPAPR *s = container_of(bus, QPCIBusSPAPR, bus);
-    return readb(s->pio_cpu_base + addr);
+    return qtest_readb(bus->qts, s->pio_cpu_base + addr);
  }
 
  static void qpci_spapr_pio_writeb(QPCIBus *bus, uint32_t addr, uint8_t val)
  {
      QPCIBusSPAPR *s = container_of(bus, QPCIBusSPAPR, bus);
-    writeb(s->pio_cpu_base + addr, val);
+    qtest_writeb(bus->qts, s->pio_cpu_base + addr, val);
  }
 
  static uint16_t qpci_spapr_pio_readw(QPCIBus *bus, uint32_t addr)
  {
      QPCIBusSPAPR *s = container_of(bus, QPCIBusSPAPR, bus);
-    return bswap16(readw(s->pio_cpu_base + addr));
+    return bswap16(qtest_readw(bus->qts, s->pio_cpu_base + addr));
  }
 
  static void qpci_spapr_pio_writew(QPCIBus *bus, uint32_t addr, uint16_t val)
  {
      QPCIBusSPAPR *s = container_of(bus, QPCIBusSPAPR, bus);
-    writew(s->pio_cpu_base + addr, bswap16(val));
+    qtest_writew(bus->qts, s->pio_cpu_base + addr, bswap16(val));
  }
 
  static uint32_t qpci_spapr_pio_readl(QPCIBus *bus, uint32_t addr)
  {
      QPCIBusSPAPR *s = container_of(bus, QPCIBusSPAPR, bus);
-    return bswap32(readl(s->pio_cpu_base + addr));
+    return bswap32(qtest_readl(bus->qts, s->pio_cpu_base + addr));
  }
 
  static void qpci_spapr_pio_writel(QPCIBus *bus, uint32_t addr, uint32_t val)
  {
      QPCIBusSPAPR *s = container_of(bus, QPCIBusSPAPR, bus);
-    writel(s->pio_cpu_base + addr, bswap32(val));
+    qtest_writel(bus->qts, s->pio_cpu_base + addr, bswap32(val));
  }
 
  static uint64_t qpci_spapr_pio_readq(QPCIBus *bus, uint32_t addr)
  {
      QPCIBusSPAPR *s = container_of(bus, QPCIBusSPAPR, bus);
-    return bswap64(readq(s->pio_cpu_base + addr));
+    return bswap64(qtest_readq(bus->qts, s->pio_cpu_base + addr));
  }
 
  static void qpci_spapr_pio_writeq(QPCIBus *bus, uint32_t addr, uint64_t val)
  {
      QPCIBusSPAPR *s = container_of(bus, QPCIBusSPAPR, bus);
-    writeq(s->pio_cpu_base + addr, bswap64(val));
+    qtest_writeq(bus->qts, s->pio_cpu_base + addr, bswap64(val));
  }
 
  static void qpci_spapr_memread(QPCIBus *bus, uint32_t addr,
                                 void *buf, size_t len)
  {
      QPCIBusSPAPR *s = container_of(bus, QPCIBusSPAPR, bus);
-    memread(s->mmio32_cpu_base + addr, buf, len);
+    qtest_memread(bus->qts, s->mmio32_cpu_base + addr, buf, len);
  }
 
  static void qpci_spapr_memwrite(QPCIBus *bus, uint32_t addr,
                                  const void *buf, size_t len)
  {
      QPCIBusSPAPR *s = container_of(bus, QPCIBusSPAPR, bus);
-    memwrite(s->mmio32_cpu_base + addr, buf, len);
+    qtest_memwrite(bus->qts, s->mmio32_cpu_base + addr, buf, len);
  }
 
  static uint8_t qpci_spapr_config_readb(QPCIBus *bus, int devfn, uint8_t offset)