This won't fly since an exact argc check breaks the case where a disk
and an explicit first stage is given while for the second stage the
default is used:
installboot sd0 /usr/mdec/biosboot
Also, the following mistyped/bogus usages are still argc-valid regarless
of this diff:
installboot /usr/mdec/biosboot
So I'll just drop this futile argc-based attempt.
The real problem seems to be that opendev(3) happily opens a regular
file as device, but only if the argument contains a slash:
# installboot -v ./biosboot
Using / as root
installing bootstrap on ./biosboot
using first-stage /usr/mdec/biosboot, second-stage /usr/mdec/boot
# installboot -v biosboot
installboot: open: /dev/rbiosboot: No such file or directory
This points at the same file and "biosboot" is obviously neither a short
form device name nor a DUID.
I'll look into that soon.
On Sat, Aug 20, 2022 at 06:36:09AM +0000, Klemens Nanni wrote:
> Just forgot to pass the disk during sparc64 tests and was surprised to
> see it treating a file as disk:
>
> # installboot -nv /usr/mdec/bootblk /ofwboot.test
> Using / as root
> would install bootstrap on ./usr/mdec/bootblk
> using first-stage ./ofwboot.test, second-stage /usr/mdec/ofwboot
> boot block is 120696 bytes (236 blocks @ 512 bytes = 120832 bytes)
> installboot: boot blocks too big (120832 > 7680)
>
> Same on amd64:
>
> $ installboot -nv /usr/mdec/biosboot /usr/mdec/boot
> Using / as root
> would install bootstrap on /usr/mdec/biosboot
> using first-stage /usr/mdec/boot, second-stage /usr/mdec/boot
> installboot: /usr/mdec/boot: 2 ELF load sections (only support 1)
>
> Require an exact argc match instead of erroring out too many args alone:
>
> $ ./obj/installboot -nv /usr/mdec/biosboot /usr/mdec/boot
> usage: installboot [-nv] [-r root] disk [stage1 [stage2]]
> installboot [-nv] -p disk
>
>
> This problem exists on all platforms, but EFI (armv7, arm64, risc64)
> currently suffers from another bug regarding an explicit stage file:
>
> $ installboot -nv sd0 /usr/mdec/BOOTAA64.EFI
> usage: installboot [-nv] [-r root] disk [stage1]
> installboot [-nv] -p disk
>
> There md_init() does not yet set `stages' (relevant for this argc fix)
> and `stage1', so `stage' remains zero-initialised, effectively always
> requiring exactly one argument (the disk).
>
> That's stuff for another diff and luckily the argc fix below does not
> change behaviour for the only currently working usage on EFI:
>
> # doas installboot -nv sd0
> Using / as root
> would install bootstrap on /dev/rsd0c
> would copy /usr/mdec/BOOTAA64.EFI to
> /tmp/installboot.7w8t7zd34s/efi/boot/bootaa64.efi
> would write /tmp/installboot.7w8t7zd34s/efi/boot/startup.nsh
> # doas ./obj/installboot -nv sd0
> Using / as root
> would install bootstrap on /dev/rsd0c
> would copy /usr/mdec/BOOTAA64.EFI to
> /tmp/installboot.OMturEqYaM/efi/boot/bootaa64.efi
> would write /tmp/installboot.OMturEqYaM/efi/boot/startup.nsh
>
>
> The argc checks now read like I would read the synopsis, making it much
> easier to read and understand than the current code:
> 1. -p?
> a. no -r?
> b. exactly one arg (disk)?
> 2. else
> a. exactly one arg (disk)?
> b. exactly one+stages args (disk + stage1 [+ stage2])?
>
>
> Pull stage1/stage2 assignments into 2. to clarify things further.
>
> This fixes the bogus case I hit and keeps existing valid usages working.
>
> Feedback? OK?
>
>
> Index: installboot.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/installboot/installboot.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 installboot.c
> --- installboot.c 19 Aug 2022 08:27:48 -0000 1.15
> +++ installboot.c 20 Aug 2022 06:29:49 -0000
> @@ -75,10 +75,13 @@ main(int argc, char **argv)
> argc -= optind;
> argv += optind;
>
> - if (argc < 1 || argc > stages + 1)
> - usage();
> - if (prepare && (root != NULL || argc > 1))
> - usage();
> + if (prepare) {
> + if (root != NULL || argc != 1)
> + usage();
> + } else {
> + if (argc != 1 && argc != stages + 1)
> + usage();
> + }
>
> dev = argv[0];
> if (argc > 1)
>
