Page MenuHomeFreeBSD

WIP bootnext: Next Generation
ClosedPublic

Authored by tsoome on Jun 29 2020, 10:23 PM.

Details

Summary

First take on bootnext update.

This work is based on https://github.com/openzfs/zfs/commit/108a454a4604df6ea3be817f3cf076726df2c67a

Test Plan

incidentally it seems to work already:)

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

zfsbootcfg should output the bootonce value when run without any
arguments.

If you could sneak in an update to lib/libbe/libbe.3 to include be_deactivate (between be_activate and be_destroy, I suppose), i'd appreciate it. =)

lib/libbe/be.c
1233 ↗(On Diff #74736)

I somewhat wonder if we should check if bootonce was set and return a special error to indicate that's the case here and let the caller decide if that's a problem or not, but I don't insist and can always revise later if we decide to care.

sbin/bectl/bectl.8
109 ↗(On Diff #74736)

New sentence should start on a new line.

sbin/bectl/bectl.c
74 ↗(On Diff #74736)

Let's bump -T to a new line, bectl activate -T

168 ↗(On Diff #74736)

and check here that argc != 1 && (!reset || argc != 0)

174 ↗(On Diff #74736)

then just pass in NULL for be_deactivate, because ds is unused for this scenario and I think it's misleading to require an argument that we're not going to use. :-)

tsoome added inline comments.
lib/libbe/be.c
1233 ↗(On Diff #74736)

I am not really sure if we should care about if bootonce was set or not; we might just want to ensure we do have sane state on disk -- for example when the bootenv area was not cleared on pool creation, or some unsupported state was stored.

sbin/zfsbootcfg/zfsbootcfg.c
58 ↗(On Diff #74648)

Actually it did not, it did require argc == 2 and only wrote the argument string to pad2:) but the idea is still OK

tsoome marked 2 inline comments as done.

Updates as requested by kevans.

I am happy with the change; thanks!

sbin/bectl/bectl.c
169 ↗(On Diff #74777)

Missing closing parenthesis here

restore version field in bootenv. It does help us to keep old stage2
not to freak on nvlist stream data.

userboot should also set zfs-bootonce environment variable

Add zfs_bootenv.h with key values for nvlist pairs.

make bootonce BE permanenty active, if zfs_bootonce_activate is YES.

libzfsbootenv should have major version 1

quick sweep. This is kinda large and hard to review, so I also suggested a few 'carve outs' that could be committed independently.

Makefile.inc1
2738 ↗(On Diff #75312)

Does this need to be conditional on cddl like the other libraries?

cddl/contrib/opensolaris/cmd/zinject/translate.c
23 ↗(On Diff #75312)

You'll want to make sure that the commit message when this goes in states you are pulling in Delphix code so that people know that any copyright updates by them come from that...

cddl/contrib/opensolaris/lib/libzfs/common/libzfs_pool.c
398 ↗(On Diff #75312)

This change could be peeled off, as it's 100% independent of the rest.

lib/libzfsbootenv/libzfsbootenv.h
33 ↗(On Diff #75312)

Typically in our tree we use
BEGIN_DECLS
and
END_DECLS
for this.

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zfs_bootenv.h
23 ↗(On Diff #75312)

BEGIN_DECL / END_DECL

Rebase on OpenZFS update:
Add support for boot environment data to be stored in the label

tsoome added inline comments.
Makefile.inc1
2738 ↗(On Diff #75312)

Yes. Since we want libzfsbootenv to be integrated to upstream, I need to move it to cddl tree and fix accordingly.

cddl/contrib/opensolaris/cmd/zinject/translate.c
23 ↗(On Diff #75312)

Since the Delphix code is nicely self contained, I did commit it and now we get much cleaner "base".

tsoome marked an inline comment as done.

move libzfsbootenv to cddl tree.

move libzfsbootenv to cddl tree

add nvstore framework and command

defaults/loader.conf should not have nextboot_enable
fix lua
update forth nextboot

keep if statement simple.

add lzbe_flags_t to notify if lzbe_set_boot_device() should add data to
current bootenv, or create new bootenv.

return VB_RAW even when there is no data

DATA_TYPE_BOOLEAN is deprecated

continuation should be indented 4 spaces

rebase after openzfs import

This revision was not accepted when it landed; it landed in state Needs Review.Sep 21 2020, 9:01 AM
This revision was automatically updated to reflect the committed changes.

If src.conf contains
WITHOUT_LOADER_ZFS=true
WITHOUT_ZFS=true

This commit (r365938) will break buildworld.

  • all_subdir_stand ---

/usr/src/stand/efi/loader_4th/../loader/main.c:39:10: fatal error: 'sys/zfs_bootenv.h' file not found
#include <sys/zfs_bootenv.h>

^~~~~~~~~~~~~~~~~~~

My target is ARM with GENERIC conf and src.conf as above.
Sorry if it has already been reported.

If src.conf contains
WITHOUT_LOADER_ZFS=true
WITHOUT_ZFS=true

This commit (r365938) will break buildworld.

  • all_subdir_stand ---

/usr/src/stand/efi/loader_4th/../loader/main.c:39:10: fatal error: 'sys/zfs_bootenv.h' file not found
#include <sys/zfs_bootenv.h>

^~~~~~~~~~~~~~~~~~~

My target is ARM with GENERIC conf and src.conf as above.
Sorry if it has already been reported.

Thanks, should be better now.

Thanks, should be better now.

Thank you, it works fine.