Re: [PATCH v8 4/4] new qTest case to test the vhost-user-blk-server

[Thomas Huth <thuth@redhat.com>]

On 05/06/2020 01.35, Coiby Xu wrote:
> This test case has the same tests as tests/virtio-blk-test.c except for
> tests have block_resize. Since vhost-user server can only server one
> client one time, two instances of qemu-storage-daemon are launched
> for the hotplug test.
> 
> In order to not block scripts/tap-driver.pl, vhost-user-blk-server will
> send "quit" command to qemu-storage-daemon's QMP monitor. So a function
> is added to libqtest.c to establish socket connection with socket
> server.
> 
> Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
> ---
>  tests/Makefile.include              |   3 +-
>  tests/qtest/Makefile.include        |   2 +
>  tests/qtest/libqos/vhost-user-blk.c | 130 +++++
>  tests/qtest/libqos/vhost-user-blk.h |  44 ++
>  tests/qtest/libqtest.c              |  54 +-
>  tests/qtest/libqtest.h              |  38 ++
>  tests/qtest/vhost-user-blk-test.c   | 737 ++++++++++++++++++++++++++++
>  7 files changed, 976 insertions(+), 32 deletions(-)
>  create mode 100644 tests/qtest/libqos/vhost-user-blk.c
>  create mode 100644 tests/qtest/libqos/vhost-user-blk.h
>  create mode 100644 tests/qtest/vhost-user-blk-test.c
[...]
> new file mode 100644
> index 0000000000..3de9c59194
> --- /dev/null
> +++ b/tests/qtest/libqos/vhost-user-blk.c
> @@ -0,0 +1,130 @@
> +/*
> + * libqos driver framework
> + *
> + * Based on tests/qtest/libqos/virtio-blk.c
> + *
> + * Copyright (c) 2020 Coiby Xu <coiby.xu@gmail.com>
> + *
> + * Copyright (c) 2018 Emanuele Giuseppe Esposito <e.emanuelegiuseppe@gmail.com>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License version 2.1 as published by the Free Software Foundation.

Thanks for the update! ...

[...]
> diff --git a/tests/qtest/libqos/vhost-user-blk.h b/tests/qtest/libqos/vhost-user-blk.h
> new file mode 100644
> index 0000000000..ef4ef09cca
> --- /dev/null
> +++ b/tests/qtest/libqos/vhost-user-blk.h
> @@ -0,0 +1,44 @@
> +/*
> + * libqos driver framework
> + *
> + * Copyright (c) 2018 Emanuele Giuseppe Esposito <e.emanuelegiuseppe@gmail.com>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License version 2 as published by the Free Software Foundation.

... but you've missed the header here :-(

[...]
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index 49075b55a1..a7b7c96206 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -31,40 +31,9 @@
>  #include "qapi/qmp/qlist.h"
>  #include "qapi/qmp/qstring.h"
>  
> -#define MAX_IRQ 256
>  #define SOCKET_TIMEOUT 50
>  #define SOCKET_MAX_FDS 16
>  
> -
> -typedef void (*QTestSendFn)(QTestState *s, const char *buf);
> -typedef void (*ExternalSendFn)(void *s, const char *buf);
> -typedef GString* (*QTestRecvFn)(QTestState *);
> -
> -typedef struct QTestClientTransportOps {
> -    QTestSendFn     send;      /* for sending qtest commands */
> -
> -    /*
> -     * use external_send to send qtest command strings through functions which
> -     * do not accept a QTestState as the first parameter.
> -     */
> -    ExternalSendFn  external_send;
> -
> -    QTestRecvFn     recv_line; /* for receiving qtest command responses */
> -} QTestTransportOps;
> -
> -struct QTestState
> -{
> -    int fd;
> -    int qmp_fd;
> -    pid_t qemu_pid;  /* our child QEMU process */
> -    int wstatus;
> -    int expected_status;
> -    bool big_endian;
> -    bool irq_level[MAX_IRQ];
> -    GString *rx;
> -    QTestTransportOps ops;
> -};

Why do you have to move struct QTestState and friends to the header
instead? I'd prefer if we could keep it here if possible?

 Thomas