diff --git a/include/sys/dsl_dataset.h b/include/sys/dsl_dataset.h index 9db39d5f81f5..6ffc5f0e1af0 100644 --- a/include/sys/dsl_dataset.h +++ b/include/sys/dsl_dataset.h @@ -188,6 +188,9 @@ typedef struct dsl_dataset { uint64_t ds_bookmarks_obj; /* DMU_OTN_ZAP_METADATA */ avl_tree_t ds_bookmarks; /* dsl_bookmark_node_t */ + /* used in snapshot check function */ + list_node_t ds_check; + /* has internal locking: */ dsl_deadlist_t ds_deadlist; bplist_t ds_pending_deadlist; @@ -446,6 +449,8 @@ void dsl_dataset_clone_swap_sync_impl(dsl_dataset_t *clone, dsl_dataset_t *origin_head, dmu_tx_t *tx); int dsl_dataset_snapshot_check_impl(dsl_dataset_t *ds, const char *snapname, dmu_tx_t *tx, boolean_t recv, uint64_t cnt, cred_t *cr); +int dsl_dataset_snapshot_reserve_space(dsl_dataset_t *ds, dmu_tx_t *tx); + void dsl_dataset_snapshot_sync_impl(dsl_dataset_t *ds, const char *snapname, dmu_tx_t *tx); diff --git a/module/zfs/dmu_recv.c b/module/zfs/dmu_recv.c index 48c3705c65a6..5c55325997bb 100644 --- a/module/zfs/dmu_recv.c +++ b/module/zfs/dmu_recv.c @@ -2935,6 +2935,11 @@ dmu_recv_end_check(void *arg, dmu_tx_t *tx) } error = dsl_dataset_snapshot_check_impl(origin_head, drc->drc_tosnap, tx, B_TRUE, 1, drc->drc_cred); + if (error != 0) { + dsl_dataset_rele(origin_head, FTAG); + return (error); + } + error = dsl_dataset_snapshot_reserve_space(origin_head, tx); dsl_dataset_rele(origin_head, FTAG); if (error != 0) return (error); @@ -2943,6 +2948,10 @@ dmu_recv_end_check(void *arg, dmu_tx_t *tx) } else { error = dsl_dataset_snapshot_check_impl(drc->drc_ds, drc->drc_tosnap, tx, B_TRUE, 1, drc->drc_cred); + if (error != 0) + return (error); + + error = dsl_dataset_snapshot_reserve_space(drc->drc_ds, tx); } return (error); } diff --git a/module/zfs/dnode_sync.c b/module/zfs/dnode_sync.c index 4e002d326a90..d1ca08e8b496 100644 --- a/module/zfs/dnode_sync.c +++ b/module/zfs/dnode_sync.c @@ -457,7 +457,7 @@ dnode_sync_free_range(void *arg, uint64_t blkid, uint64_t nblks) void dnode_evict_dbufs(dnode_t *dn) { - dmu_buf_impl_t *db_marker; + dmu_buf_impl_t *db_marker, *insert_point; dmu_buf_impl_t *db, *db_next; db_marker = kmem_alloc(sizeof (dmu_buf_impl_t), KM_SLEEP); @@ -474,12 +474,30 @@ dnode_evict_dbufs(dnode_t *dn) mutex_enter(&db->db_mtx); if (db->db_state != DB_EVICTING && zfs_refcount_is_zero(&db->db_holds)) { + /* + * It is rare but if dn_dbufs has a dbuf with + * this same bookmark but having db_state of + * DB_EVICTING, we won't be able to insert + * the db_marker. Work backward to find the correct + * insertion point. + */ db_marker->db_level = db->db_level; db_marker->db_blkid = db->db_blkid; db_marker->db_state = DB_SEARCH; - avl_insert_here(&dn->dn_dbufs, db_marker, db, - AVL_BEFORE); - + insert_point = AVL_PREV(&dn->dn_dbufs, db); + while (insert_point != NULL && + insert_point->db_level == db->db_level && + insert_point->db_blkid == db->db_blkid) { + insert_point = AVL_PREV(&dn->dn_dbufs, + insert_point); + } + if (insert_point == NULL) { + avl_insert_here(&dn->dn_dbufs, db_marker, + avl_first(&dn->dn_dbufs), AVL_BEFORE); + } else { + avl_insert_here(&dn->dn_dbufs, db_marker, + insert_point, AVL_AFTER); + } /* * We need to use the "marker" dbuf rather than * simply getting the next dbuf, because diff --git a/module/zfs/dsl_dataset.c b/module/zfs/dsl_dataset.c index aa37214d0c8e..93b270b9844e 100644 --- a/module/zfs/dsl_dataset.c +++ b/module/zfs/dsl_dataset.c @@ -1435,7 +1435,7 @@ dsl_dataset_dirty(dsl_dataset_t *ds, dmu_tx_t *tx) } } -static int +int dsl_dataset_snapshot_reserve_space(dsl_dataset_t *ds, dmu_tx_t *tx) { uint64_t asize; @@ -1515,13 +1515,10 @@ dsl_dataset_snapshot_check_impl(dsl_dataset_t *ds, const char *snapname, return (error); } - error = dsl_dataset_snapshot_reserve_space(ds, tx); - if (error != 0) - return (error); - return (0); } + int dsl_dataset_snapshot_check(void *arg, dmu_tx_t *tx) { @@ -1627,12 +1624,17 @@ dsl_dataset_snapshot_check(void *arg, dmu_tx_t *tx) nvlist_free(cnt_track); } + list_t datasets; + dsl_dataset_t *check_ds; + list_create(&datasets, sizeof (struct dsl_dataset), + offsetof(struct dsl_dataset, ds_check)); for (pair = nvlist_next_nvpair(ddsa->ddsa_snaps, NULL); pair != NULL; pair = nvlist_next_nvpair(ddsa->ddsa_snaps, pair)) { int error = 0; dsl_dataset_t *ds; char *name, *atp = NULL; char dsname[ZFS_MAX_DATASET_NAME_LEN]; + boolean_t dsheld = B_FALSE; name = nvpair_name(pair); if (strlen(name) >= ZFS_MAX_DATASET_NAME_LEN) @@ -1644,13 +1646,14 @@ dsl_dataset_snapshot_check(void *arg, dmu_tx_t *tx) if (error == 0) (void) strlcpy(dsname, name, atp - name + 1); } - if (error == 0) + if (error == 0) { error = dsl_dataset_hold(dp, dsname, FTAG, &ds); + dsheld = (error == 0); + } if (error == 0) { /* passing 0/NULL skips dsl_fs_ss_limit_check */ error = dsl_dataset_snapshot_check_impl(ds, atp + 1, tx, B_FALSE, 0, NULL); - dsl_dataset_rele(ds, FTAG); } if (error != 0) { @@ -1658,9 +1661,45 @@ dsl_dataset_snapshot_check(void *arg, dmu_tx_t *tx) fnvlist_add_int32(ddsa->ddsa_errors, name, error); } + if (dsheld) + dsl_dataset_rele(ds, FTAG); rv = error; + } else { + list_insert_tail(&datasets, ds); } } + if (rv != 0) { + while ((check_ds = list_remove_head(&datasets)) != NULL) { + dsl_dataset_rele(check_ds, FTAG); + } + list_destroy(&datasets); + return (rv); + } + + /* Now reserve space since we passed all the checks. */ + while ((check_ds = list_remove_head(&datasets)) != NULL) { + int error = 0; + + /* + * This accumulates dsl_dir_t dd_space_to_write which may + * need to be undone if we get ENOSPC while reserving. + */ + error = dsl_dataset_snapshot_reserve_space(check_ds, tx); + dsl_dataset_rele(check_ds, FTAG); + if (error != 0) + rv = error; + } + list_destroy(&datasets); + if (rv == 0) + return (rv); + + ASSERT3U(rv, ==, ENOSPC); + dsl_dir_t *dd; + + /* Undo the reserved space since no snapshot will be taken. */ + while ((dd = txg_list_remove(&dp->dp_dirty_dirs, tx->tx_txg)) != NULL) { + dsl_dir_sync(dd, tx); + } return (rv); } @@ -1984,6 +2023,11 @@ dsl_dataset_snapshot_tmp_check(void *arg, dmu_tx_t *tx) dsl_dataset_rele(ds, FTAG); return (error); } + error = dsl_dataset_snapshot_reserve_space(ds, tx); + if (error != 0) { + dsl_dataset_rele(ds, FTAG); + return (error); + } dsl_dataset_rele(ds, FTAG); return (0); diff --git a/tests/runfiles/common.run b/tests/runfiles/common.run index d6d3874f684f..6022d530c6f4 100644 --- a/tests/runfiles/common.run +++ b/tests/runfiles/common.run @@ -804,7 +804,7 @@ tests = ['clone_001_pos', 'rollback_001_pos', 'rollback_002_pos', 'snapshot_006_pos', 'snapshot_007_pos', 'snapshot_008_pos', 'snapshot_009_pos', 'snapshot_010_pos', 'snapshot_011_pos', 'snapshot_012_pos', 'snapshot_013_pos', 'snapshot_014_pos', - 'snapshot_017_pos'] + 'snapshot_017_pos', 'snapshot_fail_existing.ksh'] tags = ['functional', 'snapshot'] [tests/functional/snapused] diff --git a/tests/zfs-tests/tests/functional/snapshot/Makefile.am b/tests/zfs-tests/tests/functional/snapshot/Makefile.am index 783133a643a1..1b8e65cba2d1 100644 --- a/tests/zfs-tests/tests/functional/snapshot/Makefile.am +++ b/tests/zfs-tests/tests/functional/snapshot/Makefile.am @@ -22,7 +22,8 @@ dist_pkgdata_SCRIPTS = \ snapshot_014_pos.ksh \ snapshot_015_pos.ksh \ snapshot_016_pos.ksh \ - snapshot_017_pos.ksh + snapshot_017_pos.ksh \ + snapshot_fail_existing.ksh dist_pkgdata_DATA = \ snapshot.cfg diff --git a/tests/zfs-tests/tests/functional/snapshot/snapshot_fail_existing.ksh b/tests/zfs-tests/tests/functional/snapshot/snapshot_fail_existing.ksh new file mode 100755 index 000000000000..47d935524bbd --- /dev/null +++ b/tests/zfs-tests/tests/functional/snapshot/snapshot_fail_existing.ksh @@ -0,0 +1,189 @@ +#! /bin/ksh -p +# +# 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. +# + +# +# Copyright (c) 2013, 2016 by Delphix. All rights reserved. +# Copyright (c) 2019 by Datto Inc. All rights reserved. +# + +. $STF_SUITE/include/libtest.shlib +. $STF_SUITE/tests/functional/snapshot/snapshot.cfg + +# +# DESCRIPTION: +# Create a snapshot from regular filesystem, volume, +# or filesystem upon volume, Build a clone file system +# from the snapshot. Then test that snapshot -r handles +# the case of an already existing snap. +# +# STRATEGY: +# 1. Create snapshot use 3 combination: +# - Regular filesystem +# - Regular volume +# - Filesystem upon volume +# 2. Clone a new file system from the snapshot +# 3. Verify snap -r fails +# + +verify_runnable "both" + +# Setup array, 4 elements as a group, refer to: +# i+0: name of a snapshot +# i+1: mountpoint of the snapshot +# i+2: clone created from the snapshot +# i+3: mountpoint of the clone + +set -A args "$SNAPFS" "$SNAPDIR" "$TESTPOOL/$TESTCLONE" "$TESTDIR.0" \ + "$SNAPFS1" "$SNAPDIR3" "$TESTPOOL/$TESTCLONE1" "" \ + "$SNAPFS2" "$SNAPDIR2" "$TESTPOOL1/$TESTCLONE2" "$TESTDIR.2" + +function setup_all +{ + create_pool $TESTPOOL1 ${ZVOL_DEVDIR}/$TESTPOOL/$TESTVOL + log_must zfs create $TESTPOOL1/$TESTFS + log_must zfs set mountpoint=$TESTDIR2 $TESTPOOL1/$TESTFS + + return 0 +} + +function cleanup_all +{ + typeset -i i=0 + typeset ds + typeset snap + + for ds in $ctr/$TESTVOL1 $ctr/$TESTCLONE \ + $TESTPOOL1/$TESTCLONE1 $TESTPOOL1/$TESTCLONE2 \ + $TESTPOOL1/$TESTFS; + do + destroy_dataset $ds "-rf" + done + + destroy_pool $TESTPOOL1 + + for ds in $TESTPOOL/$TESTCLONE1 $TESTPOOL/$TESTVOL \ + $TESTPOOL/$TESTCLONE $TESTPOOL/$TESTFS; + do + destroy_dataset $ds "-rf" + done + + for snap in $ctr/$TESTFS1@$TESTSNAP1 \ + $snappool $snapvol $snapctr $snapctrvol \ + $snapctrclone $snapctrfs + do + snapexists $snap && destroy_dataset $snap "-rf" + done + + while (( i < ${#args[*]} )); do + snapexists ${args[i]} && \ + destroy_dataset "${args[i]}" "-Rf" + + [[ -d ${args[i+3]} ]] && \ + log_must rm -rf ${args[i+3]} + + [[ -d ${args[i+1]} ]] && \ + log_must rm -rf ${args[i+1]} + + (( i = i + 4 )) + done + + [[ -d $TESTDIR2 ]] && \ + log_must rm -rf $TESTDIR2 + + return 0 +} + +log_assert "Verify a cloned file system is writable." + +log_onexit cleanup_all + +setup_all + +[[ -n $TESTDIR ]] && \ + log_must rm -rf $TESTDIR/* > /dev/null 2>&1 + +typeset -i COUNT=10 +typeset -i i=0 + +for mtpt in $TESTDIR $TESTDIR2 ; do + log_note "Populate the $mtpt directory (prior to snapshot)" + typeset -i j=1 + while [[ $j -le $COUNT ]]; do + log_must file_write -o create -f $mtpt/before_file$j \ + -b $BLOCKSZ -c $NUM_WRITES -d $j + + (( j = j + 1 )) + done +done + +while (( i < ${#args[*]} )); do + # + # Take a snapshot of the test file system. + # + log_must zfs snapshot ${args[i]} + + # + # Clone a new file system from the snapshot + # + log_must zfs clone ${args[i]} ${args[i+2]} + if [[ -n ${args[i+3]} ]] ; then + log_must zfs set mountpoint=${args[i+3]} ${args[i+2]} + + FILE_COUNT=`ls -Al ${args[i+3]} | grep -v "total" \ + | grep -v "\.zfs" | wc -l` + if [[ $FILE_COUNT -ne $COUNT ]]; then + ls -Al ${args[i+3]} + log_fail "AFTER: ${args[i+3]} contains $FILE_COUNT files(s)." + fi + + log_note "Verify the ${args[i+3]} directory is writable" + j=1 + while [[ $j -le $COUNT ]]; do + log_must file_write -o create -f ${args[i+3]}/after_file$j \ + -b $BLOCKSZ -c $NUM_WRITES -d $j + (( j = j + 1 )) + done + + FILE_COUNT=`ls -Al ${args[i+3]}/after* | grep -v "total" | wc -l` + if [[ $FILE_COUNT -ne $COUNT ]]; then + ls -Al ${args[i+3]} + log_fail "${args[i+3]} contains $FILE_COUNT after* files(s)." + fi + fi + + (( i = i + 4 )) +done + +# Set up to take the snap -r that we expect to fail. +ctr=$TESTPOOL/$TESTCTR +ctrfs=$ctr/$TESTFS1 +ctrclone=$ctr/$TESTCLONE +ctrvol=$ctr/$TESTVOL1 +snappool=$SNAPPOOL +snapfs=$SNAPFS +snapctr=$ctr@$TESTSNAP +snapvol=$SNAPFS1 +snapctrvol=$ctrvol@$TESTSNAP +snapctrclone=$ctrclone@$TESTSNAP +snapctrfs=$SNAPCTR + +log_must zfs snapshot $ctrfs@$TESTSNAP1 +log_must zfs clone $ctrfs@$TESTSNAP1 $ctrclone +if is_global_zone; then + log_must zfs create -V $VOLSIZE $ctrvol +else + log_must zfs create $ctrvol +fi + +log_mustnot zfs snapshot -r $snappool + +log_pass "Snapshot -r fails as expected."