Re: [PATCH 1/3] btrfs: exercise NOCOMPRESS range and EOF handling

[Qu Wenruo <wqu@suse.com>]

在 2026/10/12 06:10, Dhruva G 写道:
> Check that a bounded --nocomp request rewrites compressed extents in the
> selected range while distant untouched regions still point to their
> original compressed storage.
> Cover a partial final sector, zero length, a length extending past EOF,
> start at EOF, and the empty-file no-op. Preserve contents and sizes across
> the conversion and a remount.
> 
> Add shared FIEMAP and integrity helpers for the NOCOMPRESS tests. Require
> full mapping coverage and inspect every overlapping extent so holes and
> partly converted ranges cannot pass. Include a small ioctl helper to
> probe kernel support and check errno without btrfs-progs translation.
> 
> Signed-off-by: Dhruva Gole <goledhruva@gmail.com>
> ---
>   .gitignore                |   1 +
>   common/btrfs_nocomp       | 134 ++++++++++++++++++++++++++++++++++++++
>   src/Makefile              |   2 +-
>   src/btrfs_defrag_nocomp.c |  63 ++++++++++++++++++
>   tests/btrfs/355           |  80 +++++++++++++++++++++++
>   tests/btrfs/355.out       |   2 +
>   6 files changed, 281 insertions(+), 1 deletion(-)
>   create mode 100644 common/btrfs_nocomp
>   create mode 100644 src/btrfs_defrag_nocomp.c
>   create mode 100755 tests/btrfs/355
>   create mode 100644 tests/btrfs/355.out
> 
> diff --git a/.gitignore b/.gitignore
> index 0b6b9452..1bfa12ef 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -216,6 +216,7 @@ tags
>   /src/truncate
>   /src/t_btrfs_received_uuid_ioctl
>   /src/btrfs_ioctl
> +/src/btrfs_defrag_nocomp
>   
>   # Symlinked files
>   /tests/generic/035.out
> diff --git a/common/btrfs_nocomp b/common/btrfs_nocomp
> new file mode 100644
> index 00000000..7e2111e1
> --- /dev/null
> +++ b/common/btrfs_nocomp
> @@ -0,0 +1,134 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) Meta Platforms, Inc. and affiliates.
> +# Helpers for BTRFS_DEFRAG_RANGE_NOCOMPRESS tests.

I don't like a dedicated file just do all those.
Why not put them into common/btrfs?

> +
> +_nc_require()

And "nc" is too generic, _require_btrfs_defrag_nocompress() would make 
more sense.

All the namings here look just some temporary scaffolding, not for 
generic purposes.

> +{
> +	_require_scratch
> +	_require_test_program btrfs_defrag_nocomp
> +	_require_xfs_io_command fiemap ranged
> +	_require_btrfs_command filesystem defragment --nocomp
> +	_require_btrfs_no_nodatacow
> +	_require_btrfs_no_nodatasum
> +	_require_btrfs_no_autodefrag
> +	_require_btrfs_support_sectorsize 4096
> +	_wants_kernel_commit 009b2056cb25 \
> +		"btrfs: defrag: add flag to force no-compression"
> +}
> +
> +_nc_write()
> +{
> +	$XFS_IO_PROG -f -c "pwrite -S $4 -b 128K $2 $3" -c fsync "$1" \
> +		>> "$seqres.full" 2>&1 || _fail "write failed: $1"
> +}
> +
> +_nc_probe()
> +{
> +	local result
> +
> +	_nc_write "$SCRATCH_MNT/probe" 0 131072 0x5a
> +	result=$($here/src/btrfs_defrag_nocomp "$SCRATCH_MNT/probe" 0 131072) ||
> +		_fail "NOCOMPRESS probe failed"

Why we need such dedicated probe program while the kernel is already 
rejecting unknown flags with ENOTTY?

And I think this probing should also be done inside the 
_require_btrfs_defrag_nocompress().

> +	case "$result" in
> +	OK) ;;
> +	EOPNOTSUPP|ENOTTY) _notrun "kernel does not support NOCOMPRESS" ;;
> +	*) _fail "unexpected NOCOMPRESS probe result: $result" ;;
> +	esac
> +	rm -f "$SCRATCH_MNT/probe"
> +}
> +
> +_nc_defrag()
> +{
> +	local file="${@: -1}"
> +
> +	$BTRFS_UTIL_PROG filesystem defragment --nocomp "$@" \
> +		>> "$seqres.full" 2>&1 || _fail "NOCOMPRESS defrag failed"
> +	$XFS_IO_PROG -c fsync "$file" >> "$seqres.full" 2>&1 ||
> +		_fail "fsync after defrag failed"
> +}
> +
> +_nc_map()
> +{
> +	$XFS_IO_PROG -c "fiemap -v $2 $3" "$1" > "$tmp.nc-map" \
> +		2>> "$seqres.full" || _fail "fiemap failed: $1"
> +	cat "$tmp.nc-map" >> "$seqres.full"
> +}
> +
> +# FIEMAP prints logical offsets in 512-byte units. Check every overlapping
> +# extent and complete coverage, so a hole or a partly converted range fails.
> +# The caller supplies byte offsets and lengths, including a partial EOF sector.
> +_nc_check_range()

This looks a little overkilled to me.

For regular hole/compression flags, what about filtering out the 
non-hole logical bytenr (e.g. replace them with 'XXX' or 'regular'), 
then output to the stdout, and use golden output to verify?