Date: Sun, 28 Aug 2022 16:35:33 -0700 From: Kyle Evans <kevans@freebsd.org> To: Peter Jeremy <peterj@freebsd.org> Cc: Ronald Klop <ronald-lists@klop.ws>, freebsd-current@freebsd.org, Ryan Moeller <freqlabs@freebsd.org>, "Patrick M. Hausen" <pmh@hausen.com>, Allan Jude <allanjude@freebsd.org> Subject: Re: Beadm can't create snapshot Message-ID: <CACNAnaEg-=tF2VeauqnigZf3tEtv8A%2B5u%2B3UBUEkXp4C8CwUfw@mail.gmail.com> In-Reply-To: <YwVKFz4rn3h1QUoM@server.rulingia.com> References: <01000182ac3b8593-fb381303-5719-4863-8fda-2530efcab31b-000000@email.amazonses.com> <2818f3da-3ae2-e6e3-9282-8b62263fb5f3@FreeBSD.org> <C4A81AF9-2C11-4931-B5D9-1B257AB583DF@hausen.com> <YwNCQSL5PmvO2nOs@server.rulingia.com> <623263165.219.1661170200563@localhost> <CACNAnaEcr=9Ua8z%2BGC10pOeBAiDhkaOENEA_GQHpu-X1dxRXeQ@mail.gmail.com> <2078216761.314.1661260774009@localhost> <YwVKFz4rn3h1QUoM@server.rulingia.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Aug 23, 2022 at 2:44 PM Peter Jeremy <peterj@freebsd.org> wrote: > > On 2022-Aug-23 15:19:34 +0200, Ronald Klop <ronald-lists@klop.ws> wrote: > >Van: Kyle Evans <kevans@freebsd.org> > >> I was not aware that beadm touches loader.conf, but I find that > >> slightly horrifying. I won't personally make bectl do that, but I > >> guess I could at least document that it doesn't... > > > >Today I looked up something for boot environments myself and read this: = https://wiki.freebsd.org/BootEnvironments#Setting_Boot_Dataset > > > >"In order for boot environments to be effective, you must let the bootfs= zpool property control which dataset gets mounted as the root. Particularl= y, /etc/fstab must be purged of any / mount, and /boot/loader.conf must not= be setting vfs.root.mountfrom directly. " > > > >So it is documented somewhere at least. > > Looking at the wiki history, Kyle wrote that in January 2020. I > wonder if he recalls where that requirement came from. > > I've gone rummaging through the mailing list history and other wiki > pages. It seems that vfs.root.mountfrom used to be required - e.g. > https://lists.freebsd.org/pipermail/freebsd-fs/2011-September/012482.htm= l > https://lists.freebsd.org/pipermail/svn-src-head/2011-October/030641.htm= l > and people wanted to change that - e.g. > https://lists.freebsd.org/pipermail/freebsd-current/2009-October/012933.= html > https://lists.freebsd.org/pipermail/freebsd-fs/2010-March/008010.html > resulting in it becoming optional in May 2012: > https://lists.freebsd.org/pipermail/svn-src-head/2012-May/036902.html > > Based on the quoted wiki entry, it seems that sometime between May > 2012 and January 2020, vfs.root.mountfrom went from "must be set" to > "must not be set" and I can't find anywhere where that is publicised. > This is a serious problem because we now have the situation where > some documentation still says to set vfs.root.mountfrom - e.g. > https://wiki.freebsd.org/RootOnZFS/GPTZFSBoot/Mirror step 2.6 > and people are still using it without being warned that it shouldn't > be used - e.g. the thread starting > https://lists.freebsd.org/pipermail/freebsd-fs/2020-July/028351.html > Allan knows the history here a lot better, but you should basically be looking for where loader started using the 'bootfs' pool property properly. That is the flag day for when vfs.root.mountfrom really shouldn't be set, because loader will derive it correctly from bootfs (via the selector in the menu, if needed). > I've had a look at the beadm source and it preserves/updates > vfs.root.mountfrom if it's present in loader.conf but doesn't add it > if it's not present. > Right, basically they're working around some bad image vendors that still did the wrong thing even as of recent versions. e.g., DigitalOcean never fixed their image to stop setting vfs.root.mountfrom and it drove me absolutely crazy. It's also necessary for bootpool setups, but bootpool setups should be vanishingly rare to the point that the remaining folks with bootpool setups should know what they need to do and probably don't need this. What beadm does is also technically wrong, and I'll complain about that in a minute, too. > IMO, if bectl isn't going to update loader.conf, it needs to warn and > fail if loader.conf contains a vfs.root.mountfrom that points to a > BE that's different to bootfs. (And ideally, a similar check of > /etc/fstab, though beadm doesn't touch that). > Honestly, I'm really not a fan of this idea for a couple of different reasons, but I wouldn't mind updating `bectl check` (probably with an optional `-v` flag) to check for this kind of stuff and warn when it finds something. My main complaint is that if I'm the one writing this patch, I don't want to just check /boot/loader.conf; I want to process it exactly like loader would to reduce false negatives. Perhaps you use /boot/loader.conf, but someone else used /boot/loader.conf.local -- the correct sanity check recursively processes loader_conf_files rooted at /boot/defaults/loader.conf. That brings me back to what beadm does, which bothers me a little bit and I'll probably prod vermaden about it. Rewriting any vfs.root.mountfrom line to the new BE is ~ok, but it completely disregards the contents of the variable. vfs.root.mountfrom can be used for more than just this limited BE selection, so it should really tighten down the scope of which ones it rewrites to values beginning with "zfs:" at the very least.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CACNAnaEg-=tF2VeauqnigZf3tEtv8A%2B5u%2B3UBUEkXp4C8CwUfw>