[v3,05/22] libqtest: Add a layer of abstraciton to send/recv

Commit Message

Alexander Bulekov Sept. 18, 2019, 11:19 p.m. UTC

This makes it simple to swap the transport functions for qtest commands
to and from the qtest client. For example, now it is possible to
directly pass qtest commands to a server handler that exists within the
same process, without the standard way of writing to a file descriptor.

Signed-off-by: Alexander Oleinik <[e-mail]>
---
 tests/libqtest.c | 64 +++++++++++++++++++++++++++++++++++++++---------
 tests/libqtest.h |  1 -
 2 files changed, 53 insertions(+), 12 deletions(-)

Comments

Stefan Hajnoczi Sept. 19, 2019, 10:24 a.m. UTC | #1

On Wed, Sep 18, 2019 at 11:19:32PM +0000, Oleinik, Alexander wrote:
> This makes it simple to swap the transport functions for qtest commands
> to and from the qtest client. For example, now it is possible to
> directly pass qtest commands to a server handler that exists within the
> same process, without the standard way of writing to a file descriptor.
> 
> Signed-off-by: Alexander Oleinik <[e-mail]>
> ---
>  tests/libqtest.c | 64 +++++++++++++++++++++++++++++++++++++++---------
>  tests/libqtest.h |  1 -
>  2 files changed, 53 insertions(+), 12 deletions(-)

Reviewed-by: Stefan Hajnoczi <[e-mail]>

Paolo Bonzini Sept. 19, 2019, 11:18 a.m. UTC | #2

On 19/09/19 01:19, Oleinik, Alexander wrote:
> +static GString *qtest_client_inproc_recv_line(void *);

This is not defined as of this patch, please move it to patch 14.

>  
> +    qtest_client_set_rx_handler(s, qtest_client_socket_recv_line, s);
> +    qtest_client_set_tx_handler(s, qtest_client_socket_send, &(s->fd));
> +

I think you can pass "s" to the tx handler as well, and remove the
send_opaque and recv_opaque fields?

Paolo

Alexander Bulekov Sept. 19, 2019, 1:27 p.m. UTC | #3

On Thu, 2019-09-19 at 13:18 +0200, Paolo Bonzini wrote:
> I think you can pass "s" to the tx handler as well, and remove the
> send_opaque and recv_opaque fields?

Qtest also uses this function to communicate over qmp  (different fd).
I can probably make the tx handler a wrapper which accepts "s", and
passes s->fd to the real socket_send.

Paolo Bonzini Sept. 19, 2019, 2:27 p.m. UTC | #4

On 19/09/19 15:27, Oleinik, Alexander wrote:
> On Thu, 2019-09-19 at 13:18 +0200, Paolo Bonzini wrote:
>> I think you can pass "s" to the tx handler as well, and remove the
>> send_opaque and recv_opaque fields?
> 
> Qtest also uses this function to communicate over qmp  (different fd).
> I can probably make the tx handler a wrapper which accepts "s", and
> passes s->fd to the real socket_send.

Yes, exactly.

Thanks,

Paolo

Patch

diff --git a/tests/libqtest.c b/tests/libqtest.c
index 4a7556462d..19feea9e17 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -35,6 +35,15 @@ 
 #define SOCKET_TIMEOUT 50
 #define SOCKET_MAX_FDS 16
 
+
+typedef struct QTestClientTransportOps {
+    void     (*send)(void* , const char*, size_t);
+    void     *send_opaque;
+
+    GString* (*recv_line)(void *);
+    void     *recv_line_opaque;
+} QTestTransportOps;
+
 struct QTestState
 {
     int fd;
@@ -45,13 +54,28 @@  struct QTestState
     bool big_endian;
     bool irq_level[MAX_IRQ];
     GString *rx;
+    QTestTransportOps ops;
 };
 
 static GHookList abrt_hooks;
 static struct sigaction sigact_old;
+static GString *recv_str;
 
 static int qtest_query_target_endianness(QTestState *s);
 
+static void qtest_client_socket_send(void *opaque,
+        const char *buf, size_t size);
+static void socket_send(int fd, const char *buf, size_t size);
+
+static GString *qtest_client_socket_recv_line(void *);
+static GString *qtest_client_inproc_recv_line(void *);
+
+static void qtest_client_set_tx_handler(QTestState *s,
+        void (*send)(void *, const char *, size_t), void *opaque);
+static void qtest_client_set_rx_handler(QTestState *s,
+        GString * (*recv)(void *), void *opaque);
+
+
 static int init_socket(const char *socket_path)
 {
     struct sockaddr_un addr;
@@ -234,6 +258,9 @@  QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
     sock = init_socket(socket_path);
     qmpsock = init_socket(qmp_socket_path);
 
+    qtest_client_set_rx_handler(s, qtest_client_socket_recv_line, s);
+    qtest_client_set_tx_handler(s, qtest_client_socket_send, &(s->fd));
+
     qtest_add_abrt_handler(kill_qemu_hook_func, s);
 
     command = g_strdup_printf("exec %s "
@@ -360,8 +387,9 @@  void qtest_quit(QTestState *s)
     g_free(s);
 }
 
-static void socket_send(int fd, const char *buf, size_t size)
+static void qtest_client_socket_send(void *opaque, const char *buf, size_t size)
 {
+    int fd = *((int *)opaque);
     size_t offset;
 
     offset = 0;
@@ -379,22 +407,22 @@  static void socket_send(int fd, const char *buf, size_t size)
     }
 }
 
-static void socket_sendf(int fd, const char *fmt, va_list ap)
+static void socket_send(int fd, const char *buf, size_t size)
 {
-    gchar *str = g_strdup_vprintf(fmt, ap);
-    size_t size = strlen(str);
-
-    socket_send(fd, str, size);
-    g_free(str);
+    qtest_client_socket_send(&fd, buf, size);
 }
 
 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);
+    gchar *str = g_strdup_vprintf(fmt, ap);
     va_end(ap);
+
+    size_t size = strlen(str);
+
+    s->ops.send(s, str, size);
+    g_free(str);
 }
 
 /* Sends a message and file descriptors to the socket.
@@ -431,8 +459,9 @@  static void socket_send_fds(int socket_fd, int *fds, size_t fds_num,
     g_assert_cmpint(ret, >, 0);
 }
 
-static GString *qtest_recv_line(QTestState *s)
+static GString *qtest_client_socket_recv_line(void* opaque)
 {
+    QTestState *s = opaque;
     GString *line;
     size_t offset;
     char *eol;
@@ -468,7 +497,7 @@  static gchar **qtest_rsp(QTestState *s, int expected_args)
     int i;
 
 redo:
-    line = qtest_recv_line(s);
+    line = s->ops.recv_line(s->ops.recv_line_opaque);
     words = g_strsplit(line->str, " ", 0);
     g_string_free(line, TRUE);
 
@@ -1327,3 +1356,16 @@  void qmp_assert_error_class(QDict *rsp, const char *class)
 
     qobject_unref(rsp);
 }
+
+static void qtest_client_set_tx_handler(QTestState *s,
+                    void (*send)(void*, const char*, size_t), void *opaque)
+{
+    s->ops.send = send;
+    s->ops.send_opaque = opaque;
+}
+static void qtest_client_set_rx_handler(QTestState *s,
+                    GString* (*recv)(void *), void *opaque)
+{
+    s->ops.recv_line = recv;
+    s->ops.recv_line_opaque = opaque;
+}
diff --git a/tests/libqtest.h b/tests/libqtest.h
index a177e502d9..40fa235a52 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -716,5 +716,4 @@  bool qtest_probe_child(QTestState *s);
  * Set expected exit status of the child.
  */
 void qtest_set_expected_status(QTestState *s, int status);
-
 #endif