Re: mounting root from NFS via ROOTDEVNAME

On Tue, May 28, 2013 at 8:13 AM, Eggert, Lars <lars_at_netapp.com> 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
>
>
> 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);
>


Sorry for not responding, I've been busy for the past few days.

Thank you for persisting with this, and trying to clean this up.
This is  relatively old code that hasn't been touched in about 15 years,
so not that many people have worked on it.

I don't like things like ROOTDEVNAME which need to be specified in
the kernel config and are not part of the GENERIC kernel.
This usually ends up where code which is behind #ifdef ROOTDEVNAME
will bitrot because not everyone uses it.

I would like to see the ROOTDEVNAME kernel option go away,
in favor of a variable that can be specified in loader.conf.

If I search the code via Opengrok, I with this:
http://bxr.su/s?n=25&start=25&sort=relevancy&q=ROOTDEVNAME&project=FreeBSD

there already seems to be a variable "rootdev" that is checked in a bunch
of places in the loader, so it would be nice if we could use that.

My personal preference  would be to delete ROOTDEVNAME from all the kernel
configs
and deprecate the kernel option, and instead use a tunable variable
which could be set in loader.conf.

This may not be practical.  Do you think it would be doable if
we can have something like this in the kernel code:

const char *rootdevname =
#ifdef ROOTDEVNAME
rootdevname = ROOTDEVNAME;
#else
rootdevname = NULL
#endif

if (rootdevname == NULL) {
        rootdevname = getenv("rootdev");
}


or something like that?
-- 
Craig