Page MenuHomeFreeBSD

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

Authored by rcm on Jan 25 2024, 3:50 PM.
Tags
None
Referenced Files
F102653410: D43591.diff
Fri, Nov 15, 9:37 AM
Unknown Object (File)
Fri, Nov 1, 3:08 PM
Unknown Object (File)
Sep 24 2024, 12:00 PM
Unknown Object (File)
Sep 24 2024, 8:42 AM
Unknown Object (File)
Sep 20 2024, 1:16 PM
Unknown Object (File)
Sep 17 2024, 2:54 PM
Unknown Object (File)
Sep 17 2024, 10:29 AM
Unknown Object (File)
Sep 16 2024, 4:17 PM
Subscribers

Details

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 Skipped
Unit
Tests Skipped

Event Timeline

rcm requested review of this revision.Jan 25 2024, 3:50 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.

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 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

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.

  1. Adds test cases for destroy and rename cases
  2. Fix bootonce leak

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

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?

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.

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 marked an inline comment as done.

Added back atf_skip for i386/armv7 to jail_cleanup

This revision is now accepted and ready to land.Jan 26 2024, 4:28 PM