Page MenuHomeFreeBSD

stand/zfs: Add all bootenvs to environment
ClosedPublic

Authored by kevans on Feb 19 2018, 2:52 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mar 18 2024, 7:38 AM
Unknown Object (File)
Feb 5 2024, 12:05 PM
Unknown Object (File)
Jan 28 2024, 1:19 AM
Unknown Object (File)
Dec 22 2023, 10:42 PM
Unknown Object (File)
Nov 1 2023, 1:51 PM
Unknown Object (File)
Sep 1 2023, 10:39 AM
Unknown Object (File)
Aug 2 2023, 10:08 AM
Unknown Object (File)
Jul 15 2023, 4:37 PM
Subscribers

Details

Summary

For the benefit of lualoader, add all bootenvs to environment when init_zfs_bootenv is invoked. All of the boot environment logic can then be implemented in pure lua, rather than going back and forth with C to implement paging.

This stores all boot environments in bootenvs[idx] and the final count of bootenvs in bootenvs_count.

While here, make a copy of currdev for init_zfs_bootenv since it will be modifying it and the caller may not necessarily want that. Some of the logic was shifted around so that the 'currdev' pointer remains at the beginning of the string and 'beroot' is moved around as needed to modify it or ultimately store it in zfs_be_root.

Tested with: lualoader (and local changes to add boot environment support)
Tested with: forth

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Not really familiar with ZFS or BEs in general. Looks good modulo a few minor style nits.

stand/zfs/zfs.c
789 ↗(On Diff #39493)

Suggest using strlcpy, and dropping the following = 0;.

Or just strdup() instead of this entire sequence, assuming it exists in libstand.

801 ↗(On Diff #39493)

error return value is ignored

814–815 ↗(On Diff #39493)

This is a little odd. Why not pass it as a parameter?

826–827 ↗(On Diff #39493)

Suggest using strlcpy

830 ↗(On Diff #39493)

style(9) nit about comparing NULL instead of treating it as a boolean :-)

840 ↗(On Diff #39493)

Does this work? I guess loader environment keys are arbitrary C strings.

This revision is now accepted and ready to land.Feb 19 2018, 3:15 PM
kevans marked 4 inline comments as done.

Address concerns noted by @cem.

This revision now requires review to proceed.Feb 19 2018, 3:27 PM
stand/zfs/zfs.c
801 ↗(On Diff #39493)

I'm actually just going to drop the error return on this one. There's nothing init_zfs_bootenv can do about it, lua's just going to have to deal with whatever failure mode we actually hit.

814–815 ↗(On Diff #39493)

Sorry, that was copy/paste leftover. That can actually get dropped in favor of just using name, because they're the same thing.

840 ↗(On Diff #39493)

Indeed it does. See D14436 for the Lua half of this that's been tested alongside this.

cem added inline comments.
stand/zfs/zfs.c
821 ↗(On Diff #39495)

The less-than comparison only makes sense if name can contain nuls up to length len, which it cannot based on logic 5-6 lines earlier. (I'd remove the check.) Minor nit.

This revision is now accepted and ready to land.Feb 19 2018, 3:40 PM

Hey, I'm out today. Do not commit this until I sign off. I have some worries that I don't have time to articulate. I'll get to it first thing in the morning. I think it might be OK, but it might not, so I want to study it carefully. Thanks.

imp requested changes to this revision.Feb 19 2018, 4:35 PM
This revision now requires changes to proceed.Feb 19 2018, 4:35 PM

In an ideal world, you'd have a richer set of Lua functionality for the ZFS module. However, our current build system make that a bit of a challenge. So for the moment, this is fine.

This revision is now accepted and ready to land.Feb 21 2018, 3:55 PM
This revision was automatically updated to reflect the committed changes.