qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Coiby Xu
Subject: Re: [PATCH v10 6/7] test: new qTest case to test the vhost-user-blk-server
Date: Sat, 10 Oct 2020 15:59:55 +0800

On Wed, Sep 23, 2020 at 01:36:06PM +0100, Stefan Hajnoczi wrote:
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.

Thank you for spotting two race conditions!

--
Best regards,
Coiby



reply via email to

[Prev in Thread] Current Thread [Next in Thread]