Re: [Nbd] [RFC 4/4] nbd: Add support for nbd as root device

[Paul C. <pau...@us...>]

On Saturday, January 31, 2015, Markus Pargmann <mp...@pe...> wrote:

> On Fri, Jan 30, 2015 at 06:30:14PM +0100, Wouter Verhelst wrote:
> > On Fri, Jan 30, 2015 at 09:04:00AM +0100, Markus Pargmann wrote:
> > > Hi,
> > >
> > > On Fri, Jan 30, 2015 at 12:42:54AM +0100, Wouter Verhelst wrote:
> > > > Not that I'm opposed to this, but you do realize that doing
> nbd-client
> > > > from initramfs or similar is possible, right? Most initramfs
> > > > implementations these days support it.
> > >
> > > Yes, that was the first idea how to implement a complete netboot for an
> > > embedded ARM device. However, an initramfs is at least around 1MB in
> > > size which has to be loaded using tftp. As the essential nbd-client
> > > connection setup and negotiation is quite small I decided to go with
> > > nbd-root support.
> > >
> > > Also it is quite useful to have nbd-root support much like nfsroot
> > > directly built-in for debugging purposes. It has the big advantage of
> > > booting/testing read-only filesystem images for embedded systems
> without
> > > the need for an initramfs.
> >
> > Fair enough, just thought I'd point it out.
> >
> > When looking at your patch set, two things pop out which you should
> > probably look at:
> > - What will happen if someone boots with root-on-NBD in your scheme and
> >   later does a pivot_root() followed by an NBD_DISCONNECT ioctl on the
> >   device?
>
> Good point. I will look if it works or fix it otherwise.
>
> > - When a connection is started by nbd-client, the kernel creates a "pid"
> >   file in sysfs, which contains the PID of the client and which the
> >   client (when called with -c, or in other cases) uses to verify whether
> >   a device is connected. At first glance, your patch does not do this,
> >   which could cause confusion.
>
> I am actually not quite happy to expose the pid of the task that is
> doing the receive handling through sysfs. As it is already in the code,
> we can't simply remove it. But I think this should be managed by
> userspace if it is necessary at some point. It seems like the pid is
> only used for the connection status?
>
>
The pid is also used to break a hung connection. See nbd_xmit_timeout.
Also, see Michal Belcyk's patch for a further improvement to this. They are
both using pids to kill the hung threads.


> Maybe it would be better to expose the connection status directly and
> deprecate the 'pid' file.
>
> >
> > I must admit I haven't checked your patch very well (my kernel fu isn't
> > that advanced) so I might have missed something, but I'd rather point
> > it out now than have to fix the pieces afterwards ;-)
>
> Yes better to discuss it before things break :).
>
> Thanks,
>
> Markus
>
> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>