Date: Thu, 4 Mar 2021 15:05:43 -0500 From: Nathan Whitehorn <nwhitehorn@freebsd.org> To: Warner Losh <imp@bsdimp.com> Cc: Brandon Bergren <bdragon@freebsd.org>, "Rodney W. Grimes" <rgrimes@freebsd.org>, Ed Maste <emaste@freebsd.org>, src-committers <src-committers@freebsd.org>, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: 2c26d77d989a - main - Remove /boot/efi from mtree, missed in 0b7472b3d8d2. Message-ID: <5c413c07-ad22-01e1-ee45-35fbc04a4875@freebsd.org> In-Reply-To: <CANCZdfrbM1cCSpmKSE6TEeqVoGfEsGe6TYZh-yQuM1Gt1KCB=Q@mail.gmail.com> References: <202103031253.123CrxKG051357@gndrsh.dnsmgr.net> <14d09680-1036-4a7e-8a0e-c3063cac8bc9@www.fastmail.com> <dbffbfce-feff-29a0-abce-7d89dbbced7f@freebsd.org> <CANCZdfqh%2BKtueVsmDZh-SCVQeXYc-7f28BCJYJYbUxr-cotbpQ@mail.gmail.com> <6e52fee6-a2fd-584f-757e-e77a8f8ea8eb@freebsd.org> <CANCZdfqLBoDPSxZisf0hsVNo6RM%2BfWGBO_jJ1t4oHe=cTtuoXQ@mail.gmail.com> <91a51b75-9872-d202-53c0-fa1a21dc9cb3@freebsd.org> <CANCZdfrbM1cCSpmKSE6TEeqVoGfEsGe6TYZh-yQuM1Gt1KCB=Q@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 3/3/21 5:25 PM, Warner Losh wrote: > > > On Wed, Mar 3, 2021 at 10:21 AM Nathan Whitehorn=20 > <nwhitehorn@freebsd.org <mailto:nwhitehorn@freebsd.org>> wrote: > > > > On 3/3/21 11:53 AM, Warner Losh wrote: > > > > [clipping non-technical pre-history] > > > Thanks. re-reading it now, I think I was more grumpy than warranted.=20 > And for that I apologize. Thanks for omitting it from the rest of the=20 > thread. No worries, this happens, especially with the pandemic. I know I've=20 definitely been more prickly this year than normal... > > > >=C2=A0 =C2=A0 =C2=A0The installer *does* mount the partition in ad= vance, so checking > >=C2=A0 =C2=A0 =C2=A0whether > >=C2=A0 =C2=A0 =C2=A0there is a mounted file system is a perfectly = reasonable test to > >=C2=A0 =C2=A0 =C2=A0do. We > >=C2=A0 =C2=A0 =C2=A0could also check fstab. I would like to unders= tand what is > actually > >=C2=A0 =C2=A0 =C2=A0wrong here first, though. Especially after thi= s misfire -- > which is > >=C2=A0 =C2=A0 =C2=A0problematic for reasons that are still not cle= ar to me, since > >=C2=A0 =C2=A0 =C2=A0there are > >=C2=A0 =C2=A0 =C2=A0a number of standard directories in hier(7) no= t in mtree -- > I want to > >=C2=A0 =C2=A0 =C2=A0make sure we actually do have consensus about = what is > changing and > >=C2=A0 =C2=A0 =C2=A0why. > > > > > > At the top level, we default to having directories in mtree unles= s > > there's a good reason not to. We disagree as to whether the > installer > > should take the presence or absence of the directory as a strong > > enough reason to do something. I don't think that's a good reason= =2E > > > > But leaving that aside, let's say we=C2=A0wanted to reuse the ins= tall > boot > > part of the installer to update boot blocks as part of > installworld. > > If we can talk through that example w/o it in mtree, then we can > leave > > it out. The last time I worked through this, though, I thought I'= d > > talked myself into needing it. > > > Looking at bootconfig, we could use machdep.bootmethod to > determine if > > we need to update the ESP. If we didn't use that, then the ESP > > shouldn't be touched. This is, at the moment, x86 centric, but > could > > trivially be added to architectures (I'm happy to add it). This > would > > prevent the 'false positive' that's possible in cases where we've= > > installed UEFI then downgraded to BIOS because of some problem > (though > > purely in the context of the installer, I guess this isn't an > issue). > > Even with your approach, we'd bogusly update an ESP (though one > could > > argue you might want that). We could also change the code so that= > > 'unsupported' architectures just didn't update. This is why I thi= nk > > it's a bit fragile to rely only on the directory being present. I= t > > should have something mounted there. If you wanted to mkfs_dos=C2= =A0+ > mkdir > > efi at the top level, you could check for that directory if you > were > > looking for a flag, though that would still update on a BIOS > boot the > > ESP, and prevent false positives if run as part of an update. > > I think we would *want* to update an ESP that is mounted but not > currently being used. If I set up a dual BIOS/EFI-boot system for > some > reason and happened to install an update while booted from BIOS, I > would > be deeply astonished if my configured-by-the-installer EFI bootload= er > did not also get updated. > > > Yea, it's unclear to me what POLA here is, to be honest. Some of that=20 > is driven by a deep desire not to accidentally update USB drives that=20 > have a bootable image on them as well, so that may overly color my=20 > thinking. Agreed on all counts here. > (As an aside, I would also much rather the installer use an update > utility to set up the ESP than have the update utility use the > installer.) > > > Agreed. We can work towards that after the release. It would be better = > if we could accumulate the scripts from a number of different places,=20 > find a good way to make them callable from those places more easily=20 > and start to move that tribal knowledge back into the base system=20 > where it belongs, imho. Baptiste raised an important point years ago=20 > that we also need to think about doing that with a way to 'plug in'=20 > $NEWEST_CLOUD's packages, containers, layout such that they could=20 > provide the details and then the automation would just work with them=20 > too: image building, release customization, boot block update, etc. > > So here's a proposal, now that everyone is in the CC list: > - We add /boot/efi back to mtree, even though I find it kind of > weird to > have it there I think we're too close to the release to have a > conclusion on this. > - We have the installer check for either the ESP directory being an= > active mountpoint or being in the in-progress fstab, whichever is > easiest to implement (they are equivalent for the installer). > > > I'm OK with both of these points. If others are opposed to the first=20 > one, I'm willing to see how people react to it in the upgrade path=20 > before changing it again. We should get closure on Ed's proposed=20 > change here. I think it's good and should go in right after your=20 > changes. I'd start on your changes, and give people until the morning=20 > to pipe up with any objections. Here's a patch to do this: https://reviews.freebsd.org/D29068 It takes several hours to do the full test of building world, building=20 release ISOs, and running them through qemu, so it will be a while yet=20 before I feel comfortable committing. But it's a two-line diff and the=20 pieces worked independently, so the chances it works are pretty high.=20 Comments appreciated. > If that seems OK, I'll post another review for the change. > > > A long-term project I've had has been to try to update the boot > blocks > > as part of installworld or maybe as part of installboot. We have > > really poor reuse as a project in this area. Every little > > orchestration thing wrote its own thing, and all of them have > done it > > badly. I was hoping to be able to reuse this code, or modify the > > installer to use whatever we come up with there. As part of that,= I > > had talked myself into thinking we always needed /boot/efi, but I= 'm > > having trouble reconstructing why that is now though I know it > had to > > do with installed systems and bootstrapping issues... I know I wa= s > > worried about questions about 'why isn't /boot/efi on the system = by > > default so I can mount it' for people that have upgraded, but I > recall > > there was more to it than that. With it in mtree, an installworld= > > (even w/o an ESP update) would create it and people could mount > it w/o > > having to mkdir which they might make as $SOMETHING_ELSE. So I > guess > > that's a bit of a weak reason to absolutely require it in mtree. > > Thanks a lot for the explanation. I'm agreed entirely about the > problem > and the difficulty -- hopefully this set of changes helps at least.= > > > It does. It starts to get people to use the same mount point for the=20 > ESP and we can then constrain the problem a bit and where we can't=20 > constrain it we can parameterize it. > > As for mtree, I was imagining this as something like /home, which > is a > standard part of the system but isn't part of mtree since it > depends on > local-system policy. It's also different from /home in that we > *do* want > it to be a standard place for updates, of course. I think there's > really > not a ton of precedent either way: we don't have any other mount > points > in there for file systems that may or may not exist depending on > circumstances, as far as I can tell. My worry with having it in > mtree is > that having it exist but potentially be a directory rather than an > actual ESP requires that update tools be a little smarter and error= s > will be a little less obvious, since updates that don't pay enough > attention will be a bit more likely to splat files there assuming > there > is an ESP even if makes no sense. It's a weak consideration either > way, > I think. > > > Yea. After a few hours of reflection, I've found that I could go=20 > either way and am having trouble understanding why I was so dead set=20 > this morning on a particular way. Chalk it up to me being a little=20 > extra grumpy at surprise changes. > > This one seems less like local policy than /home, but there's still a=20 > local aspect: Do I mount by default, and where. I think we should=20 > always, though, have a fstab entry as we'll need to update it from=20 > time to time. Even Windows has a nominal drive that it uses to mount=20 > the ESP, even if it isn't mounted by default. That's used to update it = > when scripts and such need to do that (or if you're the victim of an=20 > upgrade script that did too much that now needs to be undone). I think = > we should be similar in that regard. This would also let us take the=20 > automation of updates to the next level if we can rely on some basic=20 > things. That makes sense to me. There's also still the issue of non-EFI systems, = that differ only by install-time configuration from non-EFI systems. One = of my worries of having /boot/efi always exist is that a non-EFI system=20 may try to "update" the EFI by poking around in the empty /boot/efi and=20 think it has updated/installed something useful but has in reality done=20 nothing. But it's a tricky situation all around. -Nathan > > Warner > > > > > > Warner > >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5c413c07-ad22-01e1-ee45-35fbc04a4875>