On 06/26/2013 06:21:07 PM, Andrew Morton wrote:
> On Wed, 19 Jun 2013 17:09:18 -0400 (EDT) Paul Clements  
> <[email protected]> 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 :(

I wrote the busybox version, which might be a bit simpler:

   http://git.busybox.net/busybox/tree/networking/nbd-client.c

(Sorry about the #ifdefs, they're not mine.)

> 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 ;)

No, they're not. The C++ guys and stuffy old ex-cobol types are, and  
think it helps. (Does any architecture anywhere _not_ use int for bool?)

Rob
------------------------------------------------------------------------------
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
_______________________________________________
Nbd-general mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/nbd-general

Reply via email to