[PATCH v3] ceph/006: test snapshot data integrity after punch hole operations

[PATCH v3] ceph/006: test snapshot data integrity after punch hole operations

[ethanwu]

Add test to verify that Ceph snapshots preserve original file data
when the live file is modified with punch hole operations. The test
creates a file, takes a snapshot, punches multiple holes in the
original file, then verifies the snapshot data remains unchanged.

Signed-off-by: ethanwu <ethanwu@synology.com>
---
 v1->v2: previous version is 'ceph/006: test snapshot data integrity
 after punch hole'.
    1. move it to generic and add _require_snapshot check.
    2. modify punch hole offset/len to be 64K aligned
 v2->v3:
    1. move test back to ceph specific since it uses ceph snapshot API.
    2. support custom snapdirname mount option.
    3. add _ceph_remove_snapshot and _ceph_create_snapshot functions.

---
 common/ceph        | 76 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/ceph/006     | 58 +++++++++++++++++++++++++++++++++++
 tests/ceph/006.out |  2 ++
 3 files changed, 136 insertions(+)
 create mode 100755 tests/ceph/006
 create mode 100644 tests/ceph/006.out

diff --git a/common/ceph b/common/ceph
index df7a6814..6a19527e 100644
--- a/common/ceph
+++ b/common/ceph
@@ -38,3 +38,79 @@ _ceph_get_client_id()
 {
 	$GETFATTR_PROG --only-values -n "ceph.client_id" $TEST_DIR 2>/dev/null
 }
+
+# Get the snapshot directory name from mount options
+# Defaults to ".snap" if snapdirname option is not set
+_ceph_get_snapdirname()
+{
+	local mnt_point=$1
+	local snapdirname
+
+	# Extract snapdirname from mount options
+	snapdirname=$(findmnt -n -o OPTIONS "$mnt_point" | grep -o 'snapdirname=[^,]*' | cut -d'=' -f2)
+
+	# Default to .snap if not set
+	if [ -z "$snapdirname" ]; then
+		echo ".snap"
+	else
+		echo "$snapdirname"
+	fi
+}
+
+# Create a CephFS snapshot
+# _ceph_create_snapshot <directory_path> <snapshot_name>
+# Creates a snapshot under <directory_path>/.snap/<snapshot_name>
+# or <directory_path>/<custom_snapdir>/<snapshot_name> if snapdirname is set
+_ceph_create_snapshot()
+{
+	local dir_path=$1
+	local snap_name=$2
+	local snapdirname
+	local snapdir
+	local mnt_point
+
+	if [ -z "$dir_path" ] || [ -z "$snap_name" ]; then
+		echo "Usage: _ceph_create_snapshot <directory_path> <snapshot_name>"
+		return 1
+	fi
+
+	# Find the mount point for this directory
+	mnt_point=$(df -P "$dir_path" | tail -1 | awk '{print $NF}')
+	snapdirname=$(_ceph_get_snapdirname "$mnt_point")
+	snapdir="$dir_path/$snapdirname/$snap_name"
+
+	mkdir "$snapdir" || return 1
+	echo "$snapdir"
+}
+
+# Remove a CephFS snapshot
+# _ceph_remove_snapshot <directory_path> <snapshot_name>
+_ceph_remove_snapshot()
+{
+	local dir_path=$1
+	local snap_name=$2
+	local snapdirname
+	local snapdir
+	local mnt_point
+
+	if [ -z "$dir_path" ] || [ -z "$snap_name" ]; then
+		echo "Usage: _ceph_remove_snapshot <directory_path> <snapshot_name>"
+		return 1
+	fi
+
+	# Find the mount point for this directory
+	mnt_point=$(df -P "$dir_path" | tail -1 | awk '{print $NF}')
+	snapdirname=$(_ceph_get_snapdirname "$mnt_point")
+	snapdir="$dir_path/$snapdirname/$snap_name"
+
+	rmdir "$snapdir" 2>/dev/null
+}
+
+# this test requires ceph snapshot support
+_require_ceph_snapshot()
+{
+	local snapdirname=$(_ceph_get_snapdirname "$TEST_DIR")
+	local test_snapdir="$TEST_DIR/$snapdirname/test_snap_$$"
+	mkdir "$test_snapdir" 2>/dev/null || _notrun "Ceph snapshots not supported (requires fs flag 'allow_snaps' and client auth capability 'snap')"
+	rmdir "$test_snapdir"
+}
diff --git a/tests/ceph/006 b/tests/ceph/006
new file mode 100755
index 00000000..a5af6ca3
--- /dev/null
+++ b/tests/ceph/006
@@ -0,0 +1,58 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2025 Synology All Rights Reserved.
+#
+# FS QA Test No. ceph/006
+#
+# Test that snapshot data remains intact after punch hole operations
+# on the original file.
+#
+. ./common/preamble
+_begin_fstest auto quick snapshot
+
+. common/ceph
+
+_require_test
+_require_xfs_io_command "pwrite"
+_require_xfs_io_command "fpunch"
+_require_ceph_snapshot
+_exclude_test_mount_option "test_dummy_encryption"
+
+# TODO: commit is not merged yet. Update with final commit SHA once merged.
+_fixed_by_kernel_commit 1b7b474b3a78 \
+	"ceph: fix snapshot context missing in ceph_zero_partial_object"
+
+workdir=$TEST_DIR/test-$seq
+_ceph_remove_snapshot $workdir snap1
+rm -rf $workdir
+mkdir $workdir
+
+$XFS_IO_PROG -f -c "pwrite -S 0xab 0 1048576" $workdir/foo > /dev/null
+
+snapdir=$(_ceph_create_snapshot $workdir snap1)
+
+original_md5=$(md5sum $snapdir/foo | cut -d' ' -f1)
+
+$XFS_IO_PROG -c "fpunch 0 65536" $workdir/foo
+$XFS_IO_PROG -c "fpunch 131072 65536" $workdir/foo
+$XFS_IO_PROG -c "fpunch 262144 65536" $workdir/foo
+$XFS_IO_PROG -c "fpunch 393216 65536" $workdir/foo
+
+# Make sure we don't read from cache
+echo 3 > /proc/sys/vm/drop_caches
+
+snapshot_md5=$(md5sum $snapdir/foo | cut -d' ' -f1)
+
+if [ "$original_md5" != "$snapshot_md5" ]; then
+    echo "FAIL: Snapshot data changed after punch hole operations"
+    echo "Original md5sum: $original_md5"
+    echo "Snapshot md5sum: $snapshot_md5"
+fi
+
+_ceph_remove_snapshot $workdir snap1
+
+echo "Silence is golden"
+
+# success, all done
+status=0
+exit
diff --git a/tests/ceph/006.out b/tests/ceph/006.out
new file mode 100644
index 00000000..675c1b7c
--- /dev/null
+++ b/tests/ceph/006.out
@@ -0,0 +1,2 @@
+QA output created by 006
+Silence is golden
-- 
2.43.0


Disclaimer: The contents of this e-mail message and any attachments are confidential and are intended solely for addressee. The information may also be legally privileged. This transmission is sent in trust, for the sole purpose of delivery to the intended recipient. If you have received this transmission in error, any use, reproduction or dissemination of this transmission is strictly prohibited. If you are not the intended recipient, please immediately notify the sender by reply e-mail or phone and delete this message and its attachments, if any.

Re: [PATCH v3] ceph/006: test snapshot data integrity after punch hole operations

[Zorro Lang]

On Thu, Oct 09, 2025 at 05:31:29PM +0800, ethanwu wrote:
> Add test to verify that Ceph snapshots preserve original file data
> when the live file is modified with punch hole operations. The test
> creates a file, takes a snapshot, punches multiple holes in the
> original file, then verifies the snapshot data remains unchanged.
> 
> Signed-off-by: ethanwu <ethanwu@synology.com>
> ---
>  v1->v2: previous version is 'ceph/006: test snapshot data integrity
>  after punch hole'.
>     1. move it to generic and add _require_snapshot check.
>     2. modify punch hole offset/len to be 64K aligned
>  v2->v3:
>     1. move test back to ceph specific since it uses ceph snapshot API.
>     2. support custom snapdirname mount option.
>     3. add _ceph_remove_snapshot and _ceph_create_snapshot functions.
> 
> ---

Thanks for your update :) I'm not familiar with ceph, so I'll try to review
this patch from fstests side. Welcome ceph list provide more review points :)

>  common/ceph        | 76 ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/ceph/006     | 58 +++++++++++++++++++++++++++++++++++
>  tests/ceph/006.out |  2 ++
>  3 files changed, 136 insertions(+)
>  create mode 100755 tests/ceph/006
>  create mode 100644 tests/ceph/006.out
> 
> diff --git a/common/ceph b/common/ceph
> index df7a6814..6a19527e 100644
> --- a/common/ceph
> +++ b/common/ceph
> @@ -38,3 +38,79 @@ _ceph_get_client_id()
>  {
>  	$GETFATTR_PROG --only-values -n "ceph.client_id" $TEST_DIR 2>/dev/null
>  }
> +
> +# Get the snapshot directory name from mount options
> +# Defaults to ".snap" if snapdirname option is not set
> +_ceph_get_snapdirname()
> +{
> +	local mnt_point=$1
> +	local snapdirname
> +
> +	# Extract snapdirname from mount options
> +	snapdirname=$(findmnt -n -o OPTIONS "$mnt_point" | grep -o 'snapdirname=[^,]*' | cut -d'=' -f2)
> +
> +	# Default to .snap if not set
> +	if [ -z "$snapdirname" ]; then
> +		echo ".snap"
> +	else
> +		echo "$snapdirname"
> +	fi
> +}
> +
> +# Create a CephFS snapshot
> +# _ceph_create_snapshot <directory_path> <snapshot_name>
> +# Creates a snapshot under <directory_path>/.snap/<snapshot_name>
> +# or <directory_path>/<custom_snapdir>/<snapshot_name> if snapdirname is set
> +_ceph_create_snapshot()
> +{
> +	local dir_path=$1
> +	local snap_name=$2
> +	local snapdirname
> +	local snapdir
> +	local mnt_point
> +
> +	if [ -z "$dir_path" ] || [ -z "$snap_name" ]; then
> +		echo "Usage: _ceph_create_snapshot <directory_path> <snapshot_name>"
> +		return 1
> +	fi
> +
> +	# Find the mount point for this directory
> +	mnt_point=$(df -P "$dir_path" | tail -1 | awk '{print $NF}')
> +	snapdirname=$(_ceph_get_snapdirname "$mnt_point")
> +	snapdir="$dir_path/$snapdirname/$snap_name"
> +
> +	mkdir "$snapdir" || return 1
> +	echo "$snapdir"
> +}
> +
> +# Remove a CephFS snapshot
> +# _ceph_remove_snapshot <directory_path> <snapshot_name>
> +_ceph_remove_snapshot()
> +{
> +	local dir_path=$1
> +	local snap_name=$2
> +	local snapdirname
> +	local snapdir
> +	local mnt_point
> +
> +	if [ -z "$dir_path" ] || [ -z "$snap_name" ]; then
> +		echo "Usage: _ceph_remove_snapshot <directory_path> <snapshot_name>"
> +		return 1
> +	fi
> +
> +	# Find the mount point for this directory
> +	mnt_point=$(df -P "$dir_path" | tail -1 | awk '{print $NF}')
> +	snapdirname=$(_ceph_get_snapdirname "$mnt_point")
> +	snapdir="$dir_path/$snapdirname/$snap_name"
> +
> +	rmdir "$snapdir" 2>/dev/null

What if the snapdir can't be removed? Is that possible?

> +}
> +
> +# this test requires ceph snapshot support
> +_require_ceph_snapshot()
> +{
> +	local snapdirname=$(_ceph_get_snapdirname "$TEST_DIR")
> +	local test_snapdir="$TEST_DIR/$snapdirname/test_snap_$$"
> +	mkdir "$test_snapdir" 2>/dev/null || _notrun "Ceph snapshots not supported (requires fs flag 'allow_snaps' and client auth capability 'snap')"

Is "$snapdirname" created before calling this function? Do we need "-p" option?

> +	rmdir "$test_snapdir"
> +}
> diff --git a/tests/ceph/006 b/tests/ceph/006
> new file mode 100755
> index 00000000..a5af6ca3
> --- /dev/null
> +++ b/tests/ceph/006
> @@ -0,0 +1,58 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2025 Synology All Rights Reserved.
> +#
> +# FS QA Test No. ceph/006
> +#
> +# Test that snapshot data remains intact after punch hole operations
> +# on the original file.
> +#
> +. ./common/preamble
> +_begin_fstest auto quick snapshot
> +
> +. common/ceph
> +
> +_require_test
> +_require_xfs_io_command "pwrite"

I think "pwrite" is nearly always supported, don't need to check it :)

> +_require_xfs_io_command "fpunch"
> +_require_ceph_snapshot
> +_exclude_test_mount_option "test_dummy_encryption"
> +
> +# TODO: commit is not merged yet. Update with final commit SHA once merged.
> +_fixed_by_kernel_commit 1b7b474b3a78 \
> +	"ceph: fix snapshot context missing in ceph_zero_partial_object"
> +
> +workdir=$TEST_DIR/test-$seq
> +_ceph_remove_snapshot $workdir snap1
> +rm -rf $workdir
> +mkdir $workdir
> +
> +$XFS_IO_PROG -f -c "pwrite -S 0xab 0 1048576" $workdir/foo > /dev/null
> +
> +snapdir=$(_ceph_create_snapshot $workdir snap1)

As you just "return 1" if the _ceph_create_snapshot fails, what if the
snapshot isn't created? So how about calling "_fail" if it fails.

> +
> +original_md5=$(md5sum $snapdir/foo | cut -d' ' -f1)
> +
> +$XFS_IO_PROG -c "fpunch 0 65536" $workdir/foo
> +$XFS_IO_PROG -c "fpunch 131072 65536" $workdir/foo
> +$XFS_IO_PROG -c "fpunch 262144 65536" $workdir/foo
> +$XFS_IO_PROG -c "fpunch 393216 65536" $workdir/foo
> +
> +# Make sure we don't read from cache
> +echo 3 > /proc/sys/vm/drop_caches
> +
> +snapshot_md5=$(md5sum $snapdir/foo | cut -d' ' -f1)
> +
> +if [ "$original_md5" != "$snapshot_md5" ]; then
> +    echo "FAIL: Snapshot data changed after punch hole operations"
> +    echo "Original md5sum: $original_md5"
> +    echo "Snapshot md5sum: $snapshot_md5"
> +fi
> +
> +_ceph_remove_snapshot $workdir snap1

Better to make sure the snapshot is removed in _cleanup.

Thanks,
Zorro

> +
> +echo "Silence is golden"
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/ceph/006.out b/tests/ceph/006.out
> new file mode 100644
> index 00000000..675c1b7c
> --- /dev/null
> +++ b/tests/ceph/006.out
> @@ -0,0 +1,2 @@
> +QA output created by 006
> +Silence is golden
> -- 
> 2.43.0
> 
> 
> Disclaimer: The contents of this e-mail message and any attachments are confidential and are intended solely for addressee. The information may also be legally privileged. This transmission is sent in trust, for the sole purpose of delivery to the intended recipient. If you have received this transmission in error, any use, reproduction or dissemination of this transmission is strictly prohibited. If you are not the intended recipient, please immediately notify the sender by reply e-mail or phone and delete this message and its attachments, if any.
> 

Re: [PATCH v3] ceph/006: test snapshot data integrity after punch hole operations

[Markus Elfring]

…
> Disclaimer: The contents of this e-mail message and any attachments are confidential and are intended solely for addressee.

See also once more:
https://subspace.kernel.org/etiquette.html#do-not-include-confidentiality-disclaimers


…
> If you have received this transmission in error, any use, reproduction or dissemination of this transmission is strictly prohibited. If you are not the intended recipient, please immediately notify the sender by reply e-mail or phone and delete this message and its attachments, if any.

Do you expect this action to happen more often with your information?

Regards,
Markus