Re: [PATCH 4/4] nbd: add a nbd-control interface

[Josef Bacik <jbacik@fb.com>]

On Tue, Jan 24, 2017 at 2:11 AM, Greg KH <gregkh@linuxfoundation.org> 
wrote:
> On Mon, Jan 23, 2017 at 10:52:42AM -0500, Josef Bacik wrote:
>>  On Mon, Jan 23, 2017 at 10:03 AM, Greg KH 
>> <gregkh@linuxfoundation.org>
>>  wrote:
>>  > On Mon, Jan 23, 2017 at 09:57:52AM -0500, Josef Bacik wrote:
>>  > >  On Mon, Jan 23, 2017 at 9:52 AM, Greg KH
>>  > > <gregkh@linuxfoundation.org> wrote:
>>  > >  > On Mon, Jan 23, 2017 at 09:42:08AM -0500, Josef Bacik wrote:
>>  > >  > >
>>  > >  > >
>>  > >  > >  On Sat, Jan 21, 2017 at 4:05 AM, Greg KH
>>  > >  > > <gregkh@linuxfoundation.org> wrote:
>>  > >  > >  > On Fri, Jan 20, 2017 at 04:56:52PM -0500, Josef Bacik 
>> wrote:
>>  > >  > >  > >  This patch mirrors the loop back device behavior 
>> with a
>>  > > few
>>  > >  > >  > > changes.  First
>>  > >  > >  > >  there is no DEL operation as NBD doesn't get as much
>>  > > churn as
>>  > >  > > loop
>>  > >  > >  > > devices do.
>>  > >  > >  > >  Secondly the GET_NEXT operation can optionally 
>> create a
>>  > > new NBD
>>  > >  > >  > > device or not.
>>  > >  > >  > >  Our infrastructure people want to not allow NBD to 
>> create
>>  > > new
>>  > >  > >  > > devices as it
>>  > >  > >  > >  causes problems for them in containers.  However 
>> allow
>>  > > this to
>>  > >  > > be
>>  > >  > >  > > optional as
>>  > >  > >  > >  things like the OSS NBD client probably doesn't care 
>> and
>>  > > would
>>  > >  > > like
>>  > >  > >  > > to just be
>>  > >  > >  > >  given a device to use.
>>  > >  > >  > >
>>  > >  > >  > >  Signed-off-by: Josef Bacik <jbacik@fb.com>
>>  > >  > >  >
>>  > >  > >  > A random char device with odd ioctls?  Why?  There's no 
>> other
>>  > >  > >  > configuration choice you could possibly use?  Where is 
>> the
>>  > >  > > userspace
>>  > >  > >  > tool that uses this new kernel api?
>>  > >  > >  >
>>  > >  > >  > You aren't passing in structures to the ioctl, so why 
>> does
>>  > > this
>>  > >  > > HAVE to
>>  > >  > >  > be an ioctl?
>>  > >  > >
>>  > >  > >  Again, this is how loop does it so I assumed a known, 
>> regularly
>>  > >  > > used API was
>>  > >  > >  the best bet.  I can do literally anything, but these
>>  > > interfaces
>>  > >  > > have to be
>>  > >  > >  used by other people, including internal people.  The
>>  > >  > > /dev/whatever-control
>>  > >  > >  is a well established way for interacting with dynamic 
>> device
>>  > >  > > drivers (loop,
>>  > >  > >  DM, btrfs), so that's what I went with.  Thanks,
>>  > >  >
>>  > >  > Again, please don't duplicate what loop did, we must _learn_ 
>> from
>>  > >  > history, not repeat it :(
>>  > >
>>  > >  Sure but what am I supposed to do?  Have some random sysfs 
>> knobs?
>>  > > Thanks,
>>  >
>>  > It all depends on what you are trying to do.  I have yet to 
>> figure that
>>  > out at all here :(
>> 
>>  I explained it in the changelog and my response to Wouter.  NBD 
>> preallocates
>>  all of its /dev/nbd# devices at modprobe time, so there's no way to 
>> add new
>>  devices as we need them.
> 
> Why not fix that odd restriction?

That's the entire point of this patchset, adding the ability to create 
new devices on the fly as needed.

> 
> 
>>  Loop accomplishes this with the /dev/loop-control
>>  and an ioctl.  Then we also need a way to figure out what is the 
>> first
>>  /dev/nbd# device that isn't currently in use in order to pick the 
>> next one
>>  to configure.  Keep in mind that all of the configuration for 
>> /dev/nbd#
>>  devices is done through ioctls to those devices, so having a ioctl 
>> interface
>>  for the control device is consistent with the rest of how NBD works.
> 
> But adding a random char device node and using ioctls on it is 
> different
> than using an ioctl on an existing block device node that is already
> there.
> 
> So what _exactly_ do you need to do with this interface?  What data do
> you need sent/received to/from the kernel?  Specifics please.

I need to say "Hey kernel, can you setup some new nbd devices for me?" 
and "Hey kernel, what is the first free nbd device I can use for this 
new connection and (optionally) create a new device for me if one does 
not exist?"  Loop accomplished this (in 2011 btw, not some far off 
date) with /dev/loop-control.  Btrfs has it's scanning stuff controlled 
by /dev/btrfs-control.  Device mapper has /dev/mapper/control.  This is 
a well established and widely used way for control block based device 
drivers.  Thanks,

Josef