[PATCH] tests/qtest: Make qtest_get_arch() cleverer

Peter Maydell posted 1 patch 3 weeks, 6 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260427150007.1185559-1-peter.maydell@linaro.org
Maintainers: Fabiano Rosas <farosas@suse.de>, Laurent Vivier <lvivier@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
tests/qtest/libqtest.c | 43 +++++++++++++++++++++++++++++-------------
1 file changed, 30 insertions(+), 13 deletions(-)
[PATCH] tests/qtest: Make qtest_get_arch() cleverer
Posted by Peter Maydell 3 weeks, 6 days ago
The qtest_get_arch() function tries to determine the architecture
under test by extracting it from the binary name as provided in
QTEST_QEMU_BINARY.  The current logic finds the last '-' in the
string and assumes everything beyond it is the architecture name.
Although we also look for the substring "-system-", the only effect
this check has is that we will exit with an error if it is not
present.

Because the logic at the moment is very simplistic, although
it is possible to provide more complex commands than a bare
QEMU binary path, such as:
  QTEST_QEMU_BINARY='rr record ./qemu-system-x86_64'
it is not possible to provide extra arguments to QEMU, such as:
  QTEST_QEMU_BINARY='./qemu-system-x86_64 -d trace:foo'

Because the "-system-" check and the "find the architecture" check
are not the same, the latter example will pass the "we found
-system-" check and not notice that the "architecture name" it has
found starts further on in the string; so rather than printing an
error it will return "d trace:foo" to the test.

Improve the "find the architecture name" logic to look for the
rightmost occurrence of the substring "-system-" in
QTEST_QEMU_BINARY, and take the architecture name as starting there
and continuing until the first whitespace character or the end of the
string.

Because we now need to potentially modify the environment variable
string to terminate the architecture name if it is not the last part
of the string, we make a copy of it which we cache in a static
variable.  This lets us avoid having to modify all the callers to get
them to take ownership of the returned string.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
I wanted to be able to pass trace arguments to a QEMU to help
debug a failing test; in the past I've done this by writing
a little wrapper shell script with the right shaped name, but
we can do better than that.

 tests/qtest/libqtest.c | 43 +++++++++++++++++++++++++++++-------------
 1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 051faf31e1..116a8a3258 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -1015,22 +1015,39 @@ char *qtest_hmp(QTestState *s, const char *fmt, ...)
 
 const char *qtest_get_arch(void)
 {
-    const char *qemu = qtest_qemu_binary(NULL);
-    const char *end = strrchr(qemu, '-');
+    /*
+     * We find and cache the architecture name once, because we need to
+     * allocate memory to hold it. This memory will stay around for
+     * the lifetime of this test process.
+     */
+    static const char *arch;
 
-    if (!end) {
-        fprintf(stderr, "Can't determine architecture from binary name.\n");
-        exit(1);
+    if (!arch) {
+        /*
+         * Find the rightmost occurrence of "-system-"; the architecture
+         * name runs from there to the next whitespace.
+         */
+        const char *qemu = qtest_qemu_binary(NULL);
+        const char *sysstr = g_strrstr(qemu, "-system-");
+
+        if (sysstr) {
+            g_auto(GStrv) tokens = g_strsplit_set(sysstr + strlen("-system-"),
+                                                  " \t", 2);
+            if (tokens && tokens[0]) {
+                arch = g_steal_pointer(&tokens[0]);
+            }
+        }
+
+        if (!arch) {
+            fprintf(stderr, "Can't determine architecture from binary name.\n"
+                    "QTEST_QEMU_BINARY must include *-system-<arch> where "
+                    "'arch' is the target architecture "
+                    "(x86_64, aarch64, etc).\n");
+            exit(1);
+        }
     }
 
-    if (!strstr(qemu, "-system-")) {
-        fprintf(stderr, "QTEST_QEMU_BINARY must end with *-system-<arch> "
-                "where 'arch' is the target\narchitecture (x86_64, aarch64, "
-                "etc).\n");
-        exit(1);
-    }
-
-    return end + 1;
+    return arch;
 }
 
 static bool qtest_qom_has_concrete_type(const char *parent_typename,
-- 
2.43.0
Re: [PATCH] tests/qtest: Make qtest_get_arch() cleverer
Posted by Fabiano Rosas 3 weeks, 6 days ago
Peter Maydell <peter.maydell@linaro.org> writes:

> The qtest_get_arch() function tries to determine the architecture
> under test by extracting it from the binary name as provided in
> QTEST_QEMU_BINARY.  The current logic finds the last '-' in the
> string and assumes everything beyond it is the architecture name.
> Although we also look for the substring "-system-", the only effect
> this check has is that we will exit with an error if it is not
> present.
>
> Because the logic at the moment is very simplistic, although
> it is possible to provide more complex commands than a bare
> QEMU binary path, such as:
>   QTEST_QEMU_BINARY='rr record ./qemu-system-x86_64'
> it is not possible to provide extra arguments to QEMU, such as:
>   QTEST_QEMU_BINARY='./qemu-system-x86_64 -d trace:foo'
>
> Because the "-system-" check and the "find the architecture" check
> are not the same, the latter example will pass the "we found
> -system-" check and not notice that the "architecture name" it has
> found starts further on in the string; so rather than printing an
> error it will return "d trace:foo" to the test.
>
> Improve the "find the architecture name" logic to look for the
> rightmost occurrence of the substring "-system-" in
> QTEST_QEMU_BINARY, and take the architecture name as starting there
> and continuing until the first whitespace character or the end of the
> string.
>
> Because we now need to potentially modify the environment variable
> string to terminate the architecture name if it is not the last part
> of the string, we make a copy of it which we cache in a static
> variable.  This lets us avoid having to modify all the callers to get
> them to take ownership of the returned string.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I wanted to be able to pass trace arguments to a QEMU to help
> debug a failing test; in the past I've done this by writing
> a little wrapper shell script with the right shaped name, but
> we can do better than that.
>

There's QTEST_TRACE which can inject any command line option if you give
it a tracepoint name as first string:

QTEST_TRACE="cpu_reset -d guest_errors"

Maybe we should just rename it to QTEST_QEMU_ARGS.
Re: [PATCH] tests/qtest: Make qtest_get_arch() cleverer
Posted by Peter Maydell 3 weeks, 6 days ago
On Mon, 27 Apr 2026 at 16:40, Fabiano Rosas <farosas@suse.de> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > The qtest_get_arch() function tries to determine the architecture
> > under test by extracting it from the binary name as provided in
> > QTEST_QEMU_BINARY.  The current logic finds the last '-' in the
> > string and assumes everything beyond it is the architecture name.
> > Although we also look for the substring "-system-", the only effect
> > this check has is that we will exit with an error if it is not
> > present.
> >
> > Because the logic at the moment is very simplistic, although
> > it is possible to provide more complex commands than a bare
> > QEMU binary path, such as:
> >   QTEST_QEMU_BINARY='rr record ./qemu-system-x86_64'
> > it is not possible to provide extra arguments to QEMU, such as:
> >   QTEST_QEMU_BINARY='./qemu-system-x86_64 -d trace:foo'
> >
> > Because the "-system-" check and the "find the architecture" check
> > are not the same, the latter example will pass the "we found
> > -system-" check and not notice that the "architecture name" it has
> > found starts further on in the string; so rather than printing an
> > error it will return "d trace:foo" to the test.
> >
> > Improve the "find the architecture name" logic to look for the
> > rightmost occurrence of the substring "-system-" in
> > QTEST_QEMU_BINARY, and take the architecture name as starting there
> > and continuing until the first whitespace character or the end of the
> > string.
> >
> > Because we now need to potentially modify the environment variable
> > string to terminate the architecture name if it is not the last part
> > of the string, we make a copy of it which we cache in a static
> > variable.  This lets us avoid having to modify all the callers to get
> > them to take ownership of the returned string.
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> > I wanted to be able to pass trace arguments to a QEMU to help
> > debug a failing test; in the past I've done this by writing
> > a little wrapper shell script with the right shaped name, but
> > we can do better than that.
> >
>
> There's QTEST_TRACE which can inject any command line option if you give
> it a tracepoint name as first string:
>
> QTEST_TRACE="cpu_reset -d guest_errors"
>
> Maybe we should just rename it to QTEST_QEMU_ARGS.

That might also be useful. It would also be good to document
all these environment variables that you can use to help
in debugging qtests...

-- PMM