From 2f16803134ff1bb257108691beb6a7078e04b246 Mon Sep 17 00:00:00 2001 From: Umer Saleem Date: Thu, 25 Aug 2022 12:51:40 +0500 Subject: [PATCH] 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 Reviewed-by: Alexander Motin Reviewed-by: Brian Behlendorf Signed-off-by: Umer Saleem Closes #13635 --- include/sys/dsl_dir.h | 2 +- include/sys/fs/zfs.h | 1 + lib/libzfs/libzfs.abi | 3 +- lib/libzfs/libzfs_dataset.c | 20 +++ man/man7/zfsprops.7 | 7 + module/zcommon/zfs_prop.c | 5 + module/zfs/dsl_dataset.c | 6 +- module/zfs/dsl_dir.c | 17 ++- module/zfs/zcp_get.c | 4 + tests/runfiles/common.run | 2 +- tests/runfiles/sanity.run | 3 +- tests/zfs-tests/include/libtest.shlib | 14 ++ .../tests/functional/snapshot/Makefile.am | 3 +- .../functional/snapshot/snapshot_018_pos.ksh | 127 ++++++++++++++++++ 14 files changed, 204 insertions(+), 10 deletions(-) create mode 100755 tests/zfs-tests/tests/functional/snapshot/snapshot_018_pos.ksh diff --git a/include/sys/dsl_dir.h b/include/sys/dsl_dir.h index d635b3140423..ab1c02442fed 100644 --- a/include/sys/dsl_dir.h +++ b/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); diff --git a/include/sys/fs/zfs.h b/include/sys/fs/zfs.h index 448b30f02fd4..556831fc7993 100644 --- a/include/sys/fs/zfs.h +++ b/include/sys/fs/zfs.h @@ -186,6 +186,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; diff --git a/lib/libzfs/libzfs.abi b/lib/libzfs/libzfs.abi index 22b7bed1bc6b..1611072688a2 100644 --- a/lib/libzfs/libzfs.abi +++ b/lib/libzfs/libzfs.abi @@ -2000,7 +2000,8 @@ - + + diff --git a/lib/libzfs/libzfs_dataset.c b/lib/libzfs/libzfs_dataset.c index 4958a70754e1..b7b96f64a1f2 100644 --- a/lib/libzfs/libzfs_dataset.c +++ b/lib/libzfs/libzfs_dataset.c @@ -2942,6 +2942,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: diff --git a/man/man7/zfsprops.7 b/man/man7/zfsprops.7 index a68a507e230d..1b744cb3413a 100644 --- a/man/man7/zfsprops.7 +++ b/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 diff --git a/module/zcommon/zfs_prop.c b/module/zcommon/zfs_prop.c index 9c807dac38e6..2171b61122a4 100644 --- a/module/zcommon/zfs_prop.c +++ b/module/zcommon/zfs_prop.c @@ -708,6 +708,11 @@ zfs_prop_init(void) zprop_register_impl(ZFS_PROP_CREATION, "creation", PROP_TYPE_NUMBER, 0, NULL, PROP_READONLY, ZFS_TYPE_DATASET | ZFS_TYPE_BOOKMARK, "", "CREATION", B_FALSE, B_TRUE, NULL); + + zprop_register_impl(ZFS_PROP_SNAPSHOTS_CHANGED, "snapshots_changed", + PROP_TYPE_NUMBER, 0, NULL, PROP_READONLY, ZFS_TYPE_FILESYSTEM | + ZFS_TYPE_VOLUME, "", "SNAPSHOTS_CHANGED", B_FALSE, B_TRUE, + NULL); } boolean_t diff --git a/module/zfs/dsl_dataset.c b/module/zfs/dsl_dataset.c index e8692c95438a..b7894b9802cc 100644 --- a/module/zfs/dsl_dataset.c +++ b/module/zfs/dsl_dataset.c @@ -522,7 +522,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; @@ -1852,7 +1852,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, " "); } @@ -2810,6 +2810,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) { diff --git a/module/zfs/dsl_dir.c b/module/zfs/dsl_dir.c index aca32ff9bbb9..078668961639 100644 --- a/module/zfs/dsl_dir.c +++ b/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); } diff --git a/module/zfs/zcp_get.c b/module/zfs/zcp_get.c index 7256e4de1915..4cea5d14bff2 100644 --- a/module/zfs/zcp_get.c +++ b/module/zfs/zcp_get.c @@ -410,6 +410,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); diff --git a/tests/runfiles/common.run b/tests/runfiles/common.run index de001af9b3d7..301f286448c7 100644 --- a/tests/runfiles/common.run +++ b/tests/runfiles/common.run @@ -851,7 +851,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] diff --git a/tests/runfiles/sanity.run b/tests/runfiles/sanity.run index fb39fa54b9d5..3f9d82ddb18a 100644 --- a/tests/runfiles/sanity.run +++ b/tests/runfiles/sanity.run @@ -566,7 +566,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] diff --git a/tests/zfs-tests/include/libtest.shlib b/tests/zfs-tests/include/libtest.shlib index 992f14a81d7b..873e76b17dad 100644 --- a/tests/zfs-tests/include/libtest.shlib +++ b/tests/zfs-tests/include/libtest.shlib @@ -4047,6 +4047,20 @@ function stat_size # esac } +function stat_mtime # +{ + typeset path=$1 + + case "$UNAME" in + FreeBSD) + stat -f %m "$path" + ;; + *) + stat -c %Y "$path" + ;; + esac +} + function stat_ctime # { typeset path=$1 diff --git a/tests/zfs-tests/tests/functional/snapshot/Makefile.am b/tests/zfs-tests/tests/functional/snapshot/Makefile.am index 783133a643a1..e1db11cd7497 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_018_pos.ksh dist_pkgdata_DATA = \ snapshot.cfg diff --git a/tests/zfs-tests/tests/functional/snapshot/snapshot_018_pos.ksh b/tests/zfs-tests/tests/functional/snapshot/snapshot_018_pos.ksh new file mode 100755 index 000000000000..a206a52d3bd4 --- /dev/null +++ b/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"