Re: [Qemu-devel] [PATCH 09/13] nbd: don't change socket block during negotiate

[Marc-André Lureau]

On Fri, Nov 29, 2013 at 4:22 PM, Paolo Bonzini <address@hidden> wrote:
> If you remove this here, you need to remove also the matching
> socket_set_nonblock,

Ok

> Also, there are two callers:
>
> - nbd.c: you can add nbd_socket_block/nonblock around
> nbd_receive_negotiate in nbd_open.
>
> - qemu-nbd.c: here the socket can remain in blocking mode.  In fact it
> is blocking before the call to nbd_receive_negotiate, because
> unix_connect_opts is missing a call to qemu_set_block (bug!).  I suggest
> that you add the call to qemu_set_nonblock there, and add qemu_set_block
> in nbd_client_thread.

Sorry, I guess I assumed wrongly that the sync nbd reader and writer
would handle all cases.

So you suggest this block/unblock: (I haven't reviewed all callers of
unix_connect_opts(), I am not sure that's what you meant) Other option
would be to move the nonblock to unix_socket_outgoing.

diff --git a/block/nbd-client.c b/block/nbd-client.c
index 1abfc6a..693110d 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -348,6 +348,7 @@ int nbd_client_session_init(NbdClientSession *client,
     int ret;

     /* NBD handshake */
+    qemu_set_block(sock);
     ret = nbd_receive_negotiate(sock, client->export_name,
                                 &client->nbdflags, &client->size,
                                 &client->blocksize);
diff --git a/qemu-nbd.c b/qemu-nbd.c
index c26c98e..008fbf0 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -224,6 +224,7 @@ static void *nbd_client_thread(void *arg)
         goto out;
     }

+    qemu_set_block(sock);
     ret = nbd_receive_negotiate(sock, NULL, &nbdflags,
                                 &size, &blocksize);
     if (ret < 0) {
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 6b97dc1..81f2660 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -747,6 +747,8 @@ int unix_connect_opts(QemuOpts *opts, Error **errp,
         }
     } while (rc == -EINTR);

+    qemu_set_nonblock(sock);
+
     if (connect_state != NULL && QEMU_SOCKET_RC_INPROGRESS(rc)) {
         connect_state->fd = sock;
         qemu_set_fd_handler2(sock, NULL, NULL, wait_for_connect,

-- 
Marc-André Lureau