1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH v5 0/3] tests/microbit-test: Add UART device test
  4. View patch

[Qemu-devel] [PATCH v5 1/3] tests/libqtest: Introduce qtest_init_with_serial()

Julia Suvorova via Qemu-devel posted 3 patches 6 years, 9 months ago
[Qemu-devel] [PATCH v5 1/3] tests/libqtest: Introduce qtest_init_with_serial()
Posted by Julia Suvorova via Qemu-devel 6 years, 9 months ago 
Run qtest with a socket that connects QEMU chardev and test code.

Signed-off-by: Julia Suvorova <jusual@mail.ru>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/libqtest.c | 25 +++++++++++++++++++++++++
 tests/libqtest.h | 11 +++++++++++
 2 files changed, 36 insertions(+)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index 55750dd68d..6fb30855fa 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -315,6 +315,31 @@ QTestState *qtest_initf(const char *fmt, ...)
     return s;
 }
 
+QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd)
+{
+    int sock_fd_init;
+    char *sock_path, sock_dir[] = "/tmp/qtest-serial-XXXXXX";
+    QTestState *qts;
+
+    g_assert_true(mkdtemp(sock_dir) != NULL);
+    sock_path = g_strdup_printf("%s/sock", sock_dir);
+
+    sock_fd_init = init_socket(sock_path);
+
+    qts = qtest_initf("-chardev socket,id=s0,path=%s -serial chardev:s0 %s",
+                      sock_path, extra_args);
+
+    *sock_fd = socket_accept(sock_fd_init);
+
+    unlink(sock_path);
+    g_free(sock_path);
+    rmdir(sock_dir);
+
+    g_assert_true(*sock_fd >= 0);
+
+    return qts;
+}
+
 void qtest_quit(QTestState *s)
 {
     g_hook_destroy_link(&abrt_hooks, g_hook_find_data(&abrt_hooks, TRUE, s));
diff --git a/tests/libqtest.h b/tests/libqtest.h
index 7ea94139b0..5937f91912 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -62,6 +62,17 @@ QTestState *qtest_init(const char *extra_args);
  */
 QTestState *qtest_init_without_qmp_handshake(const char *extra_args);
 
+/**
+ * qtest_init_with_serial:
+ * @extra_args: other arguments to pass to QEMU.  CAUTION: these
+ * arguments are subject to word splitting and shell evaluation.
+ * @sock_fd: pointer to store the socket file descriptor for
+ * connection with serial.
+ *
+ * Returns: #QTestState instance.
+ */
+QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd);
+
 /**
  * qtest_quit:
  * @s: #QTestState instance to operate on.
-- 
2.17.1
Re: [Qemu-devel] [PATCH v5 1/3] tests/libqtest: Introduce qtest_init_with_serial()
Posted by Thomas Huth 6 years, 9 months ago 
On 2019-01-23 13:07, Julia Suvorova wrote:
> Run qtest with a socket that connects QEMU chardev and test code.
> 
> Signed-off-by: Julia Suvorova <jusual@mail.ru>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  tests/libqtest.c | 25 +++++++++++++++++++++++++
>  tests/libqtest.h | 11 +++++++++++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 55750dd68d..6fb30855fa 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -315,6 +315,31 @@ QTestState *qtest_initf(const char *fmt, ...)
>      return s;
>  }
>  
> +QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd)
> +{
> +    int sock_fd_init;
> +    char *sock_path, sock_dir[] = "/tmp/qtest-serial-XXXXXX";
> +    QTestState *qts;
> +
> +    g_assert_true(mkdtemp(sock_dir) != NULL);

Oh, nice, I wasn't aware of g_assert_true yet, that's a good idea here,
indeed.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Re: [Qemu-devel] [PATCH v5 1/3] tests/libqtest: Introduce qtest_init_with_serial()
Posted by Alex Bennée 6 years, 9 months ago 
Julia Suvorova <jusual@mail.ru> writes:

> Run qtest with a socket that connects QEMU chardev and test code.
>
> Signed-off-by: Julia Suvorova <jusual@mail.ru>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  tests/libqtest.c | 25 +++++++++++++++++++++++++
>  tests/libqtest.h | 11 +++++++++++
>  2 files changed, 36 insertions(+)
>
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 55750dd68d..6fb30855fa 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -315,6 +315,31 @@ QTestState *qtest_initf(const char *fmt, ...)
>      return s;
>  }
>
> +QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd)
> +{
> +    int sock_fd_init;
> +    char *sock_path, sock_dir[] = "/tmp/qtest-serial-XXXXXX";
> +    QTestState *qts;
> +
> +    g_assert_true(mkdtemp(sock_dir) != NULL);
> +    sock_path = g_strdup_printf("%s/sock", sock_dir);
> +
> +    sock_fd_init = init_socket(sock_path);
> +
> +    qts = qtest_initf("-chardev socket,id=s0,path=%s -serial chardev:s0 %s",
> +                      sock_path, extra_args);
> +
> +    *sock_fd = socket_accept(sock_fd_init);
> +
> +    unlink(sock_path);
> +    g_free(sock_path);
> +    rmdir(sock_dir);
> +
> +    g_assert_true(*sock_fd >= 0);
> +
> +    return qts;
> +}
> +
>  void qtest_quit(QTestState *s)
>  {
>      g_hook_destroy_link(&abrt_hooks, g_hook_find_data(&abrt_hooks, TRUE, s));
> diff --git a/tests/libqtest.h b/tests/libqtest.h
> index 7ea94139b0..5937f91912 100644
> --- a/tests/libqtest.h
> +++ b/tests/libqtest.h
> @@ -62,6 +62,17 @@ QTestState *qtest_init(const char *extra_args);
>   */
>  QTestState *qtest_init_without_qmp_handshake(const char *extra_args);
>
> +/**
> + * qtest_init_with_serial:
> + * @extra_args: other arguments to pass to QEMU.  CAUTION: these
> + * arguments are subject to word splitting and shell evaluation.
> + * @sock_fd: pointer to store the socket file descriptor for
> + * connection with serial.
> + *
> + * Returns: #QTestState instance.
> + */
> +QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd);
> +
>  /**
>   * qtest_quit:
>   * @s: #QTestState instance to operate on.


--
Alex Bennée
Re: [Qemu-devel] [PATCH v5 1/3] tests/libqtest: Introduce qtest_init_with_serial()
Posted by Eric Blake 6 years, 9 months ago 
On 1/23/19 8:20 AM, Alex Bennée wrote:
> 
> Julia Suvorova <jusual@mail.ru> writes:
> 
>> Run qtest with a socket that connects QEMU chardev and test code.
>>
>> Signed-off-by: Julia Suvorova <jusual@mail.ru>
>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> 
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> 

>> +QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd)
>> +{
>> +    int sock_fd_init;
>> +    char *sock_path, sock_dir[] = "/tmp/qtest-serial-XXXXXX";
>> +    QTestState *qts;
>> +
>> +    g_assert_true(mkdtemp(sock_dir) != NULL);
>> +    sock_path = g_strdup_printf("%s/sock", sock_dir);
>> +
>> +    sock_fd_init = init_socket(sock_path);
>> +
>> +    qts = qtest_initf("-chardev socket,id=s0,path=%s -serial chardev:s0 %s",
>> +                      sock_path, extra_args);

EWWWW.  Please, let's not bake in this interface.  Relying on shell
splitting is nasty.


>>
>> +/**
>> + * qtest_init_with_serial:
>> + * @extra_args: other arguments to pass to QEMU.  CAUTION: these
>> + * arguments are subject to word splitting and shell evaluation.
>> + * @sock_fd: pointer to store the socket file descriptor for
>> + * connection with serial.
>> + *
>> + * Returns: #QTestState instance.
>> + */
>> +QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd);

I would MUCH rather see an interface that used varargs to pass extra
arguments as a list/array of actual arguments, no shell splitting required.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH v5 1/3] tests/libqtest: Introduce qtest_init_with_serial()
Posted by Daniel P. Berrangé 6 years, 9 months ago 
On Wed, Jan 23, 2019 at 08:44:12AM -0600, Eric Blake wrote:
> On 1/23/19 8:20 AM, Alex Bennée wrote:
> > 
> > Julia Suvorova <jusual@mail.ru> writes:
> > 
> >> Run qtest with a socket that connects QEMU chardev and test code.
> >>
> >> Signed-off-by: Julia Suvorova <jusual@mail.ru>
> >> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> > 
> > Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> > 
> 
> >> +QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd)
> >> +{
> >> +    int sock_fd_init;
> >> +    char *sock_path, sock_dir[] = "/tmp/qtest-serial-XXXXXX";
> >> +    QTestState *qts;
> >> +
> >> +    g_assert_true(mkdtemp(sock_dir) != NULL);
> >> +    sock_path = g_strdup_printf("%s/sock", sock_dir);
> >> +
> >> +    sock_fd_init = init_socket(sock_path);
> >> +
> >> +    qts = qtest_initf("-chardev socket,id=s0,path=%s -serial chardev:s0 %s",
> >> +                      sock_path, extra_args);
> 
> EWWWW.  Please, let's not bake in this interface.  Relying on shell
> splitting is nasty.

qtest_initf() is long a standing interface that's relied on
shell splitting for years :-( I agree we should fix that, but
its a pre-existing problem that this patch doesn't make worse.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.