From owner-dev-commits-src-all@freebsd.org Wed Mar 3 22:25:29 2021 Return-Path: Delivered-To: dev-commits-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id AE61D552E33 for ; Wed, 3 Mar 2021 22:25:29 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-qk1-x735.google.com (mail-qk1-x735.google.com [IPv6:2607:f8b0:4864:20::735]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4DrT8n4Hrlz3Qvx for ; Wed, 3 Mar 2021 22:25:29 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-qk1-x735.google.com with SMTP id g185so6527865qkf.6 for ; Wed, 03 Mar 2021 14:25:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=zD0mdRv+v11ua6jZjZU6vAihX6p/414yPJa8ABaN50w=; b=llgrDZiUYXU1PWFAK42NLlELjvcfwqHfOollg0ShE69lieuSt5hoFiSESiGJRE1vUn 1blnghK1CUAtHzIL3F4ZyAB+W9Zs3FHmhKhAC0PcFjmh5BS+BTX5zsDW72lP4cO6mO++ GzJurBPZpKKI5Qeer8DxbOB9rI4+q+3IFrBcSKssJZAv4nUkDPBfot/hZSF3+tv/80up zC8d4UuM74kgsiWoy/UUdXrauANcP3c2bLZzhmj7ye1NaePiMYfzJxDBODCrd971iocq LqOqsEPngngX1EajPG8RDJSjnskWk0d85vd90CjjbWaMHbZtk5JKwZQAd6q9NvXCWxSz Tg/A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=zD0mdRv+v11ua6jZjZU6vAihX6p/414yPJa8ABaN50w=; b=qxVbhOnv1b5bkd7DkEMJ0+q41LT8AupI6h72hxNEUx2sTPrWw4hGj43SXD4sylxkoH g1qilzzWXRqm5RKsLHC2zQHDOoJ/MEea9qFr1gJcA9HxAW/jkxePyUR63TTwQuFu62Dy oh3YeX2LC/3HGgxhC4HKXYwgY5La2WCYVqtj4WakiFgymeSjyV/FDLqjcqn44kr0ig5L gvKtG4rZ3XyUnshH3GkCuk1CtiTTpVYm/WNiaKZtCOWXAeO7iCyWnAhMLtloRii9PQ0K MDwrS3ZDWswjb4TtU7uy7HpSYT1FQzE+aJKfopiXfZdg7NP3473CKJoHg7DLPzvzWxur Wlcw== X-Gm-Message-State: AOAM531CgwFdQMBzeZR9LP57WNt5uQRuexLkaSVqhG9UNmOE9gaQpUnN ooBwvK9jdZaq6vUHhf9PNJ58GemOqPzdEOy9nfN6+g== X-Google-Smtp-Source: ABdhPJx0UqejeeifdeJhDh0Wvmb+5L2XDY+G5VVsN6LehnJmTXBjh0d+uhuW7+RWuLXVd6a/cB5Q0FMcF6DDjoR0k74= X-Received: by 2002:a37:7f83:: with SMTP id a125mr1304227qkd.44.1614810328611; Wed, 03 Mar 2021 14:25:28 -0800 (PST) MIME-Version: 1.0 References: <202103031253.123CrxKG051357@gndrsh.dnsmgr.net> <14d09680-1036-4a7e-8a0e-c3063cac8bc9@www.fastmail.com> <6e52fee6-a2fd-584f-757e-e77a8f8ea8eb@freebsd.org> <91a51b75-9872-d202-53c0-fa1a21dc9cb3@freebsd.org> In-Reply-To: <91a51b75-9872-d202-53c0-fa1a21dc9cb3@freebsd.org> From: Warner Losh Date: Wed, 3 Mar 2021 15:25:17 -0700 Message-ID: Subject: Re: git: 2c26d77d989a - main - Remove /boot/efi from mtree, missed in 0b7472b3d8d2. To: Nathan Whitehorn Cc: Brandon Bergren , "Rodney W. Grimes" , Ed Maste , src-committers , "" , dev-commits-src-main@freebsd.org X-Rspamd-Queue-Id: 4DrT8n4Hrlz3Qvx X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.34 X-BeenThere: dev-commits-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for all branches of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 03 Mar 2021 22:25:29 -0000 On Wed, Mar 3, 2021 at 10:21 AM Nathan Whitehorn 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 > > >