From owner-freebsd-current@FreeBSD.ORG Tue Jun 4 16:13:40 2013 Return-Path: Delivered-To: freebsd-current@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 44727DC3 for ; Tue, 4 Jun 2013 16:13:40 +0000 (UTC) (envelope-from ian@FreeBSD.org) Received: from mho-02-ewr.mailhop.org (mho-02-ewr.mailhop.org [204.13.248.72]) by mx1.freebsd.org (Postfix) with ESMTP id 026261BF3 for ; Tue, 4 Jun 2013 16:13:39 +0000 (UTC) Received: from c-24-8-230-52.hsd1.co.comcast.net ([24.8.230.52] helo=damnhippie.dyndns.org) by mho-02-ewr.mailhop.org with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.72) (envelope-from ) id 1UjtrY-000AYZ-N7; Tue, 04 Jun 2013 16:13:32 +0000 Received: from [172.22.42.240] (revolution.hippie.lan [172.22.42.240]) by damnhippie.dyndns.org (8.14.3/8.14.3) with ESMTP id r54GDRHq015693; Tue, 4 Jun 2013 10:13:28 -0600 (MDT) (envelope-from ian@FreeBSD.org) X-Mail-Handler: Dyn Standard SMTP by Dyn X-Originating-IP: 24.8.230.52 X-Report-Abuse-To: abuse@dyndns.com (see http://www.dyndns.com/services/sendlabs/outbound_abuse.html for abuse reporting information) X-MHO-User: U2FsdGVkX1+eQiLHl493n0CCFmTHW4KW Subject: Re: mounting root from NFS via ROOTDEVNAME From: Ian Lepore To: Craig Rodrigues In-Reply-To: References: <1435656219.2547176.1359645941027.JavaMail.root@erie.cs.uoguelph.ca> <375AA8A5-E385-4528-A460-D4B8FCB9497B@netapp.com> Content-Type: text/plain; charset="us-ascii" Date: Tue, 04 Jun 2013 10:13:27 -0600 Message-ID: <1370362407.1258.110.camel@revolution.hippie.lan> Mime-Version: 1.0 X-Mailer: Evolution 2.32.1 FreeBSD GNOME Team Port Content-Transfer-Encoding: 7bit Cc: Rick Macklem , "Eggert, Lars" , "" X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 04 Jun 2013 16:13:40 -0000 On Mon, 2013-06-03 at 00:06 -0700, Craig Rodrigues wrote: > On Tue, May 28, 2013 at 8:13 AM, Eggert, Lars 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 > > @@ -45,6 +45,7 @@ __FBSDID("$FreeBSD$"); > > > > #include "opt_bootp.h" > > #include "opt_nfs.h" > > +#include "opt_rootdevname.h" > > > > #include > > #include > > @@ -870,8 +871,20 @@ 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 */ > > @@ -1440,6 +1453,16 @@ 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? I'm strongly opposed to removing the ROOTDEVNAME config option, for the simple reason that loader(8) is optional so there has to be a way to set certain basic runtime options at compile time, and ROOTDEVNAME is one of them. There is already a loader tunable for the root dev, vfs.root.mountfrom. Also, there is already a mechanism similar to your proposal above (but a bit more complex) in kern/vfs_mountroot.c. It sets up a list of potential roots based on the flags passed in from the boot(8) stage; ROOTDEVNAME will be near the beginning of the list of the RB_DFLTROOT option (-r at the boot: prompt) is used, and near the end of the list if it isn't. I vaguely remember that this mechanism doesn't work for nfs mounts, but I forget why (hopefully I'll rediscover it as I test Lars' patch this morning). -- Ian