From f476a37b3714c6c0e218814693eb623f4d77d549 Mon Sep 17 00:00:00 2001 From: QORTEC Date: Tue, 10 Oct 2023 17:07:16 -0400 Subject: [PATCH 1/5] This commit adds functions to libzfs to check if one dataset is related to another. The `related_dataset()` function checks if the second provided dataset is the same as or a child dataset of the first provided dataset. The function returns a `boolean_t` value (`B_TRUE` if related). The `zfs_related_dataset()` helper function takes a `zfs_handle_t` and checks if the provided handle is related to the provided dataset. The function returns a `boolean_t` value (`B_TRUE` if related). libzfs_dataset.c: - `related_dataset()` - `zfs_related_dataset()` libzfs.h: - `zfs_related_dataset()` Signed-off-by: QORTEC --- include/libzfs.h | 1 + lib/libzfs/libzfs_dataset.c | 29 +++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/include/libzfs.h b/include/libzfs.h index fa05b7921bb5..ee1830dd2ee0 100644 --- a/include/libzfs.h +++ b/include/libzfs.h @@ -887,6 +887,7 @@ _LIBZFS_H int zfs_name_valid(const char *, zfs_type_t); _LIBZFS_H zfs_handle_t *zfs_path_to_zhandle(libzfs_handle_t *, const char *, zfs_type_t); _LIBZFS_H int zfs_parent_name(zfs_handle_t *, char *, size_t); +_LIBZFS_H boolean_t zfs_dataset_related(zfs_handle_t *, const char *); _LIBZFS_H boolean_t zfs_dataset_exists(libzfs_handle_t *, const char *, zfs_type_t); _LIBZFS_H int zfs_spa_version(zfs_handle_t *, int *); diff --git a/lib/libzfs/libzfs_dataset.c b/lib/libzfs/libzfs_dataset.c index 11d3eb6a3c60..c037811933b3 100644 --- a/lib/libzfs/libzfs_dataset.c +++ b/lib/libzfs/libzfs_dataset.c @@ -3446,6 +3446,35 @@ is_descendant(const char *ds1, const char *ds2) return (ds2[d1len] == '/' && (strncmp(ds1, ds2, d1len) == 0)); } +/* + * Is one dataset is related to another, including self-relation? + * + * Needs to handle these cases: + * Dataset 1 "a/foo" "a/foo" "a/foo" "a/foo" + * Dataset 2 "a/foo" "a/fo" "a/foobar" "a/foo/bar" + * Related? Yes. No. No. Yes. + */ +static boolean_t +dataset_related(const char *ds1, const char *ds2) +{ + /* ds1 and ds2 are identical */ + if (strcmp(ds1, ds2) == 0) + return (B_TRUE); + + /* ds2 is a descendant of ds1 */ + if (is_descendant(ds1, ds2)) + return (B_TRUE); + + /* dataset are not related. */ + return (B_FALSE); +} + +boolean_t +zfs_dataset_related(zfs_handle_t *zhp, const char *parent) +{ + return (dataset_related(parent, zfs_get_name(zhp))); +} + /* * Given a complete name, return just the portion that refers to the parent. * Will return -1 if there is no parent (path is just the name of the From a1295cd96a2cac04536f82e031c0f4a4a36e6a4b Mon Sep 17 00:00:00 2001 From: QORTEC Date: Tue, 10 Oct 2023 17:40:41 -0400 Subject: [PATCH 2/5] Added limited/recursive operation to `zfs mount` This commit introduces limited/recursive filesystem mounting by leveraging the existing `zfs mount -a` codebase with minor additions and modifications. Now, when running `zfs mount <-a|-A> zpool/dataset`, the command will mount all datasets that are the specified dataset itself or it's children, provided they are available. In addition, `-A` flag will also mount datasets with `canmount=noauto` property. Changes in `zfs_main.c`: - `HELP_MOUNT` - Updated `usage()` message to reflect the changes. - `get_all_state_t` - Added `const char *ga_parent`; used to specify the parent dataset. - `get_one_dataset()` - Added a check; if `ga_parent` is set, skips any datasets that are not `ga_parent` itself or it's children. - `get_all_datasets()` - Added `const char *parent` property; used to pass the parent dataset to the `get_all_state_t` struct. - `share_mount_state_t` - Added `boolean_t sm_mount_noauto`; used to treat datasets with `canmount=noauto` property as as if it were `canmount=on`. - `share_mount_one()`; - Added `boolean_t mount_noauto` property. - Modified the 'noauto' check; when mounting datasets, if `mount_noauto` is true, treat the mount as if `canmount=on` or `explicit` is `B_TRUE`. - `share_mount_one_cb()` - Updated `share_mount_one()` call to include the new property, passes the value of `sm_mount_noauto`. - `share_mount()` - Changed `do_all` from `int` to `boolean_t` - Added `boolean_t do_noauto` property; used for mounting datasets with `canmount=noauto` as `canmount=on`. - Added the `-A` flag; when used sets `do_noauto` to `B_TRUE`. - Updated argument check; displaies the correct error messages. - Limited the usage of `<-a|-A> filesystem` to `zfs mount` only - Added a check; to validate that the specified filesystem is indeed a valid ZFS filesystem. - Updated `get_all_datasets()` call to include the new property, passes the value of `filesystem`. - Added `share_mount_state.sm_mount_noauto`; the `share_mount_state_t` has a new member, uses value of `do_noauto` - Updated `share_mount_one()` call to include the new property, passes the value `B_FALSE`. Signed-off-by: QORTEC --- cmd/zfs/zfs_main.c | 93 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 70 insertions(+), 23 deletions(-) diff --git a/cmd/zfs/zfs_main.c b/cmd/zfs/zfs_main.c index ea73bb018a92..3c96ddf97362 100644 --- a/cmd/zfs/zfs_main.c +++ b/cmd/zfs/zfs_main.c @@ -307,7 +307,8 @@ get_usage(zfs_help_t idx) "[filesystem|volume|snapshot] ...\n")); case HELP_MOUNT: return (gettext("\tmount\n" - "\tmount [-flvO] [-o opts] <-a | filesystem>\n")); + "\tmount [-flvO] [-o opts] " + "<-a [filesystem] | [-A] filesystem>\n")); case HELP_PROMOTE: return (gettext("\tpromote \n")); case HELP_RECEIVE: @@ -6731,6 +6732,7 @@ zfs_do_holds(int argc, char **argv) typedef struct get_all_state { boolean_t ga_verbose; get_all_cb_t *ga_cbp; + const char *ga_parent; } get_all_state_t; static int @@ -6769,6 +6771,16 @@ get_one_dataset(zfs_handle_t *zhp, void *data) zfs_close(zhp); return (0); } + + /* + * Skip any dataset that's not related to ga_parent. + */ + if (state->ga_parent != NULL && + !zfs_dataset_related(zhp, state->ga_parent)) { + zfs_close(zhp); + return (0); + } + libzfs_add_handle(state->ga_cbp, zhp); assert(state->ga_cbp->cb_used <= state->ga_cbp->cb_alloc); @@ -6776,11 +6788,12 @@ get_one_dataset(zfs_handle_t *zhp, void *data) } static void -get_all_datasets(get_all_cb_t *cbp, boolean_t verbose) +get_all_datasets(get_all_cb_t *cbp, boolean_t verbose, const char *parent) { get_all_state_t state = { .ga_verbose = verbose, - .ga_cbp = cbp + .ga_cbp = cbp, + .ga_parent = parent }; if (verbose) @@ -6808,6 +6821,7 @@ typedef struct share_mount_state { uint_t sm_total; /* number of filesystems to process */ uint_t sm_done; /* number of filesystems processed */ int sm_status; /* -1 if any of the share/mount operations failed */ + boolean_t sm_mount_noauto; /* treat 'canmount=noauto' as 'on' */ } share_mount_state_t; /* @@ -6815,7 +6829,7 @@ typedef struct share_mount_state { */ static int share_mount_one(zfs_handle_t *zhp, int op, int flags, enum sa_protocol protocol, - boolean_t explicit, const char *options) + boolean_t explicit, const char *options, boolean_t mount_noauto) { char mountpoint[ZFS_MAXPROPLEN]; char shareopts[ZFS_MAXPROPLEN]; @@ -6905,13 +6919,14 @@ share_mount_one(zfs_handle_t *zhp, int op, int flags, enum sa_protocol protocol, } /* - * canmount explicit outcome - * on no pass through - * on yes pass through - * off no return 0 - * off yes display error, return 1 - * noauto no return 0 - * noauto yes pass through + * canmount explicit mount_noauto outcome + * on no n/a pass through + * on yes n/a pass through + * off no n/a return 0 + * off yes n/a display error, return 1 + * noauto no no return 0 + * noauto no yes pass through + * noauto yes yes/no pass through */ canmount = zfs_prop_get_int(zhp, ZFS_PROP_CANMOUNT); if (canmount == ZFS_CANMOUNT_OFF) { @@ -6922,11 +6937,13 @@ share_mount_one(zfs_handle_t *zhp, int op, int flags, enum sa_protocol protocol, "'canmount' property is set to 'off'\n"), cmdname, zfs_get_name(zhp)); return (1); - } else if (canmount == ZFS_CANMOUNT_NOAUTO && !explicit) { + } else if (canmount == ZFS_CANMOUNT_NOAUTO && + !explicit && !mount_noauto) { /* * When performing a 'zfs mount -a', we skip any mounts for - * datasets that have 'noauto' set. Sharing a dataset with - * 'noauto' set is only allowed if it's mounted. + * datasets that have 'noauto' set. However, 'zfs mount -A' + * will mount datasets with 'noauto' set. Sharing a dataset + * with 'noauto' set is only allowed if it's mounted. */ if (op == OP_MOUNT) return (0); @@ -7082,7 +7099,7 @@ share_mount_one_cb(zfs_handle_t *zhp, void *arg) int ret; ret = share_mount_one(zhp, sms->sm_op, sms->sm_flags, sms->sm_proto, - B_FALSE, sms->sm_options); + B_FALSE, sms->sm_options, sms->sm_mount_noauto); pthread_mutex_lock(&sms->sm_lock); if (ret != 0) @@ -7132,18 +7149,22 @@ sa_protocol_decode(const char *protocol) static int share_mount(int op, int argc, char **argv) { - int do_all = 0; + boolean_t do_all = B_FALSE; + boolean_t do_noauto = B_FALSE; boolean_t verbose = B_FALSE; int c, ret = 0; char *options = NULL; int flags = 0; /* check options */ - while ((c = getopt(argc, argv, op == OP_MOUNT ? ":alvo:Of" : "al")) + while ((c = getopt(argc, argv, op == OP_MOUNT ? ":aAlvo:Of" : "al")) != -1) { switch (c) { case 'a': - do_all = 1; + do_all = B_TRUE; + break; + case 'A': + do_noauto = B_TRUE; break; case 'v': verbose = B_TRUE; @@ -7186,7 +7207,7 @@ share_mount(int op, int argc, char **argv) argv += optind; /* check number of arguments */ - if (do_all) { + if (do_all || do_noauto) { enum sa_protocol protocol = SA_NO_PROTOCOL; if (op == OP_SHARE && argc > 0) { @@ -7194,15 +7215,40 @@ share_mount(int op, int argc, char **argv) argc--; argv++; } - - if (argc != 0) { + /* check number of arguments */ + if (do_noauto && argc < 1) { + (void) fprintf(stderr, gettext("missing " + "dataset argument\n")); + usage(B_FALSE); + } + if ((op == OP_SHARE && argc != 0) || argc > 1) { (void) fprintf(stderr, gettext("too many arguments\n")); usage(B_FALSE); } + /* + * Limit `-a filesystem` to mount only + */ + const char *filesystem = NULL; + if (op == OP_MOUNT) + filesystem = argv[0]; + + /* + * Validate filesystem is actually a valid zfs filesystem + */ + if (filesystem != NULL) { + zfs_handle_t *zhp = zfs_open(g_zfs, filesystem, + ZFS_TYPE_FILESYSTEM); + if (zhp == NULL) { + free(options); + return (1); + } + zfs_close(zhp); + } + start_progress_timer(); get_all_cb_t cb = { 0 }; - get_all_datasets(&cb, verbose); + get_all_datasets(&cb, verbose, filesystem); if (cb.cb_used == 0) { free(options); @@ -7216,6 +7262,7 @@ share_mount(int op, int argc, char **argv) share_mount_state.sm_options = options; share_mount_state.sm_proto = protocol; share_mount_state.sm_total = cb.cb_used; + share_mount_state.sm_mount_noauto = do_noauto; pthread_mutex_init(&share_mount_state.sm_lock, NULL); /* For a 'zfs share -a' operation start with a clean slate. */ @@ -7283,7 +7330,7 @@ share_mount(int op, int argc, char **argv) ret = 1; } else { ret = share_mount_one(zhp, op, flags, SA_NO_PROTOCOL, - B_TRUE, options); + B_TRUE, options, B_FALSE); zfs_commit_shares(NULL); zfs_close(zhp); } From e07de81c28223bcd23d47b52c2ac617f270ed557 Mon Sep 17 00:00:00 2001 From: QORTEC Date: Tue, 10 Oct 2023 17:54:19 -0400 Subject: [PATCH 3/5] Updated/added documentation & tests for the changes to `zfs mount -a` zfs-mount.8: - Updated usage and documentation for the changes to `zfs mount` common.run: - Added `zfs_mount_all_002_pos` Makefile.am: - Added `functional/cli_root/zfs_mount/zfs_mount_all_002_pos.ksh` zfs_mount_009_neg.ksh: - Corrected comment spacing. - Removed negative check for `-a filesystem`. - Added negative check for mounting multiple specified filesystems. zfs_mount_011_neg.ksh: - Removed negative check for `-a filesystem` and `-A`. zfs_mount_all_001_pos.ksh: - Corrected comment spacing. zfs_mount_all_002_pos.ksh: - Checks that only the specified filesystem and its children are mounted. Signed-off-by: QORTEC --- man/man8/zfs-mount.8 | 14 +- tests/runfiles/common.run | 6 +- tests/zfs-tests/tests/Makefile.am | 1 + .../cli_root/zfs_mount/zfs_mount_009_neg.ksh | 5 +- .../cli_root/zfs_mount/zfs_mount_011_neg.ksh | 2 +- .../zfs_mount/zfs_mount_all_001_pos.ksh | 4 +- .../zfs_mount/zfs_mount_all_002_pos.ksh | 209 ++++++++++++++++++ 7 files changed, 228 insertions(+), 13 deletions(-) create mode 100644 tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_all_002_pos.ksh diff --git a/man/man8/zfs-mount.8 b/man/man8/zfs-mount.8 index 35aa187cf063..40adf7905906 100644 --- a/man/man8/zfs-mount.8 +++ b/man/man8/zfs-mount.8 @@ -43,7 +43,7 @@ .Cm mount .Op Fl Oflv .Op Fl o Ar options -.Fl a Ns | Ns Ar filesystem +.Fl a Oo filesystem Oc | Oo Fl A Oc Ar filesystem .Nm zfs .Cm unmount .Op Fl fu @@ -61,7 +61,7 @@ Displays all ZFS file systems currently mounted. .Cm mount .Op Fl Oflv .Op Fl o Ar options -.Fl a Ns | Ns Ar filesystem +.Fl a Oo filesystem Oc | Oo Fl A Oc Ar filesystem .Xc Mount ZFS filesystem on a path described by its .Sy mountpoint @@ -80,9 +80,13 @@ Allows mounting in non-empty See .Xr mount 8 for more information. -.It Fl a -Mount all available ZFS file systems. -Invoked automatically as part of the boot process if configured. +.It Fl a Op filesystem +Mount the specified filesystem and its children, provided they are available. +If no filesystem is specified, then all available ZFS file systems are mounted; +may be invoked automatically as part of the boot process if configured. +.It Fl A Ar filesystem +Mount the specified filesystem and its children, provided they are available. +Note: datasets with the `canmount=noauto` property will also be mounted. .It Ar filesystem Mount the specified filesystem. .It Fl o Ar options diff --git a/tests/runfiles/common.run b/tests/runfiles/common.run index 342f56d50d04..eddbf9a1dcbf 100644 --- a/tests/runfiles/common.run +++ b/tests/runfiles/common.run @@ -212,9 +212,9 @@ tags = ['functional', 'cli_root', 'zfs_load-key'] tests = ['zfs_mount_001_pos', 'zfs_mount_002_pos', 'zfs_mount_003_pos', 'zfs_mount_004_pos', 'zfs_mount_005_pos', 'zfs_mount_007_pos', 'zfs_mount_009_neg', 'zfs_mount_010_neg', 'zfs_mount_011_neg', - 'zfs_mount_012_pos', 'zfs_mount_all_001_pos', 'zfs_mount_encrypted', - 'zfs_mount_remount', 'zfs_mount_all_fail', 'zfs_mount_all_mountpoints', - 'zfs_mount_test_race'] + 'zfs_mount_012_pos', 'zfs_mount_all_001_pos', 'zfs_mount_all_002_pos', + 'zfs_mount_encrypted', 'zfs_mount_remount', 'zfs_mount_all_fail', + 'zfs_mount_all_mountpoints', 'zfs_mount_test_race'] tags = ['functional', 'cli_root', 'zfs_mount'] [tests/functional/cli_root/zfs_program] diff --git a/tests/zfs-tests/tests/Makefile.am b/tests/zfs-tests/tests/Makefile.am index 66aff5026f8f..483f00d9e835 100644 --- a/tests/zfs-tests/tests/Makefile.am +++ b/tests/zfs-tests/tests/Makefile.am @@ -739,6 +739,7 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \ functional/cli_root/zfs_mount/zfs_mount_013_pos.ksh \ functional/cli_root/zfs_mount/zfs_mount_014_neg.ksh \ functional/cli_root/zfs_mount/zfs_mount_all_001_pos.ksh \ + functional/cli_root/zfs_mount/zfs_mount_all_002_pos.ksh \ functional/cli_root/zfs_mount/zfs_mount_all_fail.ksh \ functional/cli_root/zfs_mount/zfs_mount_all_mountpoints.ksh \ functional/cli_root/zfs_mount/zfs_mount_encrypted.ksh \ diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_009_neg.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_009_neg.ksh index 02b3477a4452..b583a0c412fb 100755 --- a/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_009_neg.ksh +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_009_neg.ksh @@ -36,7 +36,7 @@ # DESCRIPTION: # Try each 'zfs mount' with inapplicable scenarios to make sure # it returns an error. include: -# * '-a', but also with a specific filesystem. +# * Multiple filesystems specified # # STRATEGY: # 1. Create an array of parameters @@ -53,7 +53,8 @@ for fs in $multifs ; do datasets="$datasets $TESTPOOL/$fs" done -set -A args "$mountall $TESTPOOL/$TESTFS" +set -A args "$mountall $datasets" \ + "$mountcmd $datasets" function setup_all { diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_011_neg.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_011_neg.ksh index 0e0879823619..b59de3dd60d4 100755 --- a/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_011_neg.ksh +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_011_neg.ksh @@ -57,7 +57,7 @@ log_assert "zfs mount fails with bad parameters" log_onexit cleanup fs=$TESTPOOL/$TESTFS -set -A badargs "A" "-A" "-" "-x" "-?" "=" "-o *" "-a" +set -A badargs "A" "-" "-x" "-?" "=" "-o *" for arg in "${badargs[@]}"; do log_mustnot eval "zfs mount $arg $fs >/dev/null 2>&1" diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_all_001_pos.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_all_001_pos.ksh index 5fd963c03997..1ad81136a06c 100755 --- a/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_all_001_pos.ksh +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_all_001_pos.ksh @@ -40,8 +40,8 @@ # 2. Create zfs filesystems within the given pools. # 3. Unmount all the filesystems. # 4. Verify that 'zfs mount -a' command succeed, -# and all available ZFS filesystems are mounted. -# 5. Verify that 'zfs mount' is identical with 'df -F zfs' +# and all available ZFS filesystems are mounted. +# 5. Verify that 'zfs mount' is identical with 'df -F zfs' # verify_runnable "both" diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_all_002_pos.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_all_002_pos.ksh new file mode 100644 index 000000000000..bd4ab7786281 --- /dev/null +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_all_002_pos.ksh @@ -0,0 +1,209 @@ +#!/bin/ksh -p +# +# CDDL HEADER START +# +# The contents of this file are subject to the terms of the +# Common Development and Distribution License (the "License"). +# You may not use this file except in compliance with the License. +# +# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE +# or https://opensource.org/licenses/CDDL-1.0. +# See the License for the specific language governing permissions +# and limitations under the License. +# +# When distributing Covered Code, include this CDDL HEADER in each +# file and include the License file at usr/src/OPENSOLARIS.LICENSE. +# If applicable, add the following below this CDDL HEADER, with the +# fields enclosed by brackets "[]" replaced with your own identifying +# information: Portions Copyright [yyyy] [name of copyright owner] +# +# CDDL HEADER END +# + +# +# Copyright 2007 Sun Microsystems, Inc. All rights reserved. +# Use is subject to license terms. +# + +# +# Copyright (c) 2016 by Delphix. All rights reserved. +# + +. $STF_SUITE/include/libtest.shlib +. $STF_SUITE/tests/functional/cli_root/zfs_mount/zfs_mount.kshlib + +# +# DESCRIPTION: +# Verify that 'zfs mount -a filesystem' succeeds as root. +# +# STRATEGY: +# 1. Create a group of pools with specified vdev. +# 2. Create zfs filesystems within the given pools. +# 3. Unmount all the filesystems. +# 4. Verify that 'zfs mount -a filesystem' command succeed, +# and the related available ZFS filesystems are mounted, +# and the unrelated ZFS filesystems remain unmounted +# 5. Verify that 'zfs mount' is identical with 'df -F zfs' +# + +verify_runnable "both" + +set -A fs "$TESTFS" "$TESTFS1" +set -A ctr "" "$TESTCTR" "$TESTCTR/$TESTCTR1" "$TESTCTR1" +set -A vol "$TESTVOL" "$TESTVOL1" + +# Test the mounted state of root dataset (testpool/testctr) +typeset mnt=$TESTCTR + +function setup_all +{ + typeset -i i=0 + typeset -i j=0 + typeset path + + while (( i < ${#ctr[*]} )); do + + path=${TEST_BASE_DIR%%/}/testroot$$/$TESTPOOL + if [[ -n ${ctr[i]} ]]; then + path=$path/${ctr[i]} + + setup_filesystem "$DISKS" "$TESTPOOL" \ + "${ctr[i]}" "$path" \ + "ctr" + fi + + if is_global_zone ; then + j=0 + while (( j < ${#vol[*]} )); do + setup_filesystem "$DISKS" "$TESTPOOL" \ + "${ctr[i]}/${vol[j]}" \ + "$path/${vol[j]}" \ + "vol" + ((j = j + 1)) + done + fi + + j=0 + while (( j < ${#fs[*]} )); do + setup_filesystem "$DISKS" "$TESTPOOL" \ + "${ctr[i]}/${fs[j]}" \ + "$path/${fs[j]}" + ((j = j + 1)) + done + + ((i = i + 1)) + done + + return 0 +} + +function cleanup_all +{ + typeset -i i=0 + typeset -i j=0 + typeset path + + ((i = ${#ctr[*]} - 1)) + + while (( i >= 0 )); do + if is_global_zone ; then + j=0 + while (( j < ${#vol[*]} )); do + cleanup_filesystem "$TESTPOOL" \ + "${ctr[i]}/${vol[j]}" + ((j = j + 1)) + done + fi + + j=0 + while (( j < ${#fs[*]} )); do + cleanup_filesystem "$TESTPOOL" \ + "${ctr[i]}/${fs[j]}" + ((j = j + 1)) + done + + [[ -n ${ctr[i]} ]] && \ + cleanup_filesystem "$TESTPOOL" "${ctr[i]}" + + ((i = i - 1)) + done + + [[ -d ${TEST_BASE_DIR%%/}/testroot$$ ]] && \ + rm -rf ${TEST_BASE_DIR%%/}/testroot$$ +} + +# +# This function takes a single true/false argument: +# - true: it verifies that the $mnt file system is mounted. +# - false: it verifies that the $mnt file system is unmounted. +# +# In both scenarios, it ensures that the file systems in fs remain unmounted. +# +function verify_related +{ + typeset -i i=0 + typeset -i j=0 + typeset path + typeset logfunc + + while (( i < ${#ctr[*]} )); do + + if $1 && { [[ ${ctr[i]} == $mnt ]] || [[ ${ctr[i]} == $mnt/* ]] }; then + logfunc=log_must + else + logfunc=log_mustnot + fi + + path=$TESTPOOL + [[ -n ${ctr[i]} ]] && \ + path=$path/${ctr[i]} + + if is_global_zone ; then + j=0 + while (( j < ${#vol[*]} )); do + log_mustnot mounted "$path/${vol[j]}" + ((j = j + 1)) + done + fi + + j=0 + while (( j < ${#fs[*]} )); do + $logfunc mounted "$path/${fs[j]}" + ((j = j + 1)) + done + + $logfunc mounted "$path" + + ((i = i + 1)) + done + + return 0 +} + + +log_assert "Verify that 'zfs $mountall $TESTPOOL/$mnt' succeeds as root, " \ + "and all the related available ZFS filesystems are mounted." + +log_onexit cleanup_all + +log_must setup_all + +export __ZFS_POOL_RESTRICT="$TESTPOOL" +log_must zfs $unmountall +unset __ZFS_POOL_RESTRICT + +verify_related false + +export __ZFS_POOL_RESTRICT="$TESTPOOL" +log_must zfs $mountall $TESTPOOL/$mnt +unset __ZFS_POOL_RESTRICT + +verify_related true + +log_note "Verify that 'zfs $mountcmd' will display " \ + "all available ZFS filesystems related to '$TESTPOOL/$mnt' are mounted." + +verify_mount_display + +log_pass "'zfs $mountall $TESTPOOL/$mnt' succeeds as root, " \ + "and all the related available ZFS filesystems are mounted." From 9246667a22449b6c300ed0f0e153b3d5410f8363 Mon Sep 17 00:00:00 2001 From: QORTEC Date: Tue, 10 Oct 2023 18:06:08 -0400 Subject: [PATCH 4/5] Added limited/recursive operation to `zfs unmount` This commit introduces limited/recursive filesystem unmounting by leveraging the existing `zfs unmount -a` codebase with minor additions and modifications. Now, when running `zfs unmount <-a|-A> zpool/dataset`, the command will unmount all datasets that are the specified dataset itself or it's children, provided they are available. In addition, `-A` flag will also mount datasets with `canmount=noauto` property. Changes in `zfs_main.c`: - `HELP_UNMOUNT` - Updated `usage()` message to reflect the changes. - `unshare_unmount()` - Changed `do_all` from `int` to `boolean_t` - Added `boolean_t do_noauto` property; used for mounting datasets with `canmount=noauto` as `canmount=on`. - Added the `-A` flag; when used sets `do_noauto` to `B_TRUE`. - Updated argument check; displaies the correct error messages. - Limited the usage of `<-a|-A> filesystem` to `zfs unmount` only - Added a check; to validate that the specified filesystem is indeed a valid ZFS filesystem. - Modified the 'noauto' check; when unmounting datasets, if `do_noauto` is true, treat the unmount as if `canmount=on`. - Added a check; if `filesystem` is set, skips any datasets that are not `filesystem` itself or it's children. Signed-off-by: QORTEC --- cmd/zfs/zfs_main.c | 50 ++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 42 insertions(+), 8 deletions(-) diff --git a/cmd/zfs/zfs_main.c b/cmd/zfs/zfs_main.c index 3c96ddf97362..8d82a9619012 100644 --- a/cmd/zfs/zfs_main.c +++ b/cmd/zfs/zfs_main.c @@ -347,7 +347,7 @@ get_usage(zfs_help_t idx) "@ ...\n")); case HELP_UNMOUNT: return (gettext("\tunmount [-fu] " - "<-a | filesystem|mountpoint>\n")); + "<-a [filesystem] | [-A] filesystem|mountpoint>\n")); case HELP_UNSHARE: return (gettext("\tunshare " "<-a [nfs|smb] | filesystem|mountpoint>\n")); @@ -7488,7 +7488,8 @@ unshare_unmount_path(int op, char *path, int flags, boolean_t is_manual) static int unshare_unmount(int op, int argc, char **argv) { - int do_all = 0; + boolean_t do_all = B_FALSE; + boolean_t do_noauto = B_FALSE; int flags = 0; int ret = 0; int c; @@ -7497,10 +7498,13 @@ unshare_unmount(int op, int argc, char **argv) char sharesmb[ZFS_MAXPROPLEN]; /* check options */ - while ((c = getopt(argc, argv, op == OP_SHARE ? ":a" : "afu")) != -1) { + while ((c = getopt(argc, argv, op == OP_SHARE ? ":a" : "aAfu")) != -1) { switch (c) { case 'a': - do_all = 1; + do_all = B_TRUE; + break; + case 'A': + do_noauto = B_TRUE; break; case 'f': flags |= MS_FORCE; @@ -7523,7 +7527,7 @@ unshare_unmount(int op, int argc, char **argv) argc -= optind; argv += optind; - if (do_all) { + if (do_all || do_noauto) { /* * We could make use of zfs_for_each() to walk all datasets in * the system, but this would be very inefficient, especially @@ -7555,11 +7559,31 @@ unshare_unmount(int op, int argc, char **argv) argv++; } - if (argc != 0) { + /* check number of arguments */ + if (do_noauto && argc < 1) { + (void) fprintf(stderr, gettext("missing " + "dataset argument\n")); + usage(B_FALSE); + } + if ((op == OP_SHARE && argc != 0) || argc > 1) { (void) fprintf(stderr, gettext("too many arguments\n")); usage(B_FALSE); } + /* + * Limit `-a filesystem` to unmount only + */ + const char *filesystem = NULL; + if (op == OP_MOUNT) + filesystem = argv[0]; + + /* + * Validate filesystem is actually a valid zfs filesystem + */ + if (filesystem != NULL && (zhp = zfs_open(g_zfs, + filesystem, ZFS_TYPE_FILESYSTEM)) == NULL) + return (1); + if (((pool = uu_avl_pool_create("unmount_pool", sizeof (unshare_unmount_node_t), offsetof(unshare_unmount_node_t, un_avlnode), @@ -7621,15 +7645,25 @@ unshare_unmount(int op, int argc, char **argv) NULL, NULL, 0, B_FALSE) == 0); if (strcmp(nfs_mnt_prop, "legacy") == 0) continue; - /* Ignore canmount=noauto mounts */ + /* + * Ignore canmount=noauto mounts if + * 'do_noauto' is set to false (default: false) + */ if (zfs_prop_get_int(zhp, ZFS_PROP_CANMOUNT) == - ZFS_CANMOUNT_NOAUTO) + ZFS_CANMOUNT_NOAUTO && !do_noauto) continue; break; default: break; } + /* + * Skip any dataset that's not related to filesystem. + */ + if (filesystem != NULL && + !zfs_dataset_related(zhp, filesystem)) + continue; + node = safe_malloc(sizeof (unshare_unmount_node_t)); node->un_zhp = zhp; node->un_mountp = safe_strdup(entry.mnt_mountp); From 64400c3308d7b7a5209e18eaf805cca457475eb0 Mon Sep 17 00:00:00 2001 From: QORTEC Date: Tue, 10 Oct 2023 18:12:22 -0400 Subject: [PATCH 5/5] Updated/added documentation & tests for the changes to `zfs unmount -a` zfs-mount.8: - Updated usage and documentation for the changes to `zfs unmount` common.run: - Added `zfs_unmount_all_002_pos` Makefile.am: - Added `functional/cli_root/zfs_unmount/zfs_unmount_all_002_pos.ksh` zfs_unmount_007_neg.ksh: - Removed negative check for `-a filesystem`. - Added negative check for unmounting multiple specified filesystems. zfs_unmount_008_neg.ksh: - Removed negative check for `-A`. zfs_unmount_all_001_pos.ksh: - Corrected comment spacing. zfs_unmount_all_002_pos.ksh: - Checks that only the specified filesystem and its children are unmounted. Signed-off-by: QORTEC --- man/man8/zfs-mount.8 | 23 +- tests/runfiles/common.run | 3 +- tests/zfs-tests/tests/Makefile.am | 1 + .../zfs_unmount/zfs_unmount_007_neg.ksh | 3 +- .../zfs_unmount/zfs_unmount_008_neg.ksh | 2 +- .../zfs_unmount/zfs_unmount_all_001_pos.ksh | 4 +- .../zfs_unmount/zfs_unmount_all_002_pos.ksh | 203 ++++++++++++++++++ 7 files changed, 224 insertions(+), 15 deletions(-) create mode 100644 tests/zfs-tests/tests/functional/cli_root/zfs_unmount/zfs_unmount_all_002_pos.ksh diff --git a/man/man8/zfs-mount.8 b/man/man8/zfs-mount.8 index 40adf7905906..b6f033297d69 100644 --- a/man/man8/zfs-mount.8 +++ b/man/man8/zfs-mount.8 @@ -47,7 +47,7 @@ .Nm zfs .Cm unmount .Op Fl fu -.Fl a Ns | Ns Ar filesystem Ns | Ns Ar mountpoint +.Fl a Oo filesystem Oc | Oo Fl A Oc Ar filesystem | Ar mountpoint . .Sh DESCRIPTION .Bl -tag -width "" @@ -115,21 +115,26 @@ be mounted (e.g. redacted datasets). .Nm zfs .Cm unmount .Op Fl fu -.Fl a Ns | Ns Ar filesystem Ns | Ns Ar mountpoint +.Fl a Oo filesystem Oc | Oo Fl A Oc Ar filesystem | Ar mountpoint .Xc Unmounts currently mounted ZFS file systems. .Bl -tag -width "-a" -.It Fl a -Unmount all available ZFS file systems. +.It Fl a Op filesystem +Unmount the specified filesystem and its children, provided they are available. +If no filesystem is specified, +then all available ZFS file systems are unmounted. Invoked automatically as part of the shutdown process. -.It Fl f -Forcefully unmount the file system, even if it is currently in use. -This option is not supported on Linux. -.It Fl u -Unload keys for any encryption roots unmounted by this command. +.It Fl A Ar filesystem +Unmount the specified filesystem and its children, provided they are available. +Note: datasets with the `canmount=noauto` property will also be unmounted. .It Ar filesystem Ns | Ns Ar mountpoint Unmount the specified filesystem. The command can also be given a path to a ZFS file system mount point on the system. +.It Fl u +Unload keys for any encryption roots unmounted by this command. +.It Fl f +Forcefully unmount the file system, even if it is currently in use. +This option is not supported on Linux. .El .El diff --git a/tests/runfiles/common.run b/tests/runfiles/common.run index eddbf9a1dcbf..b3be16edc159 100644 --- a/tests/runfiles/common.run +++ b/tests/runfiles/common.run @@ -305,7 +305,8 @@ tags = ['functional', 'cli_root', 'zfs_unload-key'] tests = ['zfs_unmount_001_pos', 'zfs_unmount_002_pos', 'zfs_unmount_003_pos', 'zfs_unmount_004_pos', 'zfs_unmount_005_pos', 'zfs_unmount_006_pos', 'zfs_unmount_007_neg', 'zfs_unmount_008_neg', 'zfs_unmount_009_pos', - 'zfs_unmount_all_001_pos', 'zfs_unmount_nested', 'zfs_unmount_unload_keys'] + 'zfs_unmount_all_001_pos', 'zfs_unmount_all_002_pos', 'zfs_unmount_nested', + 'zfs_unmount_unload_keys'] tags = ['functional', 'cli_root', 'zfs_unmount'] [tests/functional/cli_root/zfs_unshare] diff --git a/tests/zfs-tests/tests/Makefile.am b/tests/zfs-tests/tests/Makefile.am index 483f00d9e835..0277afb1e46e 100644 --- a/tests/zfs-tests/tests/Makefile.am +++ b/tests/zfs-tests/tests/Makefile.am @@ -923,6 +923,7 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \ functional/cli_root/zfs_unmount/zfs_unmount_008_neg.ksh \ functional/cli_root/zfs_unmount/zfs_unmount_009_pos.ksh \ functional/cli_root/zfs_unmount/zfs_unmount_all_001_pos.ksh \ + functional/cli_root/zfs_unmount/zfs_unmount_all_002_pos.ksh \ functional/cli_root/zfs_unmount/zfs_unmount_nested.ksh \ functional/cli_root/zfs_unmount/zfs_unmount_unload_keys.ksh \ functional/cli_root/zfs_unshare/cleanup.ksh \ diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_unmount/zfs_unmount_007_neg.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_unmount/zfs_unmount_007_neg.ksh index 1a8665b86ea8..ee8d30db4e73 100755 --- a/tests/zfs-tests/tests/functional/cli_root/zfs_unmount/zfs_unmount_007_neg.ksh +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_unmount/zfs_unmount_007_neg.ksh @@ -37,7 +37,6 @@ # Try each 'zfs unmount' with inapplicable scenarios to make sure # it returns an error. include: # * Multiple filesystem|mountpoint specified -# * '-a', but also with a specific filesystem|mountpoint. # # STRATEGY: # 1. Create an array of parameters @@ -54,7 +53,7 @@ for fs in $multifs ; do datasets="$datasets $TESTPOOL/$fs" done -set -A args "$unmountall $TESTPOOL/$TESTFS" \ +set -A args "$unmountall $datasets" \ "$unmountcmd $datasets" function setup_all diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_unmount/zfs_unmount_008_neg.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_unmount/zfs_unmount_008_neg.ksh index 3524efcd076d..a730a0e7b44d 100755 --- a/tests/zfs-tests/tests/functional/cli_root/zfs_unmount/zfs_unmount_008_neg.ksh +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_unmount/zfs_unmount_008_neg.ksh @@ -73,7 +73,7 @@ log_onexit cleanup fs=$TESTPOOL/$TESTFS vol=$TESTPOOL/vol.$$ snap=$TESTPOOL/$TESTFS@snap.$$ -set -A badargs "A" "-A" "F" "-F" "-" "-x" "-?" +set -A badargs "A" "F" "-F" "-" "-x" "-?" if ! ismounted $fs; then log_must zfs mount $fs diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_unmount/zfs_unmount_all_001_pos.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_unmount/zfs_unmount_all_001_pos.ksh index f98d48ad92d3..8ae80663ad03 100755 --- a/tests/zfs-tests/tests/functional/cli_root/zfs_unmount/zfs_unmount_all_001_pos.ksh +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_unmount/zfs_unmount_all_001_pos.ksh @@ -41,8 +41,8 @@ # 2. Create zfs filesystems within the given pools. # 3. Mount all the filesystems. # 4. Verify that 'zfs unmount -a[f]' command succeed, -# and all available ZFS filesystems are unmounted. -# 5. Verify that 'zfs mount' is identical with 'df -F zfs' +# and all available ZFS filesystems are unmounted. +# 5. Verify that 'zfs mount' is identical with 'df -F zfs' # verify_runnable "both" diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_unmount/zfs_unmount_all_002_pos.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_unmount/zfs_unmount_all_002_pos.ksh new file mode 100644 index 000000000000..fcd34b201cb1 --- /dev/null +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_unmount/zfs_unmount_all_002_pos.ksh @@ -0,0 +1,203 @@ +#!/bin/ksh -p +# +# CDDL HEADER START +# +# The contents of this file are subject to the terms of the +# Common Development and Distribution License (the "License"). +# You may not use this file except in compliance with the License. +# +# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE +# or https://opensource.org/licenses/CDDL-1.0. +# See the License for the specific language governing permissions +# and limitations under the License. +# +# When distributing Covered Code, include this CDDL HEADER in each +# file and include the License file at usr/src/OPENSOLARIS.LICENSE. +# If applicable, add the following below this CDDL HEADER, with the +# fields enclosed by brackets "[]" replaced with your own identifying +# information: Portions Copyright [yyyy] [name of copyright owner] +# +# CDDL HEADER END +# + +# +# Copyright 2007 Sun Microsystems, Inc. All rights reserved. +# Use is subject to license terms. +# + +# +# Copyright (c) 2016 by Delphix. All rights reserved. +# + +. $STF_SUITE/tests/functional/cli_root/zfs_mount/zfs_mount.kshlib +. $STF_SUITE/tests/functional/cli_root/zfs_unmount/zfs_unmount.kshlib + +# +# DESCRIPTION: +# Verify that 'zfs unmount -a[f]' succeeds as root. +# +# STRATEGY: +# 1. Create a group of pools with specified vdev. +# 2. Create zfs filesystems within the given pools. +# 3. Mount all the filesystems. +# 4. Verify that 'zfs unmount -a filesystem' command succeed, +# and the related available ZFS filesystems are unmounted, +# and the unrelated ZFS filesystems remain mounted +# 5. Verify that 'zfs mount' is identical with 'df -F zfs' +# + +verify_runnable "both" + +set -A fs "$TESTFS" "$TESTFS1" +set -A ctr "" "$TESTCTR" "$TESTCTR1" "$TESTCTR/$TESTCTR1" +set -A vol "$TESTVOL" "$TESTVOL1" + +# Test the mounted state of root dataset (testpool/testctr) +typeset mnt=$TESTCTR + +function setup_all +{ + typeset -i i=0 + typeset -i j=0 + typeset path + + while (( i < ${#ctr[*]} )); do + + path=${TEST_BASE_DIR%%/}/testroot$$/$TESTPOOL + if [[ -n ${ctr[i]} ]]; then + path=$path/${ctr[i]} + + setup_filesystem "$DISKS" "$TESTPOOL" \ + "${ctr[i]}" "$path" \ + "ctr" + fi + + if is_global_zone ; then + j=0 + while (( j < ${#vol[*]} )); do + setup_filesystem "$DISKS" "$TESTPOOL" \ + "${ctr[i]}/${vol[j]}" \ + "$path/${vol[j]}" \ + "vol" + ((j = j + 1)) + done + fi + j=0 + while (( j < ${#fs[*]} )); do + setup_filesystem "$DISKS" "$TESTPOOL" \ + "${ctr[i]}/${fs[j]}" \ + "$path/${fs[j]}" + ((j = j + 1)) + done + + ((i = i + 1)) + done + + return 0 +} + +function cleanup_all +{ + typeset -i i=0 + typeset -i j=0 + + ((i = ${#ctr[*]} - 1)) + + while (( i >= 0 )); do + if is_global_zone ; then + j=0 + while (( j < ${#vol[*]} )); do + cleanup_filesystem "$TESTPOOL" \ + "${ctr[i]}/${vol[j]}" + ((j = j + 1)) + done + fi + + j=0 + while (( j < ${#fs[*]} )); do + cleanup_filesystem "$TESTPOOL" \ + "${ctr[i]}/${fs[j]}" + ((j = j + 1)) + done + + [[ -n ${ctr[i]} ]] && \ + cleanup_filesystem "$TESTPOOL" "${ctr[i]}" + + ((i = i - 1)) + done + + [[ -d ${TEST_BASE_DIR%%/}/testroot$$ ]] && \ + rm -rf ${TEST_BASE_DIR%%/}/testroot$$ +} + +# +# This function verifies that the file systems in $mnt are unmounted. +# Next, it ensures that all other file systems in remain mounted. +# +function verify_related +{ + typeset -i i=0 + typeset -i j=0 + typeset path + + while (( i < ${#ctr[*]} )); do + + if { [[ ${ctr[i]} == $mnt ]] || [[ ${ctr[i]} == $mnt/* ]] }; then + logfunc=log_must + else + logfunc=log_mustnot + fi + + path=$TESTPOOL + [[ -n ${ctr[i]} ]] && \ + path=$path/${ctr[i]} + + if is_global_zone ; then + j=0 + while (( j < ${#vol[*]} )); do + log_must unmounted "$path/${vol[j]}" + ((j = j + 1)) + done + fi + + j=0 + while (( j < ${#fs[*]} )); do + $logfunc unmounted "$path/${fs[j]}" + ((j = j + 1)) + done + + $logfunc unmounted "$path" + + ((i = i + 1)) + done + + return 0 +} + + +log_assert "Verify that 'zfs $unmountall $TESTPOOL/$mnt' succeeds as root, " \ + "and all the related available ZFS filesystems are unmounted." + +log_onexit cleanup_all + +log_must setup_all + +typeset opt +for opt in "-a" "-fa"; do + export __ZFS_POOL_RESTRICT="$TESTPOOL" + log_must zfs $mountall + unset __ZFS_POOL_RESTRICT + + export __ZFS_POOL_RESTRICT="$TESTPOOL" + log_must zfs unmount $opt $TESTPOOL/$mnt + unset __ZFS_POOL_RESTRICT + + log_must verify_related + log_note "Verify that 'zfs $mountcmd' will display " \ + "all available ZFS filesystems related to '$TESTPOOL/$mnt' are unmounted." + log_must verify_mount_display + +done + +log_pass "'zfs mount -[f]a $TESTPOOL/$mnt' succeeds as root, " \ + "and all the related available ZFS filesystems are unmounted."