Add snapshots_changed as property

Make dd_snap_cmtime property persistent across mount and unmount
operations by storing in ZAP and restore the value from ZAP on hold
into dd_snap_cmtime instead of updating it.

Expose dd_snap_cmtime as 'snapshots_changed' property that provides a
mechanism to quickly determine whether snapshot list for dataset has
changed without having to mount a dataset or iterate the snapshot list.

It specifies the time at which a snapshot for a dataset was last
created or deleted. This allows us to be more efficient how often we
query snapshots.

Reviewed-by: Ryan Moeller <ryan@iXsystems.com>
Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Umer Saleem <usaleem@ixsystems.com>
Closes #13635

include/sys/dsl_dir.h

@@ -191,7 +191,7 @@ int dsl_dir_transfer_possible(dsl_dir_t *sdd, dsl_dir_t *tdd,
 boolean_t dsl_dir_is_clone(dsl_dir_t *dd);
 void dsl_dir_new_refreservation(dsl_dir_t *dd, struct dsl_dataset *ds,
     uint64_t reservation, cred_t *cr, dmu_tx_t *tx);
-void dsl_dir_snap_cmtime_update(dsl_dir_t *dd);
+void dsl_dir_snap_cmtime_update(dsl_dir_t *dd, dmu_tx_t *tx);
 inode_timespec_t dsl_dir_snap_cmtime(dsl_dir_t *dd);
 void dsl_dir_set_reservation_sync_impl(dsl_dir_t *dd, uint64_t value,
     dmu_tx_t *tx);

include/sys/fs/zfs.h

@@ -189,6 +189,7 @@ typedef enum {
 	ZFS_PROP_IVSET_GUID,		/* not exposed to the user */
 	ZFS_PROP_REDACTED,
 	ZFS_PROP_REDACT_SNAPS,
+	ZFS_PROP_SNAPSHOTS_CHANGED,
 	ZFS_NUM_PROPS
 } zfs_prop_t;
 

lib/libzfs/libzfs.abi

@@ -2398,7 +2398,8 @@
       <enumerator name='ZFS_PROP_IVSET_GUID' value='92'/>
       <enumerator name='ZFS_PROP_REDACTED' value='93'/>
       <enumerator name='ZFS_PROP_REDACT_SNAPS' value='94'/>
-      <enumerator name='ZFS_NUM_PROPS' value='95'/>
+      <enumerator name='ZFS_PROP_SNAPSHOTS_CHANGED' value='95'/>
+      <enumerator name='ZFS_NUM_PROPS' value='96'/>
     </enum-decl>
     <typedef-decl name='zfs_prop_t' type-id='4b000d60' id='58603c44'/>
     <enum-decl name='zfs_userquota_prop_t' naming-typedef-id='279fde6a' id='5258d2f6'>

lib/libzfs/libzfs_dataset.c

@@ -2935,6 +2935,26 @@ zfs_prop_get(zfs_handle_t *zhp, zfs_prop_t prop, char *propbuf, size_t proplen,
 		zcp_check(zhp, prop, val, NULL);
 		break;
 
+	case ZFS_PROP_SNAPSHOTS_CHANGED:
+		{
+			if ((get_numeric_property(zhp, prop, src, &source,
+			    &val) != 0) || val == 0) {
+				return (-1);
+			}
+
+			time_t time = (time_t)val;
+			struct tm t;
+
+			if (literal ||
+			    localtime_r(&time, &t) == NULL ||
+			    strftime(propbuf, proplen, "%a %b %e %k:%M %Y",
+			    &t) == 0)
+				(void) snprintf(propbuf, proplen, "%llu",
+				    (u_longlong_t)val);
+		}
+		zcp_check(zhp, prop, val, NULL);
+		break;
+
 	default:
 		switch (zfs_prop_get_type(prop)) {
 		case PROP_TYPE_NUMBER:

man/man7/zfsprops.7

@@ -519,6 +519,13 @@ The root user, or a user who has been granted the
 privilege with
 .Nm zfs allow ,
 can access all projects' objects usage.
+.It Sy snapshots_changed
+Provides a mechanism to quickly determine whether snapshot list has
+changed without having to mount a dataset or iterate the snapshot list.
+Specifies the time at which a snapshot for a dataset was last
+created or deleted.
+.Pp
+This allows us to be more efficient how often we query snapshots.
 .It Sy volblocksize
 For volumes, specifies the block size of the volume.
 The

module/zcommon/zfs_prop.c

@@ -734,6 +734,11 @@ zfs_prop_init(void)
 	    NULL, PROP_READONLY, ZFS_TYPE_DATASET | ZFS_TYPE_BOOKMARK,
 	    "<date>", "CREATION", B_FALSE, B_TRUE, B_TRUE, NULL, sfeatures);
 
+	zprop_register_impl(ZFS_PROP_SNAPSHOTS_CHANGED, "snapshots_changed",
+	    PROP_TYPE_NUMBER, 0, NULL, PROP_READONLY, ZFS_TYPE_FILESYSTEM |
+	    ZFS_TYPE_VOLUME, "<date>", "SNAPSHOTS_CHANGED", B_FALSE, B_TRUE,
+	    B_TRUE, NULL, sfeatures);
+
 	zfs_mod_list_supported_free(sfeatures);
 }
 

module/zfs/dsl_dataset.c

@@ -534,7 +534,7 @@ dsl_dataset_snap_remove(dsl_dataset_t *ds, const char *name, dmu_tx_t *tx,
 	matchtype_t mt = 0;
 	int err;
 
-	dsl_dir_snap_cmtime_update(ds->ds_dir);
+	dsl_dir_snap_cmtime_update(ds->ds_dir, tx);
 
 	if (dsl_dataset_phys(ds)->ds_flags & DS_FLAG_CI_DATASET)
 		mt = MT_NORMALIZE;
@@ -1865,7 +1865,7 @@ dsl_dataset_snapshot_sync_impl(dsl_dataset_t *ds, const char *snapname,
 
 	dsl_scan_ds_snapshotted(ds, tx);
 
-	dsl_dir_snap_cmtime_update(ds->ds_dir);
+	dsl_dir_snap_cmtime_update(ds->ds_dir, tx);
 
 	spa_history_log_internal_ds(ds->ds_prev, "snapshot", tx, " ");
 }
@@ -2809,6 +2809,8 @@ dsl_dataset_stats(dsl_dataset_t *ds, nvlist_t *nv)
 	    dsl_get_userrefs(ds));
 	dsl_prop_nvlist_add_uint64(nv, ZFS_PROP_DEFER_DESTROY,
 	    dsl_get_defer_destroy(ds));
+	dsl_prop_nvlist_add_uint64(nv, ZFS_PROP_SNAPSHOTS_CHANGED,
+	    dsl_dir_snap_cmtime(ds->ds_dir).tv_sec);
 	dsl_dataset_crypt_stats(ds, nv);
 
 	if (dsl_dataset_phys(ds)->ds_prev_snap_obj != 0) {

module/zfs/dsl_dir.c

@@ -207,8 +207,6 @@ dsl_dir_hold_obj(dsl_pool_t *dp, uint64_t ddobj,
 			}
 		}
 
-		dsl_dir_snap_cmtime_update(dd);
-
 		if (dsl_dir_phys(dd)->dd_parent_obj) {
 			err = dsl_dir_hold_obj(dp,
 			    dsl_dir_phys(dd)->dd_parent_obj, NULL, dd,
@@ -270,6 +268,14 @@ dsl_dir_hold_obj(dsl_pool_t *dp, uint64_t ddobj,
 			}
 		}
 
+		inode_timespec_t t = {0};
+		zap_lookup(dd->dd_pool->dp_meta_objset,
+		    dsl_dir_phys(dd)->dd_props_zapobj,
+		    zfs_prop_to_name(ZFS_PROP_SNAPSHOTS_CHANGED),
+		    sizeof (uint64_t),
+		    sizeof (inode_timespec_t) / sizeof (uint64_t), &t);
+		dd->dd_snap_cmtime = t;
+
 		dmu_buf_init_user(&dd->dd_dbu, NULL, dsl_dir_evict_async,
 		    &dd->dd_dbuf);
 		winner = dmu_buf_set_user_ie(dbuf, &dd->dd_dbu);
@@ -2243,13 +2249,18 @@ dsl_dir_snap_cmtime(dsl_dir_t *dd)
 }
 
 void
-dsl_dir_snap_cmtime_update(dsl_dir_t *dd)
+dsl_dir_snap_cmtime_update(dsl_dir_t *dd, dmu_tx_t *tx)
 {
 	inode_timespec_t t;
+	objset_t *mos = dd->dd_pool->dp_meta_objset;
+	uint64_t zapobj = dsl_dir_phys(dd)->dd_props_zapobj;
+	const char *prop_name = zfs_prop_to_name(ZFS_PROP_SNAPSHOTS_CHANGED);
 
 	gethrestime(&t);
 	mutex_enter(&dd->dd_lock);
 	dd->dd_snap_cmtime = t;
+	VERIFY0(zap_update(mos, zapobj, prop_name, sizeof (uint64_t),
+	    sizeof (inode_timespec_t) / sizeof (uint64_t), &t, tx));
 	mutex_exit(&dd->dd_lock);
 }
 

module/zfs/zcp_get.c

@@ -403,6 +403,10 @@ get_special_prop(lua_State *state, dsl_dataset_t *ds, const char *dsname,
 		break;
 	}
 
+	case ZFS_PROP_SNAPSHOTS_CHANGED:
+		numval = dsl_dir_snap_cmtime(ds->ds_dir).tv_sec;
+		break;
+
 	default:
 		/* Did not match these props, check in the dsl_dir */
 		error = get_dsl_dir_prop(ds, zfs_prop, &numval);

tests/runfiles/common.run

@@ -869,7 +869,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_018_pos']
 tags = ['functional', 'snapshot']
 
 [tests/functional/snapused]

tests/runfiles/sanity.run

@@ -568,7 +568,8 @@ tests = ['clone_001_pos', 'rollback_001_pos', 'rollback_002_pos',
     'snapshot_004_pos', 'snapshot_005_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_013_pos', 'snapshot_014_pos', 'snapshot_017_pos',
+    'snapshot_018_pos']
 tags = ['functional', 'snapshot']
 
 [tests/functional/snapused]

tests/zfs-tests/include/libtest.shlib

@@ -3432,6 +3432,20 @@ function stat_size #<path>
 	esac
 }
 
+function stat_mtime #<path>
+{
+	typeset path=$1
+
+	case "$UNAME" in
+	FreeBSD)
+		stat -f %m "$path"
+		;;
+	*)
+		stat -c %Y "$path"
+		;;
+	esac
+}
+
 function stat_ctime #<path>
 {
 	typeset path=$1

tests/zfs-tests/tests/Makefile.am

@@ -1857,6 +1857,7 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \
 	functional/snapshot/snapshot_015_pos.ksh \
 	functional/snapshot/snapshot_016_pos.ksh \
 	functional/snapshot/snapshot_017_pos.ksh \
+	functional/snapshot/snapshot_018_pos.ksh \
 	functional/snapused/cleanup.ksh \
 	functional/snapused/setup.ksh \
 	functional/snapused/snapused_001_pos.ksh \

tests/zfs-tests/tests/functional/snapshot/snapshot_018_pos.ksh

@@ -0,0 +1,127 @@
+#! /bin/ksh -p
+#
+# CDDL HEADER START
+#
+# The contents of this file are subject to the terms of the
+# Common Development and Distribution License (the "License").
+# You may not use this file except in compliance with the License.
+#
+# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
+# or http://www.opensolaris.org/os/licensing.
+# See the License for the specific language governing permissions
+# and limitations under the License.
+#
+# When distributing Covered Code, include this CDDL HEADER in each
+# file and include the License file at usr/src/OPENSOLARIS.LICENSE.
+# If applicable, add the following below this CDDL HEADER, with the
+# fields enclosed by brackets "[]" replaced with your own identifying
+# information: Portions Copyright [yyyy] [name of copyright owner]
+#
+# CDDL HEADER END
+#
+
+#
+# Copyright 2022 iXsystems, Inc.
+#
+
+. $STF_SUITE/include/libtest.shlib
+. $STF_SUITE/tests/functional/snapshot/snapshot.cfg
+
+#
+# DESCRIPTION:
+# Verify the functionality of snapshots_changed property
+#
+# STRATEGY:
+# 1. Create a pool
+# 2. Verify snapshots_changed property is NULL
+# 3. Create a filesystem
+# 4. Verify snapshots_changed property is NULL
+# 5. Create snapshots for all filesystems
+# 6. Verify snapshots_changed property shows correct time
+# 7. Unmount all filesystems
+# 8. Create a snapshot while unmounted
+# 9. Verify snapshots_changed
+# 10. Mount the filsystems
+# 11. Verify snapshots_changed
+# 12. Destroy the snapshots
+# 13. Verify snapshots_changed
+#
+
+function cleanup
+{
+	create_pool $TESTPOOL $DISKS
+}
+
+verify_runnable "both"
+
+log_assert "Verify snapshots_changed property"
+
+log_onexit cleanup
+
+snap_testpool="$TESTPOOL@v1"
+snap_testfsv1="$TESTPOOL/$TESTFS@v1"
+snap_testfsv2="$TESTPOOL/$TESTFS@v2"
+snapdir=".zfs/snapshot"
+
+# Create filesystems and check snapshots_changed is NULL
+create_pool $TESTPOOL $DISKS
+snap_changed_testpool=$(zfs get -H -o value -p snapshots_changed $TESTPOOL)
+log_must eval "[[ $snap_changed_testpool == - ]]"
+tpool_snapdir=$(get_prop mountpoint $TESTPOOL)/$snapdir
+log_must eval "[[ $(stat_mtime $tpool_snapdir) == 0 ]]"
+
+log_must zfs create $TESTPOOL/$TESTFS
+snap_changed_testfs=$(zfs get -H -o value -p snapshots_changed $TESTPOOL/$TESTFS)
+log_must eval "[[ $snap_changed_testfs == - ]]"
+tfs_snapdir=$(get_prop mountpoint $TESTPOOL/$TESTFS)/$snapdir
+log_must eval "[[ $(stat_mtime $tfs_snapdir) == 0 ]]"
+
+# Create snapshots for filesystems and check snapshots_changed reports correct time
+curr_time=$(date '+%s')
+log_must zfs snapshot $snap_testpool
+snap_changed_testpool=$(zfs get -H -o value -p snapshots_changed $TESTPOOL)
+log_must eval "[[ $snap_changed_testpool -ge $curr_time ]]"
+log_must eval "[[ $(stat_mtime $tpool_snapdir) ==  $snap_changed_testpool ]]"
+
+curr_time=$(date '+%s')
+log_must zfs snapshot $snap_testfsv1
+snap_changed_testfs=$(zfs get -H -o value -p snapshots_changed $TESTPOOL/$TESTFS)
+log_must eval "[[ $snap_changed_testfs -ge $curr_time ]]"
+log_must eval "[[ $(stat_mtime $tfs_snapdir) ==  $snap_changed_testfs ]]"
+
+# Unmount the filesystems and check snapshots_changed has correct value after unmount
+log_must zfs unmount $TESTPOOL/$TESTFS
+log_must eval "[[ $(zfs get -H -o value -p snapshots_changed $TESTPOOL/$TESTFS) == $snap_changed_testfs ]]"
+
+# Create snapshot while unmounted
+curr_time=$(date '+%s')
+log_must zfs snapshot $snap_testfsv2
+snap_changed_testfs=$(zfs get -H -o value -p snapshots_changed $TESTPOOL/$TESTFS)
+log_must eval "[[ $snap_changed_testfs -ge $curr_time ]]"
+
+log_must zfs unmount $TESTPOOL
+log_must eval "[[ $(zfs get -H -o value -p snapshots_changed $TESTPOOL) == $snap_changed_testpool ]]"
+
+# Mount back the filesystems and check snapshots_changed still has correct value
+log_must zfs mount $TESTPOOL
+log_must eval "[[ $(zfs get -H -o value -p snapshots_changed $TESTPOOL) == $snap_changed_testpool ]]"
+log_must eval "[[ $(stat_mtime $tpool_snapdir) ==  $snap_changed_testpool ]]"
+
+log_must zfs mount $TESTPOOL/$TESTFS
+log_must eval "[[ $(zfs get -H -o value -p snapshots_changed $TESTPOOL/$TESTFS) == $snap_changed_testfs ]]"
+log_must eval "[[ $(stat_mtime $tfs_snapdir) ==  $snap_changed_testfs ]]"
+
+# Destroy the snapshots and check snapshots_changed shows correct time
+curr_time=$(date '+%s')
+log_must zfs destroy $snap_testfsv1
+snap_changed_testfs=$(zfs get -H -o value -p snapshots_changed $TESTPOOL/$TESTFS)
+log_must eval "[[ $snap_changed_testfs -ge $curr_time ]]"
+log_must eval "[[ $(stat_mtime $tfs_snapdir) ==  $snap_changed_testfs ]]"
+
+curr_time=$(date '+%s')
+log_must zfs destroy $snap_testpool
+snap_changed_testpool=$(zfs get -H -o value -p snapshots_changed $TESTPOOL)
+log_must eval "[[ $snap_changed_testpool -ge $curr_time ]]"
+log_must eval "[[ $(stat_mtime $tpool_snapdir) ==  $snap_changed_testpool ]]"
+
+log_pass "snapshots_changed property behaves correctly"