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

2013-12-01 Thread Marc-André Lureau 


- 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.

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

2013-11-30 Thread Paolo Bonzini 
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

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

2013-11-29 Thread Marc-André Lureau 
From: Marc-André Lureau marcandre.lur...@redhat.com

The caller might handle non-blocking using coroutine. Leave the choice
to the caller to use a blocking or non-blocking negotiate.

Signed-off-by: Marc-André Lureau marcandre.lur...@redhat.com
---
 nbd.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/nbd.c b/nbd.c
index f847940..3af9d17 100644
--- a/nbd.c
+++ b/nbd.c
@@ -443,7 +443,6 @@ int nbd_receive_negotiate(int csock, const char *name, 
uint32_t *flags,
 
 TRACE(Receiving negotiation.);
 
-qemu_set_block(csock);
 rc = -EINVAL;
 
 if (read_sync(csock, buf, 8) != 8) {
-- 
1.8.4.2

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

2013-11-29 Thread Paolo Bonzini 
Il 29/11/2013 15:58, Marc-André Lureau ha scritto:
 From: Marc-André Lureau marcandre.lur...@redhat.com
 
 The caller might handle non-blocking using coroutine. Leave the choice
 to the caller to use a blocking or non-blocking negotiate.
 
 Signed-off-by: Marc-André Lureau marcandre.lur...@redhat.com
 ---
  nbd.c | 1 -
  1 file changed, 1 deletion(-)
 
 diff --git a/nbd.c b/nbd.c
 index f847940..3af9d17 100644
 --- a/nbd.c
 +++ b/nbd.c
 @@ -443,7 +443,6 @@ int nbd_receive_negotiate(int csock, const char *name, 
 uint32_t *flags,
  
  TRACE(Receiving negotiation.);
  
 -qemu_set_block(csock);
  rc = -EINVAL;
  
  if (read_sync(csock, buf, 8) != 8) {
 

If you remove this here, you need to remove also the matching
socket_set_nonblock,

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.

Thanks,

Paolo