On Thu, Apr 03, 2025 at 08:22:30AM +1300, Simon Glass wrote: > Hi Tom, > > On Thu, 3 Apr 2025 at 03:28, Tom Rini <tr...@konsulko.com> wrote: > > > > On Sat, Mar 15, 2025 at 02:25:58PM +0000, Simon Glass wrote: > > > > > Any 'bootable' flag in a DOS partition causes boostd to only scan > > > bootable partitions for that media. This can mean that extlinux.conf > > > files on the root disk are missed. > > > > > > Put this logic behind a flag and update the documentation. > > > > > > For now, the flag is enabled, to preserve existing behaviour. Future > > > work may provide a command (or some other mechanism) to control this. > > > > > > Signed-off-by: Simon Glass <s...@chromium.org> > > > --- > > > > > > (no changes since v4) > > > > > > Changes in v4: > > > - Rewrite the commit message > > > - Enable the flag by default > > > > > > Changes in v3: > > > - Add new patch to consider non-bootable partitions > > > > > > boot/bootdev-uclass.c | 4 +++- > > > cmd/bootflow.c | 2 +- > > > doc/develop/bootstd/overview.rst | 5 +++-- > > > include/bootflow.h | 2 ++ > > > test/boot/bootdev.c | 1 + > > > test/boot/bootflow.c | 5 +++-- > > > 6 files changed, 13 insertions(+), 6 deletions(-) > > > > Having spent another 10 minutes just now trying to understand things, > > again: > > - The commit message, and the implementation are either in disagreement > > or just too vague. Saying "to preserve existing behaviour" is unclear > > about which behavior is being preserved, since we're setting the flag > > to continue to only check bootable flagged partitions > > (BOOTFLOWIF_ONLY_BOOTABLE). But then we would continue to miss > > extlinux.conf files on root filesystems, which would seem to be the > > bug that needed fixing? > > How about 'to preserve the existing behaviour of bootstd which is to > ignore non-bootable partitions so long as there is at least one > bootable partition on the disk" ?
Yes, that's better. I've reworded the commit message with that.
> > - It's still unclear if this makes bootstd match the exiting distro
> > script behavior or not, and from your other emails it sounded like you
> > were expecting someone else to dig around.
>
> The distro scripts include the code below. My reading of it is that it
> only considers bootable partitions, except that if there is none, then
> it always scans partition 1.
>
> "scan_dev_for_boot_part=" \
> "part list ${devtype} ${devnum} -bootable devplist; " \
> "env exists devplist || setenv devplist 1; " \
> "for distro_bootpart in ${devplist}; do " \
> "if fstype ${devtype} " \
> "${devnum}:${distro_bootpart} " \
> "bootfstype; then " \
> "part uuid ${devtype} " \
> "${devnum}:${distro_bootpart} " \
> "distro_bootpart_uuid ; " \
> "run scan_dev_for_boot; " \
> "fi; " \
> "done; " \
> "setenv devplist\0" \
> \
OK, thanks, I agree with your reading of that as well.
--
Tom
signature.asc
Description: PGP signature
