Re: [Qemu-devel] [PATCH 3/3] nbd: Shutdown socket before closing.
On Thu, Jun 05, 2014 at 03:55:40AM +0200, Paolo Bonzini wrote: Il 05/06/2014 00:33, Hani Benhabiles ha scritto: IIUC, what this does is ensure that the other side gets a FIN before it gets a RST. Is this correct? Yes. Without shutdown(), this could be reproduced (unreliably) on multiple tries. This is done in nbd_client_close() too, for the same reasons AFAICT. Actually, nbd_client_close() is different because it's an abortive close of the socket. nbd_client_close() doesn't care about FIN vs. RST, it does the shutdown to force all the requests to fail (with either an error for writes, or a short read if they're receiving). This will cause a flurry of nbd_client_put() calls soon after nbd_clint_close() returns, until the last reference is dropped and the socket is closed. I see, thanks for the explanation. I'll apply the patch. Will you apply it directly or should I resend it in v3 ?
Re: [Qemu-devel] [PATCH 3/3] nbd: Shutdown socket before closing.
Il 05/06/2014 11:58, Hani Benhabiles ha scritto: On Thu, Jun 05, 2014 at 03:55:40AM +0200, Paolo Bonzini wrote: Il 05/06/2014 00:33, Hani Benhabiles ha scritto: IIUC, what this does is ensure that the other side gets a FIN before it gets a RST. Is this correct? Yes. Without shutdown(), this could be reproduced (unreliably) on multiple tries. This is done in nbd_client_close() too, for the same reasons AFAICT. Actually, nbd_client_close() is different because it's an abortive close of the socket. nbd_client_close() doesn't care about FIN vs. RST, it does the shutdown to force all the requests to fail (with either an error for writes, or a short read if they're receiving). This will cause a flurry of nbd_client_put() calls soon after nbd_clint_close() returns, until the last reference is dropped and the socket is closed. I see, thanks for the explanation. I'll apply the patch. Will you apply it directly or should I resend it in v3 ? This is independent, so I can apply it first. Paolo
Re: [Qemu-devel] [PATCH 3/3] nbd: Shutdown socket before closing.
On Tue, Jun 03, 2014 at 01:33:41PM +0200, Paolo Bonzini wrote:
Il 31/05/2014 23:39, Hani Benhabiles ha scritto:
This forces finishing data sending to client before closing the socket like
in
exports listing or replying with NBD_REP_ERR_UNSUP cases.
Signed-off-by: Hani Benhabiles h...@linux.com
---
blockdev-nbd.c | 1 +
qemu-nbd.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index b60b66d..e609f66 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -28,6 +28,7 @@ static void nbd_accept(void *opaque)
int fd = accept(server_fd, (struct sockaddr *)addr, addr_len);
if (fd = 0 !nbd_client_new(NULL, fd, nbd_client_put)) {
+shutdown(fd, SHUT_RDWR);
close(fd);
}
}
diff --git a/qemu-nbd.c b/qemu-nbd.c
index ba60436..bf42861 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -372,6 +372,7 @@ static void nbd_accept(void *opaque)
if (nbd_client_new(exp, fd, nbd_client_closed)) {
nb_fds++;
} else {
+shutdown(fd, SHUT_RDWR);
close(fd);
}
}
IIUC, what this does is ensure that the other side gets a FIN before it gets
a RST. Is this correct?
Yes. Without shutdown(), this could be reproduced (unreliably) on multiple
tries. This is done in nbd_client_close() too, for the same reasons AFAICT.
Re: [Qemu-devel] [PATCH 3/3] nbd: Shutdown socket before closing.
Il 05/06/2014 00:33, Hani Benhabiles ha scritto: IIUC, what this does is ensure that the other side gets a FIN before it gets a RST. Is this correct? Yes. Without shutdown(), this could be reproduced (unreliably) on multiple tries. This is done in nbd_client_close() too, for the same reasons AFAICT. Actually, nbd_client_close() is different because it's an abortive close of the socket. nbd_client_close() doesn't care about FIN vs. RST, it does the shutdown to force all the requests to fail (with either an error for writes, or a short read if they're receiving). This will cause a flurry of nbd_client_put() calls soon after nbd_clint_close() returns, until the last reference is dropped and the socket is closed. I'll apply the patch. Paolo
Re: [Qemu-devel] [PATCH 3/3] nbd: Shutdown socket before closing.
Il 31/05/2014 23:39, Hani Benhabiles ha scritto:
This forces finishing data sending to client before closing the socket like in
exports listing or replying with NBD_REP_ERR_UNSUP cases.
Signed-off-by: Hani Benhabiles h...@linux.com
---
blockdev-nbd.c | 1 +
qemu-nbd.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index b60b66d..e609f66 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -28,6 +28,7 @@ static void nbd_accept(void *opaque)
int fd = accept(server_fd, (struct sockaddr *)addr, addr_len);
if (fd = 0 !nbd_client_new(NULL, fd, nbd_client_put)) {
+shutdown(fd, SHUT_RDWR);
close(fd);
}
}
diff --git a/qemu-nbd.c b/qemu-nbd.c
index ba60436..bf42861 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -372,6 +372,7 @@ static void nbd_accept(void *opaque)
if (nbd_client_new(exp, fd, nbd_client_closed)) {
nb_fds++;
} else {
+shutdown(fd, SHUT_RDWR);
close(fd);
}
}
IIUC, what this does is ensure that the other side gets a FIN before it
gets a RST. Is this correct?
Paolo
[Qemu-devel] [PATCH 3/3] nbd: Shutdown socket before closing.
This forces finishing data sending to client before closing the socket like in
exports listing or replying with NBD_REP_ERR_UNSUP cases.
Signed-off-by: Hani Benhabiles h...@linux.com
---
blockdev-nbd.c | 1 +
qemu-nbd.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index b60b66d..e609f66 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -28,6 +28,7 @@ static void nbd_accept(void *opaque)
int fd = accept(server_fd, (struct sockaddr *)addr, addr_len);
if (fd = 0 !nbd_client_new(NULL, fd, nbd_client_put)) {
+shutdown(fd, SHUT_RDWR);
close(fd);
}
}
diff --git a/qemu-nbd.c b/qemu-nbd.c
index ba60436..bf42861 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -372,6 +372,7 @@ static void nbd_accept(void *opaque)
if (nbd_client_new(exp, fd, nbd_client_closed)) {
nb_fds++;
} else {
+shutdown(fd, SHUT_RDWR);
close(fd);
}
}
--
1.8.3.2
