From ac7ee3c460921311ef4ed50ce0c3836d991bcb5c Mon Sep 17 00:00:00 2001 From: Tom Caputi Date: Mon, 25 Mar 2019 17:26:40 -0400 Subject: [PATCH] Dont 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 conatined 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: as a convenient way to list out properties of all snapshots of a given dataset without having to use the depth limit. Signed-off-by: Tom Caputi --- cmd/zfs/zfs_iter.c | 14 +++++++++++-- cmd/zfs/zfs_main.c | 20 +++++++++++++++++++ .../cli_root/zfs_get/zfs_get_009_pos.ksh | 5 +++++ 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/cmd/zfs/zfs_iter.c b/cmd/zfs/zfs_iter.c index 9ad6919388ae..b3253c817927 100644 --- a/cmd/zfs/zfs_iter.c +++ b/cmd/zfs/zfs_iter.c @@ -134,13 +134,23 @@ 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_types & ZFS_TYPE_FILESYSTEM)) && + 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); + } + if (((zfs_get_type(zhp) & (ZFS_TYPE_SNAPSHOT | ZFS_TYPE_BOOKMARK)) == 0) && include_bmarks) (void) zfs_iter_bookmarks(zhp, zfs_callback, data); diff --git a/cmd/zfs/zfs_main.c b/cmd/zfs/zfs_main.c index 21c4d4334eb4..57eb305281d2 100644 --- 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 '). + */ + 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 '). + */ + 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/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 383b19ca8d66..2d97c5918acd 100755 --- 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 ' 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 ' should get expected output."