Do not iterate through filesystems unnecessarily

Currently, when attempting to list snapshots ZFS may do a lot of
extra work checking child datasets. This is because the code does
not realize that it will not be able to reach any snapshots
contained within snapshots that are at the depth limit since the
snapshots of those datasets are counted as an additional layer
deeper. This patch corrects this issue.

In addition, this patch adds the ability to do perform the commands:

$ zfs list -t snapshot <dataset>
$ zfs get -t snapshot <prop> <dataset>

as a convenient way to list out properties of all snapshots of a
given dataset without having to use the depth limit.

Reviewed-by: Alek Pinchuk <apinchuk@datto.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Richard Laager <rlaager@wiktel.com>
Reviewed-by: Matt Ahrens <mahrens@delphix.com>
Reviewed-by: George Melikov <mail@gmelikov.ru>
Signed-off-by: Tom Caputi <tcaputi@datto.com>
Closes #8539

diff --git a/cmd/zfs/zfs_iter.c b/cmd/zfs/zfs_iter.c
index 9ad6919388aedc6750ca6b78a86e3d66aa721050..d10bbed7dbd62101fbb07d48dd1bbb2de88b6453 100644 (file)
--- a/cmd/zfs/zfs_iter.c
+++ b/cmd/zfs/zfs_iter.c
@@ -134,16 +134,31 @@ zfs_callback(zfs_handle_t *zhp, void *data)
            ((cb->cb_flags & ZFS_ITER_DEPTH_LIMIT) == 0 ||
            cb->cb_depth < cb->cb_depth_limit)) {
                cb->cb_depth++;
-               if (zfs_get_type(zhp) == ZFS_TYPE_FILESYSTEM)
+
+               /*
+                * If we are not looking for filesystems, we don't need to
+                * recurse into filesystems when we are at our depth limit.
+                */
+               if ((cb->cb_depth < cb->cb_depth_limit ||
+                   (cb->cb_flags & ZFS_ITER_DEPTH_LIMIT) == 0 ||
+                   (cb->cb_types &
+                   (ZFS_TYPE_FILESYSTEM | ZFS_TYPE_VOLUME))) &&
+                   zfs_get_type(zhp) == ZFS_TYPE_FILESYSTEM) {
                        (void) zfs_iter_filesystems(zhp, zfs_callback, data);
+               }
+
                if (((zfs_get_type(zhp) & (ZFS_TYPE_SNAPSHOT |
-                   ZFS_TYPE_BOOKMARK)) == 0) && include_snaps)
+                   ZFS_TYPE_BOOKMARK)) == 0) && include_snaps) {
                        (void) zfs_iter_snapshots(zhp,
-                           (cb->cb_flags & ZFS_ITER_SIMPLE) != 0, zfs_callback,
-                           data, 0, 0);
+                           (cb->cb_flags & ZFS_ITER_SIMPLE) != 0,
+                           zfs_callback, data, 0, 0);
+               }
+
                if (((zfs_get_type(zhp) & (ZFS_TYPE_SNAPSHOT |
-                   ZFS_TYPE_BOOKMARK)) == 0) && include_bmarks)
+                   ZFS_TYPE_BOOKMARK)) == 0) && include_bmarks) {
                        (void) zfs_iter_bookmarks(zhp, zfs_callback, data);
+               }
+
                cb->cb_depth--;
        }
 

diff --git a/cmd/zfs/zfs_main.c b/cmd/zfs/zfs_main.c
index 21c4d4334eb4c2645a5a4ca111d37f4fba6ee1b5..57eb305281d2bf807a0f2680e9af830188493460 100644 (file)
--- a/cmd/zfs/zfs_main.c
+++ b/cmd/zfs/zfs_main.c
@@ -1922,6 +1922,16 @@ zfs_do_get(int argc, char **argv)
 
        fields = argv[0];
 
+       /*
+        * Handle users who want to get all snapshots of the current
+        * dataset (ex. 'zfs get -t snapshot refer <dataset>').
+        */
+       if (types == ZFS_TYPE_SNAPSHOT &&
+           (flags & ZFS_ITER_RECURSE) == 0 && limit == 0) {
+               flags |= (ZFS_ITER_DEPTH_LIMIT | ZFS_ITER_RECURSE);
+               limit = 1;
+       }
+
        if (zprop_get_list(g_zfs, fields, &cb.cb_proplist, ZFS_TYPE_DATASET)
            != 0)
                usage(B_FALSE);
@@ -3416,6 +3426,16 @@ zfs_do_list(int argc, char **argv)
        if (strcmp(fields, "space") == 0 && types_specified == B_FALSE)
                types &= ~ZFS_TYPE_SNAPSHOT;
 
+       /*
+        * Handle users who want to list all snapshots of the current
+        * dataset (ex. 'zfs list -t snapshot <dataset>').
+        */
+       if (types == ZFS_TYPE_SNAPSHOT &&
+           (flags & ZFS_ITER_RECURSE) == 0 && limit == 0) {
+               flags |= (ZFS_ITER_DEPTH_LIMIT | ZFS_ITER_RECURSE);
+               limit = 1;
+       }
+
        /*
         * If the user specifies '-o all', the zprop_get_list() doesn't
         * normally include the name of the dataset.  For 'zfs list', we always

diff --git a/tests/zfs-tests/include/libtest.shlib b/tests/zfs-tests/include/libtest.shlib
index 748a4f96dba979cebec244e2f4dbb665a4122fd0..ea9448353b68fc0732b9542015ae70310fa63386 100644 (file)
--- a/tests/zfs-tests/include/libtest.shlib
+++ b/tests/zfs-tests/include/libtest.shlib
@@ -2594,7 +2594,6 @@ function verify_opt_p_ops
                                        "when ops is $ops."
                        fi
                        log_must datasetexists $dataset
-                       log_mustnot snapexists $dataset
                        ;;
                *)
                        log_fail "$ops is not supported."

diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_009_pos.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_009_pos.ksh
index 383b19ca8d66dbd72496d617d4d35efa96f3ad53..2d97c5918acd9de104ddae57628aacc858c84eb7 100755 (executable)
--- a/tests/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_009_pos.ksh
+++ b/tests/zfs-tests/tests/functional/cli_root/zfs_get/zfs_get_009_pos.ksh
@@ -87,5 +87,10 @@ for dp in ${depth_array[@]}; do
        (( old_val=dp ))
 done
 
+# Ensure 'zfs get -t snapshot <dataset>' works as though -d 1 was specified
+log_must eval "zfs get -H -t snapshot -o name creation $DEPTH_FS > $DEPTH_OUTPUT"
+log_must eval "zfs get -H -t snapshot -d 1 -o name creation $DEPTH_FS > $EXPECT_OUTPUT"
+log_must diff $DEPTH_OUTPUT $EXPECT_OUTPUT
+
 log_pass "'zfs get -d <n>' should get expected output."