diff mbox

[09/13] nbd: don't change socket block during negotiate

Message ID CAJ+F1CKiJVdO5v99L2YHd9mJ3khfE4gXEQxi1y8EUjQeEBsu=w@mail.gmail.com
State New
Headers show

Commit Message

Marc-André Lureau Nov. 30, 2013, 3:49 p.m. UTC 
On Fri, Nov 29, 2013 at 4:22 PM, Paolo Bonzini <pbonzini@redhat.com> 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.

Comments

Paolo Bonzini Nov. 30, 2013, 7:08 p.m. UTC | #1 
Il 30/11/2013 16:49, Marc-André Lureau ha scritto:
> 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);

Also

 qemu_set_nonblock(sock);

here, and remove it from nbd_receive_negotiate.

I checked again and you need not touch unix_connect_opts, nor
nbd_client_thread.

Paolo
Marc-Andre Lureau Dec. 1, 2013, 8:56 p.m. UTC | #2 
----- Original Message -----
> Il 30/11/2013 16:49, Marc-André Lureau ha scritto:
> > 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);
> 
> Also
> 
>  qemu_set_nonblock(sock);
> 
> here, 


It's already a few lines below.

> and remove it from nbd_receive_negotiate.

> I checked again and you need not touch unix_connect_opts, nor
> nbd_client_thread.

Ok, I'll remove those.
diff mbox

Patch

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,