FreeBSD source tree https://cgit-dev.freebsd.org/src/atom?h=releng%2F12.0 2018-11-22T04:39:06Z 2018-11-22T04:39:06Z Kyle Evans kevans@FreeBSD.org 2018-11-22T04:39:06Z urn:sha1:803e875cdcba34f0debc6d9ddd9512279f1836d6 r340507: libbe(3): rewrite init to support chroot usage 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 Differential Revision: https://reviews.freebsd.org/D18012 r340508: libbe(3): Rewrite be_unmount to stop mucking with getmntinfo(2) 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. r340592: bectl(3)/libbe(3): Allow BE root to be specified 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. Differential Revision: https://reviews.freebsd.org/D18029 r340593: libbe(3): Properly account for altroot when creating new BEs 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 r340594: bectl(8): Add some regression tests These tests operate on a file-backed zpool that gets created in the kyua temp dir. root and ZFS support are both required for these tests. Current tests cover create, destroy, export/import, jail, list (kind of), mount, rename, and jail. List tests should later be extended to cover formatting and the different list flags, but for now only covers basic "are create/destroy actually reflected properly" r340635: libbe(3): Handle non-ZFS rootfs better 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. r340636: bectl(8) tests: attempt to load the ZFS module Observed in a CI test image, bectl_create test will run and be marked as skipped because the module is not loaded. The first zpool invocation will automagically load the module, but bectl_create is still skipped. Subsequent tests all pass as expected because the module is now loaded and everything is OK. Approved by: re (gjb) 2018-11-15T16:03:52Z Kyle Evans kevans@FreeBSD.org 2018-11-15T16:03:52Z urn:sha1:2210ae49651495adfc09ae404412ba843a9d939f 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 Approved by: re (gjb) 2018-11-05T18:12:41Z Kyle Evans kevans@FreeBSD.org 2018-11-05T18:12:41Z urn:sha1:1ef5e7d56c2fe0d59370b0fea7ba48d2499aaa65 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 Approved by: re (gjb) 2018-10-01T14:57:33Z Kyle Evans kevans@FreeBSD.org 2018-10-01T14:57:33Z urn:sha1:0cadc427fdaef4e865262940eb6582f49074573f 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) 2018-09-01T02:22:26Z Kyle Evans kevans@FreeBSD.org 2018-09-01T02:22:26Z urn:sha1:162ec569493ba1919857c22a957c97700246da14 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) 2018-08-24T20:44:58Z Kyle Evans kevans@FreeBSD.org 2018-08-24T20:44:58Z urn:sha1:8369ba427a9a4db4c4886fb2a6d7a7dae00de707 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) 2018-08-18T03:20:59Z Kyle Evans kevans@FreeBSD.org 2018-08-18T03:20:59Z urn:sha1:7920ad944bbe862638c164591d7ec64a64d8f62f 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. 2018-08-16T18:58:34Z Kyle Evans kevans@FreeBSD.org 2018-08-16T18:58:34Z urn:sha1:5b7803a99396adf10d7621be40bcf1f5fca85a98 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. 2018-08-16T18:37:47Z Kyle Evans kevans@FreeBSD.org 2018-08-16T18:37:47Z urn:sha1:a8e44f4da0e8b8892688d88a8faea5abfff2219e Or, in the activate case, just use snprintf since that's effectively what we're doing anyways. 2018-08-16T17:56:03Z Kyle Evans kevans@FreeBSD.org 2018-08-16T17:56:03Z urn:sha1:55b0e92b89c2df6105e91f022dbfb5e32eafbd17 While here, use sizeof() in some places that it makes sense to reduce room for error and prefer strlcpy to strncpy