Change the way efibootmgr works by specifying bootnum via -b parameter

[bcran]

Summary

Instead of passing the bootnum to each different parameter, require users
to specify -b when running operations that need a bootnum.

This allows activation of a new boot entry at same same time it's created
by adding -a onto the command line.

This is obviously a breaking change.

Sponsored by: Netflix

[bcran]

Update bsdinstall/bootconfig to use the -a parameter when creating the boot entry.
Fix some typos.
Try and improve the error message when EFI variables aren't supported: use err() to
print the underlying error.

[imp]

Be sure to commit these separately.

[imp]

I went back and forth on this when I was writing the code. I thought it was a more natural interface to say --activate #, --deactivate #, etc, rather that --activate --bootnum #. I see the use-case, though. It could be handled by a second invocation, but the original code was already confused over what set_active means (in some places using it as a boolean, other places using it and the boot number). So I think on the whole this is good, modulo my comments.

I'm also not that I'm super stoked about this interface. It's too old school. We should have something more like nvmecontrol, et al, that does efibootmgr create <create-args>, efibootmgr activate -b XXXXX <args>, etc. But that will have to wait for another day (and likely need the current setup around for backwards compat for a release or so).

usr.sbin/efibootmgr/efibootmgr.c

98 ↗

(On Diff #52281)

ditto?

103 ↗

(On Diff #52281)

does this need no_argument too?

105–106 ↗

(On Diff #52281)

ditto?

356 ↗

(On Diff #52281)

I'd move this to parse_args, with the other sanity checks like this.
I'd also trigger usage there as well to be consistent.

496 ↗

(On Diff #52281)

ditto

910 ↗

(On Diff #52281)

I don't like this change. It's not at all related, and it makes the error message worse, not better.

917 ↗

(On Diff #52281)

gratuitous {} added.

[bcran]

usr.sbin/efibootmgr/efibootmgr.c

356 ↗

(On Diff #52281)

I realized that after I've submitted the review.

910 ↗

(On Diff #52281)

Agreed, I'll revert it.

[bcran]

Remove unrelated changes, move error handling.
I'll fix the typos in a separate commit

[imp]

usr.sbin/efibootmgr/efibootmgr.c

178 ↗

(On Diff #52296)

add [-a] here too.

[bcran]

Add [-a] to create usage text

[bcran]

In D18648#397712, @imp wrote:

I went back and forth on this when I was writing the code. I thought it was a more natural interface to say --activate #, --deactivate #, etc, rather that --activate --bootnum #. I see the use-case, though. It could be handled by a second invocation, but the original code was already confused over what set_active means (in some places using it as a boolean, other places using it and the boot number). So I think on the whole this is good, modulo my comments.

Yeah, I agree that it's a more natural interface the way it is, but given the Linux efibootmgr requires the -b parameter and activating the entry with a second invocation is rather clunky (especially in scripts) I think this change makes sense.

I'm also not that I'm super stoked about this interface. It's too old school. We should have something more like nvmecontrol, et al, that does efibootmgr create <create-args>, efibootmgr activate -b XXXXX <args>, etc. But that will have to wait for another day (and likely need the current setup around for backwards compat for a release or so).

At least it's mostly compatible with the Linux efibootmgr parameters, which I presume isn't accidental.

[bcran]

In D18648#397711, @imp wrote:

Be sure to commit these separately.

I will do.

[imp]

Works for me. Again, I think it was a mistake to have flags for both verbs and options, but that's what linux did and will need to be fixed in a separate commit.