diff mbox

[1/3] nbd: allow multiple disconnects to be sent

Message ID 1500648495-13337-1-git-send-email-jbacik@fb.com (mailing list archive)
State New, archived
Headers show

Commit Message

Josef Bacik July 21, 2017, 2:48 p.m. UTC
From: Josef Bacik <jbacik@fb.com>

There's no reason to limit ourselves to one disconnect message per
socket.  Sometimes networks do strange things, might as well let
sysadmins hit the panic button as much as they want.

Signed-off-by: Josef Bacik <jbacik@fb.com>
---
 drivers/block/nbd.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Jens Axboe July 22, 2017, 5:13 p.m. UTC | #1
On 07/21/2017 08:48 AM, josef@toxicpanda.com wrote:
> From: Josef Bacik <jbacik@fb.com>
> 
> There's no reason to limit ourselves to one disconnect message per
> socket.  Sometimes networks do strange things, might as well let
> sysadmins hit the panic button as much as they want.

Applied 1-3 for 4.13, thanks Josef.
Wouter Verhelst July 24, 2017, 8:08 p.m. UTC | #2
On Fri, Jul 21, 2017 at 10:48:13AM -0400, josef@toxicpanda.com wrote:
> From: Josef Bacik <jbacik@fb.com>
> 
> There's no reason to limit ourselves to one disconnect message per
> socket.  Sometimes networks do strange things, might as well let
> sysadmins hit the panic button as much as they want.

The protocol spec is pretty clear that any requests sent after the
disconnect request was sent out are not guaranteed to be processed
anymore.

Doesn't this allow more requests to be sent out? Or is the
NBD_DISCONNECT_REQUESTED flag enough to make that impossible?
Josef Bacik July 24, 2017, 11:24 p.m. UTC | #3
On Mon, Jul 24, 2017 at 10:08:21PM +0200, Wouter Verhelst wrote:
> On Fri, Jul 21, 2017 at 10:48:13AM -0400, josef@toxicpanda.com wrote:
> > From: Josef Bacik <jbacik@fb.com>
> > 
> > There's no reason to limit ourselves to one disconnect message per
> > socket.  Sometimes networks do strange things, might as well let
> > sysadmins hit the panic button as much as they want.
> 
> The protocol spec is pretty clear that any requests sent after the
> disconnect request was sent out are not guaranteed to be processed
> anymore.
> 
> Doesn't this allow more requests to be sent out? Or is the
> NBD_DISCONNECT_REQUESTED flag enough to make that impossible?
> 

This just allows users to call the disconnect ioctl/netlink thing multiple times
and have it send the DISCONNECT command if they want.  We've had problems with
our in-hosue nbd server missing messages, and it's just a pain to have to
unstuck it because the server messed up.  It's just for the rare case the server
is being weird, not because we expect/guarantee that subsequent disconnect
commands will be processed.  Thanks,

Josef
Wouter Verhelst July 25, 2017, 10:08 a.m. UTC | #4
On Mon, Jul 24, 2017 at 07:24:30PM -0400, Josef Bacik wrote:
> On Mon, Jul 24, 2017 at 10:08:21PM +0200, Wouter Verhelst wrote:
> > On Fri, Jul 21, 2017 at 10:48:13AM -0400, josef@toxicpanda.com wrote:
> > > From: Josef Bacik <jbacik@fb.com>
> > > 
> > > There's no reason to limit ourselves to one disconnect message per
> > > socket.  Sometimes networks do strange things, might as well let
> > > sysadmins hit the panic button as much as they want.
> > 
> > The protocol spec is pretty clear that any requests sent after the
> > disconnect request was sent out are not guaranteed to be processed
> > anymore.
> > 
> > Doesn't this allow more requests to be sent out? Or is the
> > NBD_DISCONNECT_REQUESTED flag enough to make that impossible?
> > 
> 
> This just allows users to call the disconnect ioctl/netlink thing multiple times
> and have it send the DISCONNECT command if they want.

Right.

> We've had problems with our in-hosue nbd server missing messages,

That's pretty bad...

> and it's just a pain to have to unstuck it because the server messed up.
> It's just for the rare case the server is being weird, not because we
> expect/guarantee that subsequent disconnect commands will be processed.
> Thanks,

Okay, makes sense. Just thought I'd ask :-)
diff mbox

Patch

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 87a0a29..f91e7ac 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -991,9 +991,8 @@  static int nbd_disconnect(struct nbd_device *nbd)
 	struct nbd_config *config = nbd->config;
 
 	dev_info(disk_to_dev(nbd->disk), "NBD_DISCONNECT\n");
-	if (!test_and_set_bit(NBD_DISCONNECT_REQUESTED,
-			      &config->runtime_flags))
-		send_disconnects(nbd);
+	set_bit(NBD_DISCONNECT_REQUESTED, &config->runtime_flags);
+	send_disconnects(nbd);
 	return 0;
 }