git: 2c26d77d989a - main - Remove /boot/efi from mtree, missed in 0b7472b3d8d2.

[Warner Losh]

On Wed, Mar 3, 2021 at 10:21 AM Nathan Whitehorn <nwhitehorn at 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. And
for that I apologize. Thanks for omitting it from the rest of the thread.


> >
> >     The installer *does* mount the partition in advance, so checking
> >     whether
> >     there is a mounted file system is a perfectly reasonable test to
> >     do. We
> >     could also check fstab. I would like to understand what is actually
> >     wrong here first, though. Especially after this misfire -- which is
> >     problematic for reasons that are still not clear to me, since
> >     there are
> >     a number of standard directories in hier(7) not in mtree -- I want to
> >     make sure we actually do have consensus about what is changing and
> >     why.
> >
> >
> > At the top level, we default to having directories in mtree unless
> > 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.
> >
> > But leaving that aside, let's say we wanted to reuse the install 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 think
> > it's a bit fragile to rely only on the directory being present. It
> > should have something mounted there. If you wanted to mkfs_dos + 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 bootloader
> did not also get updated.
>

Yea, it's unclear to me what POLA here is, to be honest. Some of that is
driven by a deep desire not to accidentally update USB drives that have a
bootable image on them as well, so that may overly color my thinking.


> (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, find a
good way to make them callable from those places more easily and start to
move that tribal knowledge back into the base system where it belongs,
imho. Baptiste raised an important point years ago that we also need to
think about doing that with a way to 'plug in' $NEWEST_CLOUD's packages,
containers, layout such that they could provide the details and then the
automation would just work with them 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 one,
I'm willing to see how people react to it in the upgrade path before
changing it again. We should get closure on Ed's proposed change here. I
think it's good and should go in right after your changes. I'd start on
your changes, and give people until the morning to pipe up with any
objections.


> 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 was
> > 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 ESP
and we can then constrain the problem a bit and where we can't 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 errors
> 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 either way
and am having trouble understanding why I was so dead set this morning on a
particular way. Chalk it up to me being a little extra grumpy at surprise
changes.

This one seems less like local policy than /home, but there's still a local
aspect: Do I mount by default, and where. I think we should always, though,
have a fstab entry as we'll need to update it from time to time. Even
Windows has a nominal drive that it uses to mount 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 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 automation of updates to the next level if
we can rely on some basic things.

Warner


>
> >
> > Warner
>
>
>