Re: mounting root from NFS via ROOTDEVNAME

On Thu, Jun 6, 2013 at 9:54 AM, Ian Lepore <ian_at_freebsd.org> wrote:

> 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:
>

Your patch is better than the existing code.  Your patch makes the code
more readable.

In my earlier response on this thread, I threw out the idea of axing
ROOTDEVNAME from the kernel config options.
I had a feeling that there would be opposition to it, so I'm OK with it
staying.
What I don't like to see is a lot of code behind conditional preprocessor
logic of #ifdef ROOTDEVNAME.
This often leads to bitrot if ROOTDEVNAME is not part of the GENERIC kernel
config.

However, with your patch, the amount of code inside #ifdef ROOTDEVNAME
blocks
is very minimal, so I can live with that.

With your patch, we can trigger all the behavior by setting vfs.mountfrom
in loader.conf with an "nfs:" prefix,
so that is good, in terms of testing the feature with GENERIC kernels.

I would say, if Lars Eggert has time to provide feedback and is OK with
your patch, then go with your patch and commit it.

The only minor feedback I would give is that in this comment:

+    /*
+     * Choose a root filesystem.  If a value is forced in the environment
+     * and it contains "nfs:", use it unconditionally.  Otherwise, use
+     * ROOTDEVNAME if it contains "nfs:" and either the server didn't
+     * provide a name or the boot options say to force ROOTDEVNAME.
+     */
+

If you could take the sentence starting with "Otherwise, use",
and maybe split it up into two sentences, that would make the comment
a little bit more readable.

Thanks.


--
Craig