Page MenuHomeFreeBSD

bectl unjail: unmount boot environment when given jail id or jail name.
AcceptedPublic

Authored by rew on Dec 4 2020, 8:05 PM.
Tags
None
Referenced Files
Unknown Object (File)
Dec 27 2023, 4:31 AM
Unknown Object (File)
Oct 26 2023, 9:41 AM
Unknown Object (File)
Jan 16 2023, 4:41 PM
Unknown Object (File)
Jan 10 2023, 6:48 PM
Unknown Object (File)
Nov 26 2022, 7:55 AM
Subscribers

Details

Reviewers
kevans
allanjude
Summary

When passing a jail id or jail name to bectl unjail, bectl(8) fails to
unmount the boot environment after the jail is destroyed.

Currently, the jail id or jail name is passed directly to be_unmount(),
which results in be_unmount() failing.

This patch gets the boot environment name from the jail path before calling
be_unmount().

Test Plan

I added two tests:

  1. unjail using a jail id
  2. unjail using a jail name

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 35930
Build 32819: arc lint + arc unit

Event Timeline

rew requested review of this revision.Dec 4 2020, 8:05 PM
sbin/bectl/bectl_jail.c
481–488

We can actually simplify this logic entirely and do it without bename_from_path, I believe -- that last parameter to be_mounted_at is an nvlist * that will contain the same details returned by be_get_bootenv_props -- just need to allocate the nvlist for it and snatch the name.

Updating D27479: bectl unjail: unmount boot environment when given jail id or jail name.

Take advantage of be_mounted_at() to get nvlist of boot environment properties.

Inside be_mounted_at(), use info.name instead of libbe's root when getting
property info for the dataset.

Initialize bootonce inside be_mounted_at() so it's properly handled in
prop_list_builder_cb().

lib/libbe/be_access.c
220–222

Should this be propinfo.single_object = true?

It would allow for nvlist_lookup_string(belist, "name", &name) without having to get an nvpair first and then grabbing the name.

It appears bectl_cmd_unjail() is the only in-tree consumer of be_mounted_at().

Also, is it just me or does my local commit history make this updated revision look awkward?

lib/libbe/be_access.c
220–222

Huh, that's odd. What does your commit trail from this revision back to main look like? single_object = true is definitely what I really wanted here, and I think it's fine to go ahead and change it.

This diff is updated after running the below commands and should (hopefully) match
the description in my previous update summary.

% git reset --hard HEAD^

% git log --pretty=oneline
14ce8707c8d18266af6b90f0d2aa15e5ce27df59 (HEAD -> bectl-fix) bectl(8): unmount boot environment when given jail id or jail name
fd340a122259d44a7d01a72890ff40411f87d79c (freebsd/main, freebsd/HEAD, main) sbin/camcontrol: use calloc/strlcpy where appropriate.

My flow was:

checkout new branch from main (git checkout -b bectl-fix)
do some work.
local commit 1.
do some more work.
local commit 2.
arc diff --update -DXXXXX

Phabricator is showing the diff between commit 1 and 2. I would have expected
to show the diff between local commit 2 and main though.

Maybe I should have done a git rebase and drop commit 1 on my previous phabricator
update?

In D27479#624527, @rew wrote:

This diff is updated after running the below commands and should (hopefully) match
the description in my previous update summary.

Yes, this looks better, thanks!

My flow was:

checkout new branch from main (git checkout -b bectl-fix)
do some work.
local commit 1.
do some more work.
local commit 2.
arc diff --update -DXXXXX

Phabricator is showing the diff between commit 1 and 2. I would have expected
to show the diff between local commit 2 and main though.

Maybe I should have done a git rebase and drop commit 1 on my previous phabricator
update?

IMO squashing like that is reasonable, or IIRC you can specify main at the end of arc diff --update -D... to collect "all changes from main to the tip of the branch"

One last comment, feel free to smack it pre-commit.

sbin/bectl/bectl_jail.c
499

Let's cleanup the belist here and in the above failure case, then call this good.

This revision is now accepted and ready to land.Mar 12 2022, 2:19 AM