From 79d6a1b1dab474733f3879de0f0fbfe30e4de436 Mon Sep 17 00:00:00 2001 From: Paul Dagnelie Date: Wed, 26 Aug 2020 21:38:27 -0700 Subject: [PATCH] Always track temporary fses and snapshots for accounting The root cause of the issue is that we only occasionally do as the comments in the code suggest and actually ignore the %recv dataset when it comes to filesystem limit tracking. Specifically, the only time we ignore it is when initializing the filesystem and snapshot limit values; when creating a new %recv dataset or deleting one, we always update the bookkeeping. This causes a problem if you init the fs count on a filesystem that already has a %recv dataset, since the bookmarking will be decremented but not incremented. This is resolved in this patch by simply always tracking the %recv dataset as a child. Reviewed-by: Matt Ahrens Reviewed by: Jerry Jelinek Signed-off-by: Paul Dagnelie Closes #10791 --- module/zfs/dsl_dir.c | 23 ++---- tests/runfiles/common.run | 2 +- .../cli_root/zfs_receive/Makefile.am | 1 + .../zfs_receive/zfs_receive_new_props.ksh | 77 +++++++++++++++++++ 4 files changed, 85 insertions(+), 18 deletions(-) create mode 100755 tests/zfs-tests/tests/functional/cli_root/zfs_receive/zfs_receive_new_props.ksh diff --git a/module/zfs/dsl_dir.c b/module/zfs/dsl_dir.c index 73b7943b8f..29672e9a61 100644 --- a/module/zfs/dsl_dir.c +++ b/module/zfs/dsl_dir.c @@ -121,13 +121,6 @@ * and updated by dsl_fs_ss_count_adjust(). A new limit value is setup in * dsl_dir_activate_fs_ss_limit() and the counts are adjusted, if necessary, by * dsl_dir_init_fs_ss_count(). - * - * There is a special case when we receive a filesystem that already exists. In - * this case a temporary clone name of %X is created (see dmu_recv_begin). We - * never update the filesystem counts for temporary clones. - * - * Likewise, we do not update the snapshot counts for temporary snapshots, - * such as those created by zfs diff. */ extern inline dsl_dir_phys_t *dsl_dir_phys(dsl_dir_t *dd); @@ -593,11 +586,9 @@ dsl_dir_init_fs_ss_count(dsl_dir_t *dd, dmu_tx_t *tx) &chld_dd)); /* - * Ignore hidden ($FREE, $MOS & $ORIGIN) objsets and - * temporary datasets. + * Ignore hidden ($FREE, $MOS & $ORIGIN) objsets. */ - if (chld_dd->dd_myname[0] == '$' || - chld_dd->dd_myname[0] == '%') { + if (chld_dd->dd_myname[0] == '$') { dsl_dir_rele(chld_dd, FTAG); continue; } @@ -910,14 +901,12 @@ dsl_fs_ss_count_adjust(dsl_dir_t *dd, int64_t delta, const char *prop, strcmp(prop, DD_FIELD_SNAPSHOT_COUNT) == 0); /* - * When we receive an incremental stream into a filesystem that already - * exists, a temporary clone is created. We don't count this temporary - * clone, whose name begins with a '%'. We also ignore hidden ($FREE, - * $MOS & $ORIGIN) objsets. + * We don't do accounting for hidden ($FREE, $MOS & $ORIGIN) objsets. */ - if ((dd->dd_myname[0] == '%' || dd->dd_myname[0] == '$') && - strcmp(prop, DD_FIELD_FILESYSTEM_COUNT) == 0) + if (dd->dd_myname[0] == '$' && strcmp(prop, + DD_FIELD_FILESYSTEM_COUNT) == 0) { return; + } /* * e.g. if renaming a dataset with no snapshots, count adjustment is 0 diff --git a/tests/runfiles/common.run b/tests/runfiles/common.run index 615c4efc46..851488602f 100644 --- a/tests/runfiles/common.run +++ b/tests/runfiles/common.run @@ -216,7 +216,7 @@ tests = ['zfs_receive_001_pos', 'zfs_receive_002_pos', 'zfs_receive_003_pos', 'zfs_receive_016_pos', 'receive-o-x_props_override', 'zfs_receive_from_encrypted', 'zfs_receive_to_encrypted', 'zfs_receive_raw', 'zfs_receive_raw_incremental', 'zfs_receive_-e', - 'zfs_receive_raw_-d', 'zfs_receive_from_zstd'] + 'zfs_receive_raw_-d', 'zfs_receive_from_zstd', 'zfs_receive_new_props'] tags = ['functional', 'cli_root', 'zfs_receive'] [tests/functional/cli_root/zfs_rename] diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_receive/Makefile.am b/tests/zfs-tests/tests/functional/cli_root/zfs_receive/Makefile.am index f13a955d83..2b281e71c2 100644 --- a/tests/zfs-tests/tests/functional/cli_root/zfs_receive/Makefile.am +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_receive/Makefile.am @@ -21,6 +21,7 @@ dist_pkgdata_SCRIPTS = \ receive-o-x_props_override.ksh \ zfs_receive_from_encrypted.ksh \ zfs_receive_from_zstd.ksh \ + zfs_receive_new_props.ksh \ zfs_receive_to_encrypted.ksh \ zfs_receive_raw.ksh \ zfs_receive_raw_incremental.ksh \ diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_receive/zfs_receive_new_props.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_receive/zfs_receive_new_props.ksh new file mode 100755 index 0000000000..54f13355f5 --- /dev/null +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_receive/zfs_receive_new_props.ksh @@ -0,0 +1,77 @@ +#!/bin/ksh -p +# +# CDDL HEADER START +# +# This file and its contents are supplied under the terms of the +# Common Development and Distribution License ("CDDL"), version 1.0. +# You may only use this file in accordance with the terms of version +# 1.0 of the CDDL. +# +# A full copy of the text of the CDDL should have accompanied this +# source. A copy of the CDDL is also available via the Internet at +# http://www.illumos.org/license/CDDL. +# +# CDDL HEADER END +# + +# +# Copyright (c) 2020 by Delphix. All rights reserved. +# + +. $STF_SUITE/include/libtest.shlib + +# +# DESCRIPTION: +# ZFS receive test to handle Issue #10698 +# +# STRATEGY: +# 1. Create a pool with filesystem_limits disabled +# 2. Create a filesystem on that pool +# 3. Enable filesystem limits on that pool +# 4. On a pool with filesystem limits enabled, create a filesystem and set a +# limit +# 5. Snapshot limited filesystem +# 6. send -R limited filesystem and receive over filesystem with limits disabled +# + +verify_runnable "both" + +function cleanup +{ + destroy_pool "$poolname" + destroy_pool "$rpoolname" + log_must rm -f "$vdevfile" + log_must rm -f "$rvdevfile" + log_must rm -f "$streamfile" +} + +log_onexit cleanup + +log_assert "ZFS should handle receiving streams with filesystem limits on \ + pools where the feature was recently enabled" + +poolname=sendpool +rpoolname=recvpool +vdevfile="$TEST_BASE_DIR/vdevfile.$$" +rvdevfile="$TEST_BASE_DIR/rvdevfile.$$" +sendfs="$poolname/fs" +recvfs="$rpoolname/rfs" +streamfile="$TEST_BASE_DIR/streamfile.$$" + +log_must truncate -s $MINVDEVSIZE "$rvdevfile" +log_must truncate -s $MINVDEVSIZE "$vdevfile" +log_must zpool create -O mountpoint=none -o feature@filesystem_limits=disabled \ + "$rpoolname" "$rvdevfile" +log_must zpool create -O mountpoint=none "$poolname" "$vdevfile" + +log_must zfs create "$recvfs" +log_must zpool set feature@filesystem_limits=enabled "$rpoolname" + +log_must zfs create -o filesystem_limit=100 "$sendfs" +log_must zfs snapshot "$sendfs@a" + +log_must zfs send -R "$sendfs@a" >"$streamfile" +log_must eval "zfs recv -svuF $recvfs <$streamfile" + +log_pass "ZFS can handle receiving streams with filesystem limits on \ + pools where the feature was recently enabled"