Date: Thu, 22 Nov 2018 01:25:45 +0000 (UTC) From: Kyle Evans <kevans@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org Subject: svn commit: r340751 - in stable/12: etc/mtree lib/libbe sbin/bectl sbin/bectl/tests Message-ID: <201811220125.wAM1PjMK011131@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: kevans Date: Thu Nov 22 01:25:44 2018 New Revision: 340751 URL: https://svnweb.freebsd.org/changeset/base/340751 Log: MFC r340507-r340508, r340592-r340594, r340635-r340636: bectl(8) fixes 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. Added: stable/12/sbin/bectl/tests/ - copied from r340594, head/sbin/bectl/tests/ Modified: stable/12/etc/mtree/BSD.tests.dist stable/12/lib/libbe/be.c stable/12/lib/libbe/be.h stable/12/lib/libbe/be_access.c stable/12/lib/libbe/be_impl.h stable/12/lib/libbe/be_info.c stable/12/lib/libbe/libbe.3 stable/12/sbin/bectl/Makefile stable/12/sbin/bectl/bectl.c stable/12/sbin/bectl/tests/bectl_test.sh Directory Properties: stable/12/ (props changed) Modified: stable/12/etc/mtree/BSD.tests.dist ============================================================================== --- stable/12/etc/mtree/BSD.tests.dist Wed Nov 21 22:37:49 2018 (r340750) +++ stable/12/etc/mtree/BSD.tests.dist Thu Nov 22 01:25:44 2018 (r340751) @@ -390,6 +390,8 @@ .. .. sbin + bectl + .. dhclient .. devd Modified: stable/12/lib/libbe/be.c ============================================================================== --- stable/12/lib/libbe/be.c Wed Nov 21 22:37:49 2018 (r340750) +++ stable/12/lib/libbe/be.c Thu Nov 22 01:25:44 2018 (r340751) @@ -29,11 +29,12 @@ #include <sys/cdefs.h> __FBSDID("$FreeBSD$"); +#include <sys/param.h> +#include <sys/mount.h> #include <sys/stat.h> -#include <sys/types.h> +#include <sys/ucred.h> #include <ctype.h> -#include <kenv.h> #include <libgen.h> #include <libzfs_core.h> #include <stdio.h> @@ -55,23 +56,30 @@ static int be_create_child_cloned(libbe_handle_t *lbh, * zfs_be_root set by loader(8). data is expected to be a libbe_handle_t *. */ static int -be_locate_rootfs(zfs_handle_t *chkds, void *data) +be_locate_rootfs(libbe_handle_t *lbh) { - libbe_handle_t *lbh; - char *mntpoint; + struct statfs sfs; + struct extmnttab entry; + zfs_handle_t *zfs; - lbh = (libbe_handle_t *)data; - if (lbh == NULL) + /* + * Check first if root is ZFS; if not, we'll bail on rootfs capture. + * Unfortunately needed because zfs_path_to_zhandle will emit to + * stderr if / isn't actually a ZFS filesystem, which we'd like + * to avoid. + */ + if (statfs("/", &sfs) == 0) { + statfs2mnttab(&sfs, &entry); + if (strcmp(entry.mnt_fstype, MNTTYPE_ZFS) != 0) + return (1); + } else return (1); - - mntpoint = NULL; - if (zfs_is_mounted(chkds, &mntpoint) && strcmp(mntpoint, "/") == 0) { - strlcpy(lbh->rootfs, zfs_get_name(chkds), sizeof(lbh->rootfs)); - free(mntpoint); + zfs = zfs_path_to_zhandle(lbh->lzh, "/", ZFS_TYPE_FILESYSTEM); + if (zfs == NULL) return (1); - } else if(mntpoint != NULL) - free(mntpoint); + strlcpy(lbh->rootfs, zfs_get_name(zfs), sizeof(lbh->rootfs)); + zfs_close(zfs); return (0); } @@ -80,52 +88,40 @@ be_locate_rootfs(zfs_handle_t *chkds, void *data) * dataset, for example, zroot/ROOT. */ libbe_handle_t * -libbe_init(void) +libbe_init(const char *root) { - struct stat sb; - dev_t root_dev, boot_dev; libbe_handle_t *lbh; - zfs_handle_t *rootds; char *poolname, *pos; int pnamelen; lbh = NULL; poolname = pos = NULL; - rootds = NULL; - /* Verify that /boot and / are mounted on the same filesystem */ - /* TODO: use errno here?? */ - if (stat("/", &sb) != 0) - goto err; - - root_dev = sb.st_dev; - - if (stat("/boot", &sb) != 0) - goto err; - - boot_dev = sb.st_dev; - - if (root_dev != boot_dev) { - fprintf(stderr, "/ and /boot not on same device, quitting\n"); - goto err; - } - if ((lbh = calloc(1, sizeof(libbe_handle_t))) == NULL) goto err; if ((lbh->lzh = libzfs_init()) == NULL) goto err; - /* Obtain path to boot environment root */ - if ((kenv(KENV_GET, "zfs_be_root", lbh->root, - sizeof(lbh->root))) == -1) - goto err; + /* + * Grab rootfs, we'll work backwards from there if an optional BE root + * has not been passed in. + */ + if (be_locate_rootfs(lbh) != 0) { + if (root == NULL) + goto err; + *lbh->rootfs = '\0'; + } + if (root == NULL) { + /* Strip off the final slash from rootfs to get the be root */ + strlcpy(lbh->root, lbh->rootfs, sizeof(lbh->root)); + pos = strrchr(lbh->root, '/'); + if (pos == NULL) + goto err; + *pos = '\0'; + } else + strlcpy(lbh->root, root, sizeof(lbh->root)); - /* Remove leading 'zfs:' if present, otherwise use value as-is */ - if (strcmp(lbh->root, "zfs:") == 0) - strlcpy(lbh->root, strchr(lbh->root, ':') + sizeof(char), - sizeof(lbh->root)); - if ((pos = strchr(lbh->root, '/')) == NULL) goto err; @@ -144,17 +140,6 @@ libbe_init(void) sizeof(lbh->bootfs), NULL, true) != 0) goto err; - /* Obtain path to boot environment rootfs (currently booted) */ - /* XXX Get dataset mounted at / by kenv/GUID from mountroot? */ - if ((rootds = zfs_open(lbh->lzh, lbh->root, ZFS_TYPE_DATASET)) == NULL) - goto err; - - zfs_iter_filesystems(rootds, be_locate_rootfs, lbh); - zfs_close(rootds); - rootds = NULL; - if (*lbh->rootfs == '\0') - goto err; - return (lbh); err: if (lbh != NULL) { @@ -337,6 +322,7 @@ be_deep_clone_prop(int prop, void *cb) zprop_source_t src; char pval[BE_MAXPATHLEN]; char source[BE_MAXPATHLEN]; + char *val; dccb = cb; /* Skip some properties we don't want to touch */ @@ -356,7 +342,15 @@ be_deep_clone_prop(int prop, void *cb) if (src != ZPROP_SRC_LOCAL) return (ZPROP_CONT); - nvlist_add_string(dccb->props, zfs_prop_to_name(prop), (char *)pval); + /* Augment mountpoint with altroot, if needed */ + val = pval; + if (prop == ZFS_PROP_MOUNTPOINT && *dccb->altroot != '\0') { + if (pval[strlen(dccb->altroot)] == '\0') + strlcpy(pval, "/", sizeof(pval)); + else + val = pval + strlen(dccb->altroot); + } + nvlist_add_string(dccb->props, zfs_prop_to_name(prop), val); return (ZPROP_CONT); } @@ -399,6 +393,10 @@ be_deep_clone(zfs_handle_t *ds, void *data) dccb.zhp = ds; dccb.props = props; + if (zpool_get_prop(isdc->lbh->active_phandle, ZPOOL_PROP_ALTROOT, + dccb.altroot, sizeof(dccb.altroot), NULL, true) != 0 || + strcmp(dccb.altroot, "-") == 0) + *dccb.altroot = '\0'; if (zprop_iter(be_deep_clone_prop, &dccb, B_FALSE, B_FALSE, ZFS_TYPE_FILESYSTEM) == ZPROP_INVAL) return (-1); @@ -503,10 +501,6 @@ be_create_from_existing(libbe_handle_t *lbh, const cha int be_validate_snap(libbe_handle_t *lbh, const char *snap_name) { - zfs_handle_t *zfs_hdl; - char buf[BE_MAXPATHLEN]; - char *delim_pos; - int err = BE_ERR_SUCCESS; if (strlen(snap_name) >= BE_MAXPATHLEN) return (BE_ERR_PATHLEN); @@ -515,27 +509,7 @@ be_validate_snap(libbe_handle_t *lbh, const char *snap ZFS_TYPE_SNAPSHOT)) return (BE_ERR_NOENT); - strlcpy(buf, snap_name, sizeof(buf)); - - /* Find the base filesystem of the snapshot */ - if ((delim_pos = strchr(buf, '@')) == NULL) - return (BE_ERR_INVALIDNAME); - *delim_pos = '\0'; - - if ((zfs_hdl = - zfs_open(lbh->lzh, buf, ZFS_TYPE_DATASET)) == NULL) - return (BE_ERR_NOORIGIN); - - if ((err = zfs_prop_get(zfs_hdl, ZFS_PROP_MOUNTPOINT, buf, - sizeof(buf), NULL, NULL, 0, 1)) != 0) - err = BE_ERR_BADMOUNT; - - if ((err != 0) && (strncmp(buf, "/", sizeof(buf)) != 0)) - err = BE_ERR_BADMOUNT; - - zfs_close(zfs_hdl); - - return (err); + return (BE_ERR_SUCCESS); } Modified: stable/12/lib/libbe/be.h ============================================================================== --- stable/12/lib/libbe/be.h Wed Nov 21 22:37:49 2018 (r340750) +++ stable/12/lib/libbe/be.h Thu Nov 22 01:25:44 2018 (r340751) @@ -63,7 +63,7 @@ typedef enum be_error { /* Library handling functions: be.c */ -libbe_handle_t *libbe_init(void); +libbe_handle_t *libbe_init(const char *root); void libbe_close(libbe_handle_t *); /* Bootenv information functions: be_info.c */ Modified: stable/12/lib/libbe/be_access.c ============================================================================== --- stable/12/lib/libbe/be_access.c Wed Nov 21 22:37:49 2018 (r340750) +++ stable/12/lib/libbe/be_access.c Thu Nov 22 01:25:44 2018 (r340751) @@ -164,37 +164,18 @@ be_unmount(libbe_handle_t *lbh, char *bootenv, int fla { int err, mntflags; char be[BE_MAXPATHLEN]; - struct statfs *mntbuf; - int mntsize; - char *mntpath; + zfs_handle_t *root_hdl; if ((err = be_root_concat(lbh, bootenv, be)) != 0) return (set_error(lbh, err)); - if ((mntsize = getmntinfo(&mntbuf, MNT_NOWAIT)) == 0) { - if (errno == EIO) - return (set_error(lbh, BE_ERR_IO)); - return (set_error(lbh, BE_ERR_NOMOUNT)); - } + if ((root_hdl = zfs_open(lbh->lzh, be, ZFS_TYPE_FILESYSTEM)) == NULL) + return (set_error(lbh, BE_ERR_ZFSOPEN)); - mntpath = NULL; - for (int i = 0; i < mntsize; ++i) { - /* 0x000000de is the type number of zfs */ - if (mntbuf[i].f_type != 0x000000de) - continue; + mntflags = (flags & BE_MNT_FORCE) ? MS_FORCE : 0; - if (strcmp(mntbuf[i].f_mntfromname, be) == 0) { - mntpath = mntbuf[i].f_mntonname; - break; - } - } - - if (mntpath == NULL) - return (set_error(lbh, BE_ERR_NOMOUNT)); - - mntflags = (flags & BE_MNT_FORCE) ? MNT_FORCE : 0; - - if ((err = unmount(mntpath, mntflags)) != 0) { + if (zfs_unmount(root_hdl, NULL, mntflags) != 0) { + zfs_close(root_hdl); switch (errno) { case ENAMETOOLONG: return (set_error(lbh, BE_ERR_PATHLEN)); @@ -210,6 +191,7 @@ be_unmount(libbe_handle_t *lbh, char *bootenv, int fla return (set_error(lbh, BE_ERR_UNKNOWN)); } } + zfs_close(root_hdl); - return (set_error(lbh, BE_ERR_SUCCESS)); + return (BE_ERR_SUCCESS); } Modified: stable/12/lib/libbe/be_impl.h ============================================================================== --- stable/12/lib/libbe/be_impl.h Wed Nov 21 22:37:49 2018 (r340750) +++ stable/12/lib/libbe/be_impl.h Thu Nov 22 01:25:44 2018 (r340751) @@ -55,6 +55,7 @@ struct libbe_deep_clone { struct libbe_dccb { zfs_handle_t *zhp; nvlist_t *props; + char altroot[MAXPATHLEN]; }; typedef struct prop_data { Modified: stable/12/lib/libbe/be_info.c ============================================================================== --- stable/12/lib/libbe/be_info.c Wed Nov 21 22:37:49 2018 (r340750) +++ stable/12/lib/libbe/be_info.c Thu Nov 22 01:25:44 2018 (r340751) @@ -42,7 +42,10 @@ const char * be_active_name(libbe_handle_t *lbh) { - return (strrchr(lbh->rootfs, '/') + sizeof(char)); + if (*lbh->rootfs != '\0') + return (strrchr(lbh->rootfs, '/') + sizeof(char)); + else + return (lbh->rootfs); } @@ -63,7 +66,10 @@ const char * be_nextboot_name(libbe_handle_t *lbh) { - return (strrchr(lbh->bootfs, '/') + sizeof(char)); + if (*lbh->bootfs != '\0') + return (strrchr(lbh->bootfs, '/') + sizeof(char)); + else + return (lbh->bootfs); } @@ -289,30 +295,11 @@ int be_exists(libbe_handle_t *lbh, char *be) { char buf[BE_MAXPATHLEN]; - nvlist_t *dsprops; - char *mntpoint; - bool valid; be_root_concat(lbh, be, buf); if (!zfs_dataset_exists(lbh->lzh, buf, ZFS_TYPE_DATASET)) return (BE_ERR_NOENT); - /* Also check if it's mounted at / */ - if (be_prop_list_alloc(&dsprops) != 0) - return (BE_ERR_UNKNOWN); - - if (be_get_dataset_props(lbh, buf, dsprops) != 0) { - nvlist_free(dsprops); - return (BE_ERR_UNKNOWN); - } - - if (nvlist_lookup_string(dsprops, "mountpoint", &mntpoint) == 0) { - valid = (strcmp(mntpoint, "/") == 0); - nvlist_free(dsprops); - return (valid ? BE_ERR_SUCCESS : BE_ERR_BADMOUNT); - } - - nvlist_free(dsprops); - return (BE_ERR_BADMOUNT); + return (BE_ERR_SUCCESS); } Modified: stable/12/lib/libbe/libbe.3 ============================================================================== --- stable/12/lib/libbe/libbe.3 Wed Nov 21 22:37:49 2018 (r340750) +++ stable/12/lib/libbe/libbe.3 Thu Nov 22 01:25:44 2018 (r340751) @@ -28,7 +28,7 @@ .\" .\" $FreeBSD$ .\" -.Dd August 31, 2018 +.Dd November 17, 2018 .Dt LIBBE 3 .Os .Sh NAME @@ -39,7 +39,7 @@ .Sh SYNOPSIS .In be.h .Ft "libbe_handle_t *hdl" Ns -.Fn libbe_init void +.Fn libbe_init "const char *be_root" .Pp .Ft void .Fn libbe_close "libbe_handle_t *hdl" @@ -157,13 +157,16 @@ errno otherwise as described in .Pp The .Fn libbe_init -function initializes +function takes an optional BE root and initializes .Nm , returning a .Vt "libbe_handle_t *" on success, or .Dv NULL on error. +If a BE root is supplied, +.Nm +will only operate out of that pool and BE root. An error may occur if: .Bl -column .It /boot and / are not on the same filesystem and device, @@ -184,11 +187,15 @@ invalidating the handle in the process. .Pp The .Fn be_active_name -function returns the name of the currently booted boot environment, +function returns the name of the currently booted boot environment. +This boot environment may not belong to the same BE root as the root libbe +is operating on! .Pp The .Fn be_active_path function returns the full path of the currently booted boot environment. +This boot environment may not belong to the same BE root as the root libbe +is operating on! .Pp The .Fn be_nextboot_name Modified: stable/12/sbin/bectl/Makefile ============================================================================== --- stable/12/sbin/bectl/Makefile Wed Nov 21 22:37:49 2018 (r340750) +++ stable/12/sbin/bectl/Makefile Thu Nov 22 01:25:44 2018 (r340751) @@ -1,5 +1,7 @@ # $FreeBSD$ +.include <src.opts.mk> + PROG= bectl MAN= bectl.8 @@ -15,5 +17,8 @@ CFLAGS+= -I${SRCTOP}/sys/cddl/compat/opensolaris CFLAGS+= -I${SRCTOP}/sys/cddl/contrib/opensolaris/uts/common CFLAGS+= -DNEED_SOLARIS_BOOLEAN + +HAS_TESTS= yes +SUBDIR.${MK_TESTS}+= tests .include <bsd.prog.mk> Modified: stable/12/sbin/bectl/bectl.c ============================================================================== --- stable/12/sbin/bectl/bectl.c Wed Nov 21 22:37:49 2018 (r340750) +++ stable/12/sbin/bectl/bectl.c Thu Nov 22 01:25:44 2018 (r340751) @@ -489,12 +489,25 @@ int main(int argc, char *argv[]) { const char *command; + char *root; int command_index, rc; + root = NULL; if (argc < 2) return (usage(false)); - command = argv[1]; + if (strcmp(argv[1], "-r") == 0) { + if (argc < 4) + return (usage(false)); + root = strdup(argv[2]); + command = argv[3]; + argc -= 3; + argv += 3; + } else { + command = argv[1]; + argc -= 1; + argv += 1; + } /* Handle command aliases */ if (strcmp(command, "umount") == 0) @@ -512,13 +525,12 @@ main(int argc, char *argv[]) } - if ((be = libbe_init()) == NULL) + if ((be = libbe_init(root)) == NULL) return (-1); libbe_print_on_error(be, true); - /* XXX TODO: can be simplified if offset by 2 instead of one */ - rc = command_map[command_index].fn(argc-1, argv+1); + rc = command_map[command_index].fn(argc, argv); libbe_close(be); return (rc); Modified: stable/12/sbin/bectl/tests/bectl_test.sh ============================================================================== --- head/sbin/bectl/tests/bectl_test.sh Mon Nov 19 02:30:12 2018 (r340594) +++ stable/12/sbin/bectl/tests/bectl_test.sh Thu Nov 22 01:25:44 2018 (r340751) @@ -34,7 +34,7 @@ bectl_create_setup() disk=$2 mnt=$3 - kldstat -qm zfs || atf_skip "ZFS module not loaded on the current system" + kldload -n -q zfs || atf_skip "ZFS module not loaded on the current system" atf_check mkdir -p ${mnt} atf_check truncate -s 1G ${disk} atf_check zpool create -o altroot=${mnt} ${zpool} ${disk}
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201811220125.wAM1PjMK011131>