From owner-freebsd-current@FreeBSD.ORG Thu Jun 6 16:54:44 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 0737E32C for ; Thu, 6 Jun 2013 16:54:44 +0000 (UTC) (envelope-from ian@FreeBSD.org) Received: from mho-01-ewr.mailhop.org (mho-03-ewr.mailhop.org [204.13.248.66]) by mx1.freebsd.org (Postfix) with ESMTP id C07371DE0 for ; Thu, 6 Jun 2013 16:54:43 +0000 (UTC) Received: from c-24-8-230-52.hsd1.co.comcast.net ([24.8.230.52] helo=damnhippie.dyndns.org) by mho-01-ewr.mailhop.org with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.72) (envelope-from ) id 1UkdSP-0007GL-GE; Thu, 06 Jun 2013 16:54:37 +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 r56GsW5C018186; Thu, 6 Jun 2013 10:54:32 -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/GtuP4EcQwU8+l6rSg/CAJ Subject: Re: mounting root from NFS via ROOTDEVNAME From: Ian Lepore To: Rick Macklem In-Reply-To: <433498856.127008.1370217441419.JavaMail.root@erie.cs.uoguelph.ca> References: <433498856.127008.1370217441419.JavaMail.root@erie.cs.uoguelph.ca> Content-Type: multipart/mixed; boundary="=-yNeznRJDq9vY4abI0Sra" Date: Thu, 06 Jun 2013 10:54:32 -0600 Message-ID: <1370537672.1086.30.camel@revolution.hippie.lan> Mime-Version: 1.0 X-Mailer: Evolution 2.32.1 FreeBSD GNOME Team Port Cc: Craig Rodrigues , freebsd-current , Lars Eggert 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: Thu, 06 Jun 2013 16:54:44 -0000 --=-yNeznRJDq9vY4abI0Sra Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 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 > > > > @@ -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); > > > > > > > > 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 --=-yNeznRJDq9vY4abI0Sra Content-Disposition: inline; filename="nfsroot.diff" Content-Type: text/x-patch; name="nfsroot.diff"; charset="us-ascii" Content-Transfer-Encoding: 7bit Index: sys/nfs/bootp_subr.c =================================================================== --- sys/nfs/bootp_subr.c (revision 251469) +++ sys/nfs/bootp_subr.c (working copy) @@ -45,6 +45,7 @@ __FBSDID("$FreeBSD$"); #include "opt_bootp.h" #include "opt_nfs.h" +#include "opt_rootdevname.h" #include #include @@ -55,6 +56,7 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include #include #include @@ -167,6 +169,7 @@ struct bootpc_tagcontext { struct bootpc_globalcontext { STAILQ_HEAD(, bootpc_ifcontext) interfaces; u_int32_t xid; + int any_root_overrides; int gotrootpath; int gotgw; int ifnum; @@ -865,13 +868,14 @@ bootpc_call(struct bootpc_globalcontext *gctx, str BOOTP_SETTLE_DELAY; } else printf(" (ignored)"); - if (ifctx->gotrootpath) { + if (ifctx->gotrootpath || + gctx->any_root_overrides) { gotrootpath = 1; rtimo = time_second + BOOTP_SETTLE_DELAY; - printf(" (got root path)"); - } else - printf(" (no root path)"); + if (ifctx->gotrootpath) + printf(" (got root path)"); + } printf("\n"); } } /* while secs */ @@ -1371,7 +1375,7 @@ static void bootpc_decode_reply(struct nfsv3_diskless *nd, struct bootpc_ifcontext *ifctx, struct bootpc_globalcontext *gctx) { - char *p; + char *p, *s; unsigned int ip; ifctx->gotgw = 0; @@ -1438,8 +1442,28 @@ bootpc_decode_reply(struct nfsv3_diskless *nd, str } } - p = bootpc_tag(&gctx->tag, &ifctx->reply, ifctx->replylen, + /* + * 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. + */ + p = NULL; + if ((s = getenv("vfs.root.mountfrom")) != NULL) { + if ((p = strstr(s, "nfs:")) != NULL) + p = strdup(p + 4, M_TEMP); + freeenv(s); + } + if (p == NULL) { + p = bootpc_tag(&gctx->tag, &ifctx->reply, ifctx->replylen, TAG_ROOT); + } +#ifdef ROOTDEVNAME + if ((p == NULL || (boothowto & RB_DFLTROOT) != 0) && + (p = strstr(ROOTDEVNAME, "nfs:")) != NULL) { + p += 4; + } +#endif if (p != NULL) { if (gctx->setrootfs != NULL) { printf("rootfs %s (ignored) ", p); @@ -1544,6 +1568,17 @@ bootpc_init(void) gctx->starttime = time_second; /* + * If ROOTDEVNAME is defined or vfs.root.mountfrom is set then we have + * root-path overrides that can potentially let us boot even if we don't + * get a root path from the server, so we can treat that as a non-error. + */ +#ifdef ROOTDEVNAME + gctx->any_root_overrides = 1; +#else + gctx->any_root_overrides = testenv("vfs.root.mountfrom"); +#endif + + /* * Find a network interface. */ CURVNET_SET(TD_TO_VNET(td)); @@ -1652,19 +1687,10 @@ retry: bootpc_compose_query(ifctx, td); error = bootpc_call(gctx, td); - if (error != 0) { -#ifdef BOOTP_NFSROOT - panic("BOOTP call failed"); -#else printf("BOOTP call failed\n"); -#endif } - rootdevnames[0] = "nfs:"; -#ifdef NFSCLIENT - rootdevnames[1] = "oldnfs:"; -#endif mountopts(&nd->root_args, NULL); STAILQ_FOREACH(ifctx, &gctx->interfaces, next) @@ -1672,7 +1698,7 @@ retry: bootpc_decode_reply(nd, ifctx, gctx); #ifdef BOOTP_NFSROOT - if (gctx->gotrootpath == 0) + if (gctx->gotrootpath == 0 && gctx->any_root_overrides == 0) panic("bootpc: No root path offered"); #endif @@ -1699,9 +1725,16 @@ retry: error = md_mount(&nd->root_saddr, nd->root_hostnam, nd->root_fh, &nd->root_fhsize, &nd->root_args, td); - if (error != 0) - panic("nfs_boot: mountd root, error=%d", error); - + if (error != 0) { + if (gctx->any_root_overrides == 0) + panic("nfs_boot: mount root, error=%d", error); + else + goto out; + } + rootdevnames[0] = "nfs:"; +#ifdef NFSCLIENT + rootdevnames[1] = "oldnfs:"; +#endif nfs_diskless_valid = 3; } Index: sys/kern/vfs_mountroot.c =================================================================== --- sys/kern/vfs_mountroot.c (revision 251469) +++ sys/kern/vfs_mountroot.c (working copy) @@ -714,8 +714,8 @@ parse_mount(char **conf) goto out; } - if (strcmp(fs, "zfs") != 0 && dev[0] != '\0' && - !parse_mount_dev_present(dev)) { + if (strcmp(fs, "zfs") != 0 && strstr(fs, "nfs") == NULL && + dev[0] != '\0' && !parse_mount_dev_present(dev)) { printf("mountroot: waiting for device %s ...\n", dev); delay = hz / 10; timeout = root_mount_timeout * hz; --=-yNeznRJDq9vY4abI0Sra--