be_destroy is documented to recursively destroy a boot environment. In the
case of snapshots, one would take this to mean that these are also
recursively destroyed. However, this was previously not the case.
be_destroy would descend into the be_destroy callback and attempt to
zfs_iter_children on the top-level snapshot, which is bogus.
Our alternative approach is to take note of the snapshot name and iterate
through all of fs children of the BE to try destruction in the children.
The -o option is also fixed to work properly with deep BEs. If the BE was
created with `bectl create -e otherDeepBE newDeepBE`, for instance, then a
recursive snapshot of otherDeepBE would have been taken for construction of
newDeepBE but a subsequent destroy with BE_DESTROY_ORIGIN set would only
clean up the snapshot at the root of otherDeepBE: ${BEROOT}/otherDeepBE@...
The most recent iteration instead pretends not to know how these things
work, verifies that the origin is another BE and then passes that back
through be_destroy to DTRT when snapshots and deep BEs may be in play.
MFC after: 1 week
Currently origin snapshots are left behind when a BE is destroyed, whether
it was an auto-created snapshot or explicitly specified via, for example,
`bectl create -e be@mysnap ...`.
Removing it automatically could be argued as a POLA violation in some
circumstances, so provide a flag to be_destroy for it. An accompanying
option will be added to bectl(8) to utilize this.
Some minor style/consistency nits in the affected areas also addressed.
Reported by: Shawn Webb
MFC after: 1 week
Previously, we directly used libzfs_core's lzc_receive to import to a
temporary snapshot, then cloned the snapshot and setup the properties. This
failed when attempting to import replication streams with questionable
error.
libzfs's zfs_receive is a much better fit here, so we now use it instead
with the destination dataset and let libzfs take care of the dirty details.
be_import is greatly simplified as a result.
Reported by: Marie Helene Kvello-Aune <freebsd@mhka.no>
MFC after: 1 week
This set of changes is geared towards making bectl respect deep boot
environments when they exist and are mounted. The deep BE composition
functionality (`bectl add`) remains disabled for the time being. This set of
changes has no effect for the average user. but allows deep BE users to
upgrade properly with their current setup.
libbe(3): Open the target boot environment and get a zfs handle, then pass
that with the target mountpoint to be_mount_iter; If the BE_MNT_DEEP flag is
set call zfs_iter_filesystems and mount the child datasets.
Similar logic is employed when unmounting the datasets, save for children
are unmounted first.
bectl(8): Change bectl_cmd_jail to pass the BE_MNT_DEEP flag when
calling be_mount as well as call be_unmount when cleaning up after the
jail has exited instead of umount(2) directly.
PR: 234795
Submitted by: Wes Maag <jwmaag_gmail.com> (test additions by kevans)
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D18796
We could perhaps have a method that does this given a dataset, but it's yet
clear that we'll always want to bypass the altroot when we grab the
mountpoint. For now, we'll refactor things a bit so we grab the altroot
length when libbe is initialized and have a common method that does the
necessary augmentation (replace with / if it's the root, return a pointer to
later in the string if not).
This will be used in some upcoming work to make be_mount work properly for
deep BEs.
MFC after: 1 week
Previously, the following sequence of events was feasible under some
circumstance:
bectl create test
bectl activate test
# the test BE dataset gets promoted and set as bootfs
bectl destroy test
I was unable to reproduce the destroy succeeding, but we should be rejecting
this before it even gets to libzfs because it would leave the system in an
inconsistent state. Forcing the user to be explicit as to which environment
should be activated instead is much better.
Reported by: Graham Perrin <grahamperrin@gmail.com>
MFC after: 3 days
Those two manual pages are already referencing each other in the HISTORY
sections, which people might skip. Mention those manual pages explicitly in
the SEE ALSO sections. Also, remove a reference to be(1) from libbe(3).
Reviewed by: bcr
Approved by: krion (mentor, implicit), mat (mentor, implicit)
Differential Revision: https://reviews.freebsd.org/D18136
As requested by a TODO in the source code.
Reviewed by: bcr
Approved by: krion (mentor, implicit), mat (mentor, implicit)
Differential Revision: https://reviews.freebsd.org/D18063
If rootfs isn't ZFS, current version will emit an error claiming so and fail
to initialize libbe. As a consumer, bectl -r (undocumented) can be specified
to operate on a BE independently of whether on a UFS or ZFS root.
Unbreak this for the UFS case by only erroring out the init if we can't
determine a ZFS dataset for rootfs and no BE root was specified. Consumers
of libbe should take care to ensure that rootfs is non-empty if they're
trying to use it, because this could certainly be the case.
Some check is needed before zfs_path_to_zhandle because it will
unconditionally emit to stderr if the path isn't a ZFS filesystem, which is
unhelpful for our purposes.
This should also unbreak the bectl(8) tests on a UFS root, as is the case in
Jenkins' -test runs.
MFC after: 3 days
Previously we would blindly copy the 'mountpoint' property, which includes
the altroot. The altroot needs to be snipped off prior to setting it on the
new BE, though, or you'll end up with a new BE and a mountpoint of /mnt with
altroot=/mnt
MFC after: 3 days
Add an undocumented -r option preceding the bectl subcommand to specify a BE
root to operate out of. This will remain undocumented for now, as some
caveats apply:
- BEs cannot be activated in the pool that doesn't contain the rootfs
- bectl create cannot work out of the box without the -e option right now,
since it defaults to the rootfs and cross-pool cloning doesn't work like
that (IIRC)
Plumb the BE root through to libbe(3) so that some things -can- be done to
it, e.g.
bectl -r tank/ROOT create -e default upgrade
bectl -r tank/ROOT mount upgrade /mnt
this aides in some upgrade setups where rootfs is not necessarily ZFS, and
also makes it easier/possible to regression-test bectl when combined with a
file-backed zpool.
MFC after: 3 days
Differential Revision: https://reviews.freebsd.org/D18029
Go through the ZFS layer instead; given a BE, we can derive the dataset,
zfs_open it, then zfs_unmount. ZFS takes care of the dirty details and
likely gets it more correct than we did for more interesting setups.
MFC after: 3 days
libbe(3) currently uses zfs_be_root and locates which of its children is
currently mounted at "/". This is reasonable, but not correct in the case of
a chroot, for two reasons:
- chroot root may be of a different zpool than zfs_be_root
- chroot root will not show up as mounted at "/"
Fix both of these by rewriting libbe_init to work from the rootfs down.
zfs_path_to_zhandle on / will resolve to the dataset mounted at the new
root, rather than the real root. From there, we can derive the BE root/pool
and grab the bootfs off of the new pool. This does no harm in the average
case, and opens up bectl to operating on different pools for scenarios where
one may be, for instance, updating a pool that generally gets re-rooted into
from a separate UFS root or zfs bootpool.
While here, I've also:
- Eliminated the check for /boot and / to be on the same partition. This
leaves one open to a setup where /boot (and consequently, kernel/modules)
are not included in the boot environment. This may very well be an
intentional setup done by someone that knows what they're doing, we should
not kill BE usage because of it.
- Eliminated the validation bits of BEs and snapshots that enforced
'mountpoint' to be "/" -- this broke when trying to operate on an imported
pool with an altroot, but we need not be this picky.
Reported by: philip
Reviewed by: philip, allanjude (previous version)
Tested by: philip
MFC after: 3 days
Differential Revision: https://reviews.freebsd.org/D18012
The previously activated BE should have canmount=noauto set on it upon
activation of the new BE, but we previously did not touch canmount on either
old or new BE.
PR: 233113
MFC after: 3 days
Most easily reproducible by attempting to activate the currently activated
BE, one would get a "not a cloned filesystem" error instead of success or a
sane message.
PR: 232488
MFC after: 3 days
This allows older BEs to be destroyed as they become replaced by a BE
created from them: e.g.
bectl create -e brokenworld fixedworld
bectl activate fixedworld
bectl destroy brokenworld
Submitted by: Shawn Webb
Approved by: re (gjb)
Obtained from: HardenedBSD (5948c0581e)
Some paths through be_exists will set the error state, others will not
There are multiple reasons that a call can fail, so clean it up a bit: all
paths now return an appropriate error code so the caller can attempt to
distinguish between a BE legitimately not existing and just having the wrong
mountpoint. The caller is expected to bubble the error through to the
internal error handler as needed.
This fixes some unfriendliness with bectl(8)'s activate subcommand, where
it might fail due to a bad mountpoint but the only message output is a
generic "failed to activate" message.
Approved by: re (gjb)
vermaden (maintainer of beadm) points out the following inconsistencies:
- "missing command" is not printed prior to usage if the error is simply a
missing command; this should be obvious from the context
- "bectl rename" isn't using the "don't unmount" flag (zfs rename -u), so
the active BE can't be renamed. It doesn't make sense in our context to
*not* use -u, so use it.
Documentation updates reflect the above and note an inconsistency with the
'destroy' command that is consistent with other parts of the base system.
A fix for libbe(3) not properly being installed to /lib is included.
SHLIBDIR should have been added when it was moved in r337995.
Approved by: re (kib)
Some background: in the GSoC project, libbe/Makefile lived in lib/libbe. I
created projects/bectl branch, maintained the above for all of five
minutes before I misread Makefile.inc1 and decided that it couldn't possibly
build outside of cddl/, so I kicked the Makefile out into the cddl/ build
and all was good. The misreading was of the bit where .WAIT is added to
SUBDIR after lib, libexec but prior to building bin and cddl *only during
the install targets*, which is the critical part.
Fast forward- buildworld was still broken in my branch unbeknownst to me
because I didn't nuke my OBJDIR. Combing through Makefile.inc1 eventually
revealed the necessary magic to make sure that libbe's dependencies are
specified well enough, and it becomes clear what needs done to make a
non-cddl/ build work. This is an interesting prospect, because the build
split is kind of annoying to work with.
IGNORE_PRAGMA is added to avoid dropping WARNS by one more. This was
previously pulled in via cddl/Makefile.inc.
Previously, we only validated names for character restrictions. This is
helpful, but we should've also checked length restrictions- dataset names
must be restricted to MAXNAMELEN.
While here, move validation before doing a bunch of concatenations and fix
error handling in be_rename. It was previously setting the error state based
on return value from a libzfs function, which is wrong: libzfs errors don't
necessarily match cleanly to libbe errors. This would cause the assertion in
be_error to hit when the error was printed.
Deleting the temp snapshot isn't immediately possible because it's the
origin of the newly imported boot environment. However, this is trivially
solved by opening the new boot environment and promoting it. The roles are
now reversed and the temp snapshot/dataset may be completely destroyed.
Remove the BUGS from libbe(3) and bectl(8).
The mostly-undocumented 'add' functionality, from initial read-through, is
intended for construction of deep ("bdrewery style") boot environments.
However, it's mostly broken at this point. `#if SOON` it out on both sides
so that we're not exposing a broken API/feature.
Work will resume on it in due time.
be_add_child functionality gets split out into separate places as a bonus.
A lot of places here we'll gloss over libzfs errors, because they shouldn't
be happening given the conditions that we're operating under. "Unknown
error" is what I'm intending to use for the moment to indicate an
exceptional circumstance- exceptional enough that we can't tell the consumer
did because we're not so certain that they did anything.
- File names don't necessarily need to be repeated
- Add SPDX tags
- Add a missing copyright for Kyle Kneitinger in bectl.8, originally written
by him in GSoC 2017; his standard copyright notice has been copied from
other files within the same directory to remain consistent with how he
clearly wished to portray it
This makes the be_exists behavior match the comments that assert that we've
already checked that the dataset derived from the BE name is set to mount at
/.
Other changes of note:
- bectl_list sees another change; changing mountpoint based on mount status
turns out to be a bad idea, so instead make the mounted property of the
returned nvlist the path that it's mounted at
- Always return the "mountpoint" property in "mountpoint" if it's ste
Loader is still relied upon at the beginning of libbe to specify the be
root, but we can derive from that the primary zpool and any vdevs that we
need to set nextboot bits on.
This lets me successfully `bectl activate -t test`, but UEFI loader doesn't
quite yet understand so it's effectively defunct.
be_get_dataset_snapshots has been added to libbe(3), effectively returning
the same information as be_get_bootenv_props but for snapshots of the given
dataset. The assumption is that one will have the BE dataset name before
wanting to grab snapshots.
This also accomplishes the following:
- Proxy through zfs_nicenum as be_nicenum, because it looks better than
humanize_number and would presumably be useful to other libbe consumers.
- Rename be_get_snapshot_props to be_get_dataset_props, make it more useful
