Re: mounting root from NFS via ROOTDEVNAME

On Sun, 2013-06-02 at 19:57 -0400, Rick Macklem wrote:
> Ian Lepore wrote:
> > On Sat, 2013-06-01 at 20:28 -0400, Rick Macklem wrote:
> > > Lars Eggert wrote:
> > > > Hi,
> > > >
> > > > to conclude this thread, the patch below allows one to specify an
> > > > nfs
> > > > rootfs via the ROOTDEVNAME kernel option, which will be mounted
> > > > when
> > > > BOOTP does not return a root-path option.
> > > >
> > > > Lars
> > > >
> > > My only concern with this (mainly because I don't understand the
> > > rules
> > > applied to boot alternatives well enough) is that a few arm configs
> > > have
> > > both BOOTP, BOOTP_NFSROOT and ROOTDEVNAME specified, where
> > > ROOTDEVNAME
> > > is set to "ufs:...". I don't think this patch will break them, since
> > > I
> > > think they'll use NFS and ignore the ROOTDEVNAME, but I'm not
> > > completely
> > > sure. Does someone else know how HL201 boots, for example?
> > >
> > > Lars, if you have a src commit, you can consider this reviewed by
> > > me. If
> > > not, maybe Craig can review it and then I'll commit it.
> > >
> > > Thanks for doing this, rick
> > >
> > > >
> > > > diff --git a/sys/nfs/bootp_subr.c b/sys/nfs/bootp_subr.c
> > > > index 2c57a91..972fb12 100644
> > > > --- a/sys/nfs/bootp_subr.c
> > > > +++ b/sys/nfs/bootp_subr.c
> > > > _at__at_ -45,6 +45,7 _at__at_ __FBSDID("$FreeBSD$");
> > > >
> > > > #include "opt_bootp.h"
> > > > #include "opt_nfs.h"
> > > > +#include "opt_rootdevname.h"
> > > >
> > > > #include <sys/param.h>
> > > > #include <sys/systm.h>
> > > > _at__at_ -870,8 +871,20 _at__at_ bootpc_call(struct bootpc_globalcontext
> > > > *gctx,
> > > > struct thread *td)
> > > > rtimo = time_second +
> > > > BOOTP_SETTLE_DELAY;
> > > > printf(" (got root path)");
> > > > - } else
> > > > + } else {
> > > > printf(" (no root path)");
> > > > +#ifdef ROOTDEVNAME
> > > > + /*
> > > > + * If we'll mount rootfs from
> > > > + * ROOTDEVNAME, we can accept
> > > > + * offers without root paths.
> > > > + */
> > > > + gotrootpath = 1;
> > > > + rtimo = time_second +
> > > > + BOOTP_SETTLE_DELAY;
> > > > + printf(" (ROOTDEVNAME)");
> > > > +#endif
> > > > + }
> > > > printf("\n");
> > > > }
> > > > } /* while secs */
> > > > _at__at_ -1440,6 +1453,16 _at__at_ bootpc_decode_reply(struct nfsv3_diskless
> > > > *nd,
> > > > struct bootpc_ifcontext *ifctx,
> > > >
> > > > p = bootpc_tag(&gctx->tag, &ifctx->reply, ifctx->replylen,
> > > > TAG_ROOT);
> > > > +#ifdef ROOTDEVNAME
> > > > + /*
> > > > + * If there was no root path in BOOTP, use the one in
> > > > ROOTDEVNAME.
> > > > + */
> > > > + if (p == NULL) {
> > > > + p = strdup(ROOTDEVNAME, M_TEMP);
> > > > + if (strcmp(strsep(&p, ":"), "nfs") != 0)
> > > > + panic("ROOTDEVNAME is not an NFS mount point");
> > > > + }
> > > > +#endif
> > > > if (p != NULL) {
> > > > if (gctx->setrootfs != NULL) {
> > > > printf("rootfs %s (ignored) ", p);
> > > >
> > 
> > I've seen several requests over the past year for an nfs ROOTDEVNAME
> > along with BOOTP to work properly from ARM developers (myself
> > included),
> > so I don't think we should worry about breaking existing config that
> > happens to be checked in but a) hasn't been tested by anyone for ages,
> > and b) doesn't work anyway (ROOTDEVNAME just gets ignored).
> > 
> > -- Ian
> > 
> Cool. Thanks. Would you like to review and/or test the above?
> 
> I'll be happy to commit it if Lars doesn't have a src commit bit. (I've
> seen his posts, but can't remember if he is a committer?)
> 
> rick

Well, I started out just testing Lars' patch (it works) but that
inspired me to pick up the work I toyed with months ago in this area, to
try to get BOOTP_NFSROOT to respect other root-path options such as
setting vfs.root.mountfrom in the environment and using the RB_DFLTROOT
boot option.  The attached patch does those things, as follows:

This maintains the historical BOOTP_NFSROOT behavior of panicking on a
failure to mount the root path provided by the server, unless you've
provided an alternative via ROOTDEVNAME or vfs.root.mountfrom.  I was
afraid to change this behavior because it amounts to a bit of a retry
loop that could eventually recover from a transient network or server
problem.

The user can now override the root path from loader(8) even if the
kernel is compiled with BOOTP_NFSROOT.  If vfs.root.mountfrom is set in
the environment it is used unconditionally -- it always overrides the
BOOTP info.  If it begins with [old]nfs: then the BOOTP code uses it
instead of the server-provided info.  If it specifies some other
filesystem then the bootp code will not panic and the code in
vfs_mountroot.c will invoke the right filesystem to do the mount.

If the kernel is compiled with the ROOTDEVNAME option, then that name is
used by the BOOTP code if either 
      * The server doesn't provide a pathname.
      * The boothowto flags include RB_DFLTROOT.  
The latter allows the user to specify an alternate path in ROOTDEVNAME
such as ufs:/dev/da0s1a and boot from that path by setting
boot_dftlroot=1 in loader(8) or using the '-r' option in boot(8).

The one thing not provided here is automatic failover from a
server-provided path to a compiled-in one without the user manually
requesting that.  The code just isn't currently structured in a way that
makes that possible with a lot of rewrite.  I think the ability to set
vfs.root.mountfrom and to use ROOTDEVNAME automatically when the server
doesn't provide a name covers the most common needs.

I'll commit this in a few days unless there are objections or glitches
in testing.

-- Ian