1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH] tests/libqtest: Use a proper error message if QTEST_QEMU_BINARY is missing

Thomas Huth posted 1 patch 6 years, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1503916557-1292-1-git-send-email-thuth@redhat.com
Test FreeBSD passed
Test checkpatch passed
Test docker passed
Test s390x passed
tests/libqtest.c | 24 +++++++++++++++---------
1 file changed, 15 insertions(+), 9 deletions(-)
[Qemu-devel] [PATCH] tests/libqtest: Use a proper error message if QTEST_QEMU_BINARY is missing
Posted by Thomas Huth 6 years, 8 months ago 
The user can currently still cause an abort() if running certain tests
(like the prom-env-test) without setting the QTEST_QEMU_BINARY first.
A similar problem has been fixed with commit 7c933ad61b8f3f51337
already, but forgot to also take care of the qtest_get_arch() function,
so let's introduce a proper wrapper around getenv("QTEST_QEMU_BINARY")
that can be used in both locations now.

Buglink: https://bugs.launchpad.net/qemu/+bug/1713434
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/libqtest.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index b9a1f18..342a77b 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -150,6 +150,19 @@ void qtest_add_abrt_handler(GHookFunc fn, const void *data)
     g_hook_prepend(&abrt_hooks, hook);
 }
 
+static const char *qtest_qemu_binary(void)
+{
+    const char *qemu_bin;
+
+    qemu_bin = getenv("QTEST_QEMU_BINARY");
+    if (!qemu_bin) {
+        fprintf(stderr, "Environment variable QTEST_QEMU_BINARY required\n");
+        exit(1);
+    }
+
+    return qemu_bin;
+}
+
 QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
 {
     QTestState *s;
@@ -157,13 +170,7 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
     gchar *socket_path;
     gchar *qmp_socket_path;
     gchar *command;
-    const char *qemu_binary;
-
-    qemu_binary = getenv("QTEST_QEMU_BINARY");
-    if (!qemu_binary) {
-        fprintf(stderr, "Environment variable QTEST_QEMU_BINARY required\n");
-        exit(1);
-    }
+    const char *qemu_binary = qtest_qemu_binary();
 
     s = g_malloc(sizeof(*s));
 
@@ -624,8 +631,7 @@ char *qtest_hmp(QTestState *s, const char *fmt, ...)
 
 const char *qtest_get_arch(void)
 {
-    const char *qemu = getenv("QTEST_QEMU_BINARY");
-    g_assert(qemu != NULL);
+    const char *qemu = qtest_qemu_binary();
     const char *end = strrchr(qemu, '/');
 
     return end + strlen("/qemu-system-");
-- 
1.8.3.1
Re: [Qemu-devel] [PATCH] tests/libqtest: Use a proper error message if QTEST_QEMU_BINARY is missing
Posted by Eric Blake 6 years, 8 months ago 
On 08/28/2017 05:35 AM, Thomas Huth wrote:
> The user can currently still cause an abort() if running certain tests
> (like the prom-env-test) without setting the QTEST_QEMU_BINARY first.
> A similar problem has been fixed with commit 7c933ad61b8f3f51337
> already, but forgot to also take care of the qtest_get_arch() function,
> so let's introduce a proper wrapper around getenv("QTEST_QEMU_BINARY")
> that can be used in both locations now.
> 
> Buglink: https://bugs.launchpad.net/qemu/+bug/1713434
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/libqtest.c | 24 +++++++++++++++---------
>  1 file changed, 15 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] tests/libqtest: Use a proper error message if QTEST_QEMU_BINARY is missing
Posted by Philippe Mathieu-Daudé 6 years, 8 months ago 
On 08/28/2017 07:35 AM, Thomas Huth wrote:
> The user can currently still cause an abort() if running certain tests
> (like the prom-env-test) without setting the QTEST_QEMU_BINARY first.
> A similar problem has been fixed with commit 7c933ad61b8f3f51337
> already, but forgot to also take care of the qtest_get_arch() function,
> so let's introduce a proper wrapper around getenv("QTEST_QEMU_BINARY")
> that can be used in both locations now.
> 
> Buglink: https://bugs.launchpad.net/qemu/+bug/1713434
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>   tests/libqtest.c | 24 +++++++++++++++---------
>   1 file changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index b9a1f18..342a77b 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -150,6 +150,19 @@ void qtest_add_abrt_handler(GHookFunc fn, const void *data)
>       g_hook_prepend(&abrt_hooks, hook);
>   }
>   
> +static const char *qtest_qemu_binary(void)
> +{
> +    const char *qemu_bin;
> +
> +    qemu_bin = getenv("QTEST_QEMU_BINARY");
> +    if (!qemu_bin) {
> +        fprintf(stderr, "Environment variable QTEST_QEMU_BINARY required\n");
> +        exit(1);
> +    }
> +
> +    return qemu_bin;
> +}
> +
>   QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
>   {
>       QTestState *s;
> @@ -157,13 +170,7 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
>       gchar *socket_path;
>       gchar *qmp_socket_path;
>       gchar *command;
> -    const char *qemu_binary;
> -
> -    qemu_binary = getenv("QTEST_QEMU_BINARY");
> -    if (!qemu_binary) {
> -        fprintf(stderr, "Environment variable QTEST_QEMU_BINARY required\n");
> -        exit(1);
> -    }
> +    const char *qemu_binary = qtest_qemu_binary();
>   
>       s = g_malloc(sizeof(*s));
>   
> @@ -624,8 +631,7 @@ char *qtest_hmp(QTestState *s, const char *fmt, ...)
>   
>   const char *qtest_get_arch(void)
>   {
> -    const char *qemu = getenv("QTEST_QEMU_BINARY");
> -    g_assert(qemu != NULL);
> +    const char *qemu = qtest_qemu_binary();
>       const char *end = strrchr(qemu, '/');
>   
>       return end + strlen("/qemu-system-");
>
Re: [Qemu-devel] [PATCH] tests/libqtest: Use a proper error message if QTEST_QEMU_BINARY is missing
Posted by John Snow 6 years, 8 months ago 

On 08/28/2017 06:35 AM, Thomas Huth wrote:
> The user can currently still cause an abort() if running certain tests
> (like the prom-env-test) without setting the QTEST_QEMU_BINARY first.
> A similar problem has been fixed with commit 7c933ad61b8f3f51337
> already, but forgot to also take care of the qtest_get_arch() function,
> so let's introduce a proper wrapper around getenv("QTEST_QEMU_BINARY")
> that can be used in both locations now.
> 
> Buglink: https://bugs.launchpad.net/qemu/+bug/1713434
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: John Snow <jsnow@redhat.com>
Re: [Qemu-devel] [PATCH] tests/libqtest: Use a proper error message if QTEST_QEMU_BINARY is missing
Posted by Michael Tokarev 6 years, 7 months ago 
28.08.2017 13:35, Thomas Huth weote:
> The user can currently still cause an abort() if running certain tests
> (like the prom-env-test) without setting the QTEST_QEMU_BINARY first.
> A similar problem has been fixed with commit 7c933ad61b8f3f51337
> already, but forgot to also take care of the qtest_get_arch() function,
> so let's introduce a proper wrapper around getenv("QTEST_QEMU_BINARY")
> that can be used in both locations now.

Applied to -trivial, thanks!

/mjt

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.