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

[Markus Pargmann <mpa@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 3169 bytes --]

On Sat, Jan 31, 2015 at 08:52:18AM -0500, Paul Clements wrote:
> On Saturday, January 31, 2015, Markus Pargmann <mpa@pengutronix.de> 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.

I think nbd_xmit_timeout only uses task structs and is within the
kernel. There is only a printk which uses the pid.

> Also, see Michal Belcyk's patch for a further improvement to this. They are
> both using pids to kill the hung threads.

Yes there are some occurances of the nbd->pid field, but it seems it is
not essential for the patch. The timeout issues are still on my todo to
reproduce and fix them.

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 |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]