zfsbootcfg: a simple tool to set next boot (one time) options for zfsboot

[avg]

Summary

zfsboot reads one time boot directives from ZFS pool Pad2 area.
The area was previously described as "Boot Block Header", but currently
it is know as Pad2, marked as reserved and is zeroed out on pool creation.
The new code interprets data in this area, if any, using the same format
as boot.config. The area is immediately wiped out.
Failure to parse the directives results in a reboot right after
the wipe out. Otherwise the boot sequence proceeds as usual.

zfsbootcfg writes zfsboot arguments specified on the command line to the Pad2
area of a disk identified by vfs.zfs.boot.primary_pool and
vfs.zfs.boot.primary_vdev kenv variables that are set by loader during boot.

[jhb]

The only thing that stuck out to me is that 'nextboot' on FreeBSD doesn't select alternate partitions/filesystems (boot0cfg had a way to do that for boot0, and gpart can set 'bootme', etc. flags for GPT that do something like this in conjunction with gptboot(8)). 'nextboot' allows you to choose an alternate kernel from the current filesystem for the next boot (but falling back to the 'stock' kernel if it has to reboot due to crash on boot, etc.). To that end, I wish there was a different name that avoided conflating the different semantics of nextboot vs znextboot. I can't think of one off the top of my head.

Code wise the limited changes I saw in sys/boot looked ok. Matt will have to speak to the ZFS-related changes I imagine.

[avg]

In D7612#158342, @jhb wrote:

To that end, I wish there was a different name that avoided conflating the different semantics of nextboot vs znextboot. I can't think of one off the top of my head.

Perhaps something like zfsbootcfg(following the same style as boot0cfg) ?

[jhb]

In D7612#158345, @avg wrote:

In D7612#158342, @jhb wrote:

To that end, I wish there was a different name that avoided conflating the different semantics of nextboot vs znextboot. I can't think of one off the top of my head.

Perhaps something like zfsbootcfg(following the same style as boot0cfg) ?

That would work for me. Thanks!

[avg]

rename to zfsbootcfg

[avg]

allow empty argument to wipe out the next boot options

[avg]

add a basic manual page

[rpokala]

sbin/zfsbootcfg/Makefile

14

ISTR seeing something on the lists that ${.CURDIR}/../../ should be avoided, and to use ${SRCTOP} instead?

sbin/zfsbootcfg/zfsbootcfg.8

39

This should be either

The
.Nm
utility is used to set

OR

.Nm
is used to set

45

the next time the machine is booted.

76

without changing the

80

s/znextboot/zfsbootcfg/

(or use .Nm? I'm not sure if that works in this context...?)

84

Same as above.

91

.Fx 12.0 .

98

uses the

[emaste]

sbin/zfsbootcfg/Makefile

14

I'm not sure there's been an explicit mention, but ${SRCTOP} is indeed preferred in new Makefiles.

[avg]

• use SRCTOP
• manual page fixes suggested by rpokala

[avg]

sbin/zfsbootcfg/zfsbootcfg.8

91

I was not sure what to put here.
Will I have to change the version when I MFC the change to stable/XX ?

[wblock]

sbin/zfsbootcfg/zfsbootcfg.8

96

Needs a comma after "moment"

112

"be able to" is not needed.

[tsoome]

There is an issue:) The idea is nice and will complement the freebsd logic, but You are missing EFI version. I think UEFI and BIOS versions should be kept as functionally close as possible, especially as one can have both variants in the same system, and select based on boot device name...

[tsoome]

Note about tooling, for illumos port I'll definitely go for "beadm activate -t bename" as it will keep the user experience logic, but since current beadm in freebsd is script, you need something to actually set the data. Also as there is an work to port beadm/libbe bits, its still something to think about.

[tsoome]

cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h

216

It would be nice to have get/set nextboot interface, so we could have an way to verify the nextboot setup.

[tsoome]

I hope Matt can give more/better advice, but I have this nagging feeling about label updates in kernel...

First of all, we have nice generic label update mechanism, I think, it should be possible to put it into the use there, and avoid custom interface for specific field.

What about something like this:

When nextboot is set (SET ioctl), we update "pad2" data field and sync pool/vdev labels on disk, note, that we have 4 labels per disk anyhow, so we can get *all* the label instances in place.

Now from loader, when pool is probed, we can fetch the pad2 (need better name there;), while probing, and as soon as we have determined the best UB, we can either nuke the pad2 or pass the location back to boot2 for nuking. Nuking in place would even be nice as it will happen inside zfs module and therefore we do not need to duplicate the implementation for bios/uefi (or other platforms). So the nextboot is reset, and on reboot, the normal bootfs property logic will take over.

When kernel is starting and opening the boot pool, it can apply additional label sync with clean pad2, to make sure the nextboot is reset for all label copies.

I think this might help to reduce down the corner cases, also perhaps could simplify the possible get/set interface; I would still like to see the get interface because whenever I perform the changes, I do like to verify if things are indeed properly set (guess I'm not alone there:) - for example even after beadm activate, I usually do verify the result from beadm list;)

Anyhow, those are just the ideas, perhaps flawed ones, perhaps useful.

[avg]

improve the man page based on the wblock's comments

[avg]

@tsoome I hope it's okay if I take your comments as suggestions for the future enhancements rather than blockers.

EFI is a relatively new thing, I am not very familiar with the code, I do not have any systems with it, and I hope that something better (more flexible and convenient) could be done for EFI using its capabilities.

'Get' interface would be, of course, very nice to have, but for now, if you are unsure what's configured for the next boot, you can simply overwrite or clear it.

Regarding the higher level tools. Yep, they could use zfsbootcfg or the new libzfs function or the new ioctl directly depending on their language and what's more convenient for them. I do not see any blockers here.

With regard to using all labels, I need to think about that. I wanted to keep the code very simple for now. Your suggestion sounds good to me and it does not seem hard to implement. Just need to think about it more.

Thank you!

[tsoome]

In D7612#168708, @avg wrote:

@tsoome I hope it's okay if I take your comments as suggestions for the future enhancements rather than blockers.

EFI is a relatively new thing, I am not very familiar with the code, I do not have any systems with it, and I hope that something better (more flexible and convenient) could be done for EFI using its capabilities.

'Get' interface would be, of course, very nice to have, but for now, if you are unsure what's configured for the next boot, you can simply overwrite or clear it.

Regarding the higher level tools. Yep, they could use zfsbootcfg or the new libzfs function or the new ioctl directly depending on their language and what's more convenient for them. I do not see any blockers here.

With regard to using all labels, I need to think about that. I wanted to keep the code very simple for now. Your suggestion sounds good to me and it does not seem hard to implement. Just need to think about it more.

Thank you!

Absolutely, I was trying to iterate about ideas I got, and perhaps not all are possible at freebsd context, at least not immediately . And once you have first iteration, it can be improved/redone etc.

[wblock]

sbin/zfsbootcfg/zfsbootcfg.8

33

This should probably just be "next boot".

51

If booting fails,

52

Passive -> active
s/will automatically revert/automatically reverts/

No need for the possessive "its" here, just "the".

53

boot configuration for the next boot.

58

read the boot option information from the first disk found in the first

59

ZFS pool found.

[avg]

man pages fixes suggested by wblock

[avg]

@wblock thank you!

[wblock]

In D7612#170785, @avg wrote:

@wblock thank you!

You're welcome, but none of them seem to have changed the source.

[avg]

In D7612#171482, @wblock wrote:

In D7612#170785, @avg wrote:

@wblock thank you!

You're welcome, but none of them seem to have changed the source.

Apologies, I failed to understand this comment... could you please explain?

[wblock]

In D7612#171485, @avg wrote:

In D7612#171482, @wblock wrote:

In D7612#170785, @avg wrote:

@wblock thank you!

You're welcome, but none of them seem to have changed the source.

Apologies, I failed to understand this comment... could you please explain?

They were all marked done, but without any changes to the man page source. This might be Phabricator "helping" out, or a not-updated diff, or just that they were considered but rejected.

[avg]

@wblock I remember making them and updating the diff.
I look at this https://reviews.freebsd.org/D7612?vs=21017&id=21295&whitespace=ignore-most#toc and I see the changes.

[wblock]

In D7612#171511, @avg wrote:

@wblock I remember making them and updating the diff.
I look at this https://reviews.freebsd.org/D7612?vs=21017&id=21295&whitespace=ignore-most#toc and I see the changes.

That did show some of them. I went back through and checked them. Thanks!

sbin/zfsbootcfg/zfsbootcfg.8

33

The suggestion was to s/reboot/boot/, not remote "the". So for this line, it should be:

.Nd "specify zfsboot options for the next boot"

41

It's hard to say how to mark this up. The sentence probably should read (rendered) as

zfsbootcfg is used to set boot.config(5)-style options to be used by

This markup renders okay, but might not be the right way to do it:

.Nm
is used to set
.Xr boot.config 5 Ns -style
options to be used by

51

Missing a comma for the pause after "fails":

If booting fails, the machine automatically reverts to the previous

52

Probably don't need "boot" again here:

configuration for the next boot.

57

read the boot option information from the first disk found in the first

[avg]

more man page fixes for things that I misunderstood or missed.
suggested by wblock

[avg]

sbin/zfsbootcfg/zfsbootcfg.8

33

Sorry for my misunderstanding, will fix.

[wblock]

Thank you!