All of lore.kernel.org
 help / color / mirror / Atom feed
From: Zorro Lang <zlang@kernel.org>
To: Filipe Manana <fdmanana@kernel.org>
Cc: fstests@vger.kernel.org, linux-btrfs@vger.kernel.org,
	 Filipe Manana <fdmanana@suse.com>
Subject: Re: [PATCH] btrfs: test an incremental send scenario with hole punching
Date: Sun, 15 Nov 2026 05:21:30 +0800	[thread overview]
Message-ID: <avjP-YcABrnO5RZ1@zlang-mailbox> (raw)
In-Reply-To: <CAL3q7H4KZ-xerun6qOCxQDd4GPPj=B6KLZwa16rNQEVUKjAYDA@mail.gmail.com>

On Sat, Nov 14, 2026 at 04:51:27PM +0000, Filipe Manana wrote:
> On Sat, Nov 14, 2026 at 3:05 PM Zorro Lang <zlang@kernel.org> wrote:
> >
> > On Wed, Oct 21, 2026 at 09:17:25PM +0100, fdmanana@kernel.org wrote:
> > > From: Filipe Manana <fdmanana@suse.com>
> > >
> > > Test an incremental send scenario where we punched a hole into a tail
> > > section of a file extent item that is the last item in a leaf and the
> > > next leaf in the subvolume tree was not changed (remains shared between
> > > the parent and send snapshots).
> > >
> > > This currently fails and is fixed by the following kernel patch:
> > >
> > >   btrfs: send: fix missing hole punching during incremental send
> > >
> > > Signed-off-by: Filipe Manana <fdmanana@suse.com>
> > > ---
> > >  tests/btrfs/362     | 84 +++++++++++++++++++++++++++++++++++++++++++++
> > >  tests/btrfs/362.out |  7 ++++
> > >  2 files changed, 91 insertions(+)
> > >  create mode 100755 tests/btrfs/362
> > >  create mode 100644 tests/btrfs/362.out
> > >
> > > diff --git a/tests/btrfs/362 b/tests/btrfs/362
> > > new file mode 100755
> > > index 00000000..aca56540
> > > --- /dev/null
> > > +++ b/tests/btrfs/362
> > > @@ -0,0 +1,84 @@
> > > +#! /bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +# Copyright (c) 2026 SUSE S.A.  All Rights Reserved.
> > > +#
> > > +# FS QA Test 362
> > > +#
> > > +# Test an incremental send scenario where we punched a hole into a tail section
> > > +# of a file extent item that is the last item in a leaf and the next leaf in the
> > > +# subvolume tree was not changed (remains shared between the parent and send
> > > +# snapshots).
> > > +#
> > > +. ./common/preamble
> > > +_begin_fstest auto quick send
> > > +
> > > +_cleanup()
> > > +{
> > > +     cd /
> > > +     rm -fr $tmp.*
> > > +     rm -fr $send_files_dir
> > > +}
> > > +
> > > +. ./common/filter
> > > +
> > > +_require_test
> > > +_require_scratch
> > > +_require_btrfs_support_sectorsize 4096
> > > +_require_xfs_io_command "fpunch"
> > > +
> > > +_fixed_by_kernel_commit xxxxxxxxxx \
> > > +     "btrfs: send: fix missing hole punching during incremental send"
> > > +
> > > +send_files_dir=$TEST_DIR/btrfs-test-$seq
> > > +
> > > +rm -fr $send_files_dir
> > > +mkdir $send_files_dir
> > > +
> > > +first_stream="$send_files_dir/1.send"
> > > +second_stream="$send_files_dir/2.send"
> > > +
> > > +# We need a very specific leaf layout where we punch a hole in the last file
> > > +# extent item of a leaf and the next leaf was not modified in the send snapshot,
> > > +# so we set a fixed size of 16K.
> > > +_scratch_mkfs -n 16K >> $seqres.full 2>&1 || _fail "first mkfs failed"
> > > +_scratch_mount
> > > +
> > > +touch $SCRATCH_MNT/foo
> > > +# 205 extents of 64K each with a 64K hole between them.
> > > +for ((i = 0; i < 410; i += 2)); do
> > > +     offset=$((i * 64 * 1024))
> > > +     $XFS_IO_PROG -c "pwrite -b 64K -S 0xab $offset 64K" \
> > > +             $SCRATCH_MNT/foo > /dev/null
> > > +done
> > > +
> > > +_btrfs subvolume snapshot -r $SCRATCH_MNT $SCRATCH_MNT/snap1
> > > +
> > > +# Punch a hole in the last 16K of the last file extent item of the first leaf,
> > > +# which starts at file offset 12845056, so 12845056 + 64K - 16K = 12894208.
> > > +# This does not cause any change in the second leaf of the fs tree, so it will
> > > +# be shared between the snapshots.
> > > +$XFS_IO_PROG -c "fpunch 12894208 16K" $SCRATCH_MNT/foo
> > > +
> > > +_btrfs subvolume snapshot -r $SCRATCH_MNT $SCRATCH_MNT/snap2
> > > +
> > > +echo "File digest in the original fs: $(_md5_checksum $SCRATCH_MNT/snap2/foo)"
> > > +
> > > +_btrfs send -f $first_stream $SCRATCH_MNT/snap1
> > > +_btrfs send -f $second_stream -p $SCRATCH_MNT/snap1 $SCRATCH_MNT/snap2
> > > +
> > > +# Create a new fs and apply both send streams.
> > > +_scratch_unmount
> > > +_scratch_mkfs >> $seqres.full 2>&1 || _fail "second mkfs failed"
> > > +_scratch_mount
> > > +
> > > +_btrfs receive -f $first_stream $SCRATCH_MNT
> > > +_btrfs receive -f $second_stream $SCRATCH_MNT
> > > +
> > > +echo "File digest in the new fs:      $(_md5_checksum $SCRATCH_MNT/snap2/foo)"
> > > +
> > > +# Check the region we punched a hole is full of zeroes.
> > > +echo "Hole data:"
> > > +od -A d -t x1 -j 12894208 -N $((16 * 1024)) $SCRATCH_MNT/snap2/foo
> >
> >
> > Can this be replaced with common helper: _hexdump ?
> 
> I don't have a strong objection to that, but honestly I prefer it as
> it is because it prints the addresses in decimal format, which is more
> human readable and directly matches the value passed to the hole
> punching operation above, making it easier to read.
> 
> Do you want to make the change yourself, or should I send a new
> version just for that?

If I were to change it, I'd have to merge the kernel patch above, rebuild and
install the kernel, run the tests, and then update the .out file.
Never mind, just using od works fine too, since the _hexdump helper was
originally introduced to replace the widespread use of the hexdump command
anyway. Anyway, no need to send a new version. If I find some time, I'll
make the change myself, or merging it directly :)

Thanks,
Zorro

> 
> >
> > Thanks,
> > Zorro
> >
> > > +
> > > +# success, all done
> > > +_exit 0
> > > diff --git a/tests/btrfs/362.out b/tests/btrfs/362.out
> > > new file mode 100644
> > > index 00000000..789abb53
> > > --- /dev/null
> > > +++ b/tests/btrfs/362.out
> > > @@ -0,0 +1,7 @@
> > > +QA output created by 362
> > > +File digest in the original fs: a5a06694d9f7fc3245afb2f391636e44
> > > +File digest in the new fs:      a5a06694d9f7fc3245afb2f391636e44
> > > +Hole data:
> > > +12894208 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > +*
> > > +12910592
> > > --
> > > 2.53.0
> > >
> > >
> 

      reply	other threads:[~2026-11-14 21:21 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-21 20:17 [PATCH] btrfs: test an incremental send scenario with hole punching fdmanana
2026-11-12 17:12 ` Johannes Thumshirn
2026-11-14 15:05 ` Zorro Lang
2026-11-14 16:51   ` Filipe Manana
2026-11-14 21:21     ` Zorro Lang [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=avjP-YcABrnO5RZ1@zlang-mailbox \
    --to=zlang@kernel.org \
    --cc=fdmanana@kernel.org \
    --cc=fdmanana@suse.com \
    --cc=fstests@vger.kernel.org \
    --cc=linux-btrfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.