Page MenuHomeFreeBSD
Differential D43591

libbe: handle destroying/renaming temporary/bootonce boot environments
ClosedPublic
Actions

Authored by rcm on Jan 25 2024, 3:50 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Nov 16, 6:19 PM
Unknown Object (File)
Fri, Nov 15, 3:25 AM
Unknown Object (File)
Thu, Nov 14, 7:46 PM
Unknown Object (File)
Thu, Nov 14, 8:39 AM
Unknown Object (File)
Thu, Nov 14, 6:27 AM
Unknown Object (File)
Mon, Nov 11, 9:39 PM
Unknown Object (File)
Mon, Nov 11, 3:40 PM
Unknown Object (File)
Mon, Nov 11, 1:38 PM
View All 26 Files
Subscribers
imp

Details

Reviewers
kevans
Commits
rG5086b6ec32d5: libbe: handle destroying/renaming temporary/bootonce boot environments
Summary

When a temporary/bootonce boot environment is renamed, we need to also
update the bootenv nvlist on-disk to reflect the new name. Additionally,
when a temporary/bootonce boot environment is destroyed, we also need to
clear out the on-disk state.

Sponsored by: Rubicon Communications, LLC ("Netgate")

Test Plan
  1. bectl create -r new
  2. bectl activate -t new
  3. bectl list (confirm new is bootonce)
  4. bectl rename new new_new
  5. bectl list (confirm new_new is bootonce)

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

rcm created this revision.Jan 25 2024, 3:50 PM
Herald added a subscriber: imp. · View Herald TranscriptJan 25 2024, 3:50 PM
rcm requested review of this revision.Jan 25 2024, 3:50 PM
kevans added a comment.Jan 25 2024, 4:10 PM

Can you add a test for this in sbin/bectl/tests/bectl_test.sh as well, please? Presumably you could use zfsbootcfg -z rather than checking the bectl list output.

I note that we should also do something on destroy of the bootonce dataset.

rcm added a comment.Jan 25 2024, 4:15 PM
In D43591#994147, @kevans wrote:

Can you add a test for this in sbin/bectl/tests/bectl_test.sh as well, please? Presumably you could use zfsbootcfg -z rather than checking the bectl list output.

I note that we should also do something on destroy of the bootonce dataset.

Yup. Test is forthcoming. Thanks for tip on zfsbootcfg.

I will also look at the destroy case.

rcm updated this revision to Diff 133352.Jan 25 2024, 4:30 PM
rcm edited the summary of this revision. (Show Details)

Handles destroying bootonce case

rcm retitled this revision from libbe: handle renaming temporary/bootonce boot environments to libbe: handle destroying/renaming temporary/bootonce boot environments.Jan 25 2024, 4:31 PM
kevans added a comment.Jan 25 2024, 5:04 PM

Seems to LGTM. At some point we may want to update lbh->bootonce as new environments are activated/deactivated, but this is sufficient for bectl's needs and we don't really make many guarantees about the caching (or lack thereof) that occurs in the handle today, IIRC.

lib/libbe/be.c
201

Need to free(lbh->bootonce)here; turns out we were leaking it before... whoops.

rcm updated this revision to Diff 133353.Jan 25 2024, 5:10 PM
  1. Adds test cases for destroy and rename cases
  2. Fix bootonce leak
rcm added a comment.Jan 25 2024, 5:11 PM
In D43591#994155, @kevans wrote:

Seems to LGTM. At some point we may want to update lbh->bootonce as new environments are activated/deactivated, but this is sufficient for bectl's needs and we don't really make many guarantees about the caching (or lack thereof) that occurs in the handle today, IIRC.

Yes this is true. Anyone consuming libbe would need to grab a new handle to ensure consistent view of the world. I think we talked about handle refreshing a while ago.

lib/libbe/be.c
201

good catch, i'll include that

kevans added inline comments.Jan 25 2024, 10:22 PM
sbin/bectl/tests/bectl_test.sh
500

It's not clear to me that this situation's actually resolved itself; did you try running these on an i386 kernel?

586

What was I smoking?

rcm added inline comments.Jan 25 2024, 10:28 PM
sbin/bectl/tests/bectl_test.sh
500

As I understand it, the cleanup is not run if the body is skipped? But I could be wrong.

kevans added inline comments.Jan 25 2024, 10:32 PM
sbin/bectl/tests/bectl_test.sh
500

At the time it was added that was not true, I don't think any work has been done in kyua to avoid running cleanup on skip since then

rcm marked 3 inline comments as done.Jan 25 2024, 11:41 PM
rcm added inline comments.
sbin/bectl/tests/bectl_test.sh
500

I will add this back.

rcm updated this revision to Diff 133397.Jan 26 2024, 2:04 PM
rcm marked an inline comment as done.

Added back atf_skip for i386/armv7 to jail_cleanup

kevans accepted this revision.Jan 26 2024, 4:28 PM

Thanks!

This revision is now accepted and ready to land.Jan 26 2024, 4:28 PM
Closed by commit rG5086b6ec32d5: libbe: handle destroying/renaming temporary/bootonce boot environments (authored by rcm). · Explain WhyJan 29 2024, 3:09 PM
This revision was automatically updated to reflect the committed changes.
rcm added a commit: rG5086b6ec32d5: libbe: handle destroying/renaming temporary/bootonce boot environments.

Revision Contents
Changeset List

PathSize
lib/
libbe/
23 lines
1 line
7 lines
sbin/
bectl/
tests/
102 lines

Diff 133496

View Options

lib/libbe/be.c

Loading...
View Options

lib/libbe/be_impl.h

Loading...
View Options

lib/libbe/be_info.c

Loading...
View Options

sbin/bectl/tests/bectl_test.sh

Loading...