Skip to content

Commit

Permalink
Dont iterate through filesystems unnecessarily
Browse files Browse the repository at this point in the history
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 <tcaputi@datto.com>
  • Loading branch information
Tom Caputi committed Mar 27, 2019
1 parent c048dda commit ac7ee3c
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 2 deletions.
14 changes: 12 additions & 2 deletions cmd/zfs/zfs_iter.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
20 changes: 20 additions & 0 deletions cmd/zfs/zfs_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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."

0 comments on commit ac7ee3c

Please sign in to comment.