On Wed, 19 Jun 2013 17:09:18 -0400 (EDT) Paul Clements
<paul.clements@...124...> wrote:
> Currently, when a disconnect is requested by the user (via
NBD_DISCONNECT
> ioctl) the return from NBD_DO_IT is undefined (it is usually one of
> several error codes). This means that nbd-client does not know if a
> manual disconnect was performed or whether a network error occurred.
> Because of this, nbd-client's persist mode (which tries to
reconnect after
> error, but not after manual disconnect) does not always work
correctly.
>
> This change fixes this by causing NBD_DO_IT to always return 0 if a
user
> requests a disconnect. This means that nbd-client can correctly
either
> persist the connection (if an error occurred) or disconnect (if the
user
> requested it).
This sounds like something which users of 3.10 and earlier kernels
might want, so I added the Cc:stable tag. Please let me know if
you disagree.
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -623,6 +623,8 @@ static int __nbd_ioctl(struct block_device
*bdev, struct nbd_device *nbd,
> if (!nbd->sock)
> return -EINVAL;
>
> + nbd->disconnect = 1;
> +
> nbd_send_req(nbd, &sreq);
> return 0;
> }
> @@ -654,6 +656,7 @@ static int __nbd_ioctl(struct block_device
*bdev, struct nbd_device *nbd,
> nbd->sock = SOCKET_I(inode);
> if (max_part > 0)
> bdev->bd_invalidated = 1;
> + nbd->disconnect = 0; /* we're connected
now */
> return 0;
> } else {
> fput(file);
> @@ -742,6 +745,8 @@ static int __nbd_ioctl(struct block_device
*bdev, struct nbd_device *nbd,
> set_capacity(nbd->disk, 0);
> if (max_part > 0)
> ioctl_by_bdev(bdev, BLKRRPART, 0);
> + if (nbd->disconnect) /* user requested, ignore socket
errors */
> + return 0;
> return nbd->harderror;
> }
hm, how does nbd work... Hard to tell as nothing seems to be
documented
anywhere :(
afacit the code assumes that the user will run ioctl(NBD_DISCONNECT)
and
then ioctl(NBD_DO_IT) and then ioctl(NBD_SET_SOCK), yes? Does this
change mean that if userspace calls the ioctls in an
other-than-expected order, Weird Things will happen? Would it be
safer
to clear ->disconnect in NBD_DO_IT?
> --- a/include/linux/nbd.h
> +++ b/include/linux/nbd.h
> @@ -41,6 +41,7 @@ struct nbd_device {
> u64 bytesize;
> pid_t pid; /* pid of nbd-client, if attached */
> int xmit_timeout;
> + int disconnect; /* a disconnect has been requested by user */
> };
The cool kids are using bool lately ;)