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