Re: [PATCH v10 6/7] test: new qTest case to test the vhost-user-blk-server

[Stefan Hajnoczi]

On Fri, Sep 18, 2020 at 04:09:11PM +0800, Coiby Xu wrote:
> +int qtest_socket_client(char *server_socket_path)
> +{
> +    struct sockaddr_un serv_addr;
> +    int sock;
> +    int ret;
> +    int retries = 0;
> +    sock = socket(PF_UNIX, SOCK_STREAM, 0);
> +    g_assert_cmpint(sock, !=, -1);
> +    serv_addr.sun_family = AF_UNIX;
> +    snprintf(serv_addr.sun_path, sizeof(serv_addr.sun_path), "%s",
> +             server_socket_path);
> +
> +    for (retries = 0; retries < 3; retries++) {
> +        ret = connect(sock, (struct sockaddr *)&serv_addr, 
> sizeof(serv_addr));
> +        if (ret == 0) {
> +            break;
> +        }
> +        g_usleep(G_USEC_PER_SEC);
> +    }

This is a race condition. On a heavily loaded machine the server might
not be available within 3 seconds and the test will fail randomly.

Solutions:
1. Wait output from the server indicating it is ready (e.g. 'Listening
   on /path/to/foo.sock...') when spawning the server process.
2. Create the listen socket and pass the fd to the server process. This
   way the socket already exists can the client will block in connect
   until the server accepts the connection.
3. Create a socketpair. Pass one side to the server and use the other
   side in the client.

However, I think this is okay for now. After my patch series that
converts the vhost-user-blk server to the new block exports API we can
consider how to pass file descriptors.

> +static void quit_storage_daemon(void *qmp_test_state)
> +{
> +    const char quit_str[] = "{ 'execute': 'quit' }";
> +
> +    /* Before quiting storate-daemon, quit qemu to avoid dubious messages */
> +    qobject_unref(qtest_qmp(global_qtest, quit_str));
> +
> +    /*
> +     * Give storage-daemon enough time to wake up&terminate
> +     * vu_client_trip coroutine so the Coroutine object could
> +     * be cleaned up. Otherwise LeakSanitizer would complain
> +     * about memory leaks.
> +     */
> +    g_usleep(1000);

Also a race that may cause random failures. This can be addressed after
the block exports API conversion.

signature.asc
Description: PGP signature