Re: [PATCH] fstests: btrfs: test incremental send for orphan inodes

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]



Filipe Manana <fdmanana@xxxxxxxxxx> 於 2022年10月17日 週一 晚上7:19寫道:
>
> On Fri, Oct 14, 2022 at 5:46 PM bingjingc <bingjingc@xxxxxxxxxxxx> wrote:
> >
> > From: BingJing Chang <bingjingc@xxxxxxxxxxxx>
> >
> > Test that an incremental send operation can handle orphan files or
> > directories in or not in the parent snapshot and the send snapshot.
> >
> > This issue is fixed by a kernel patch with the commit 9ed0a72e5b355d
> > ("btrfs: send: fix failures when processing inodes with no links")
> >
> > Signed-off-by: BingJing Chang <bingjingc@xxxxxxxxxxxx>
> > ---
> >  tests/btrfs/278     | 218 ++++++++++++++++++++++++++++++++++++++++++++
> >  tests/btrfs/278.out |   3 +
> >  2 files changed, 221 insertions(+)
> >  create mode 100755 tests/btrfs/278
> >  create mode 100644 tests/btrfs/278.out
> >
> > diff --git a/tests/btrfs/278 b/tests/btrfs/278
> > new file mode 100755
> > index 00000000..82da29e0
> > --- /dev/null
> > +++ b/tests/btrfs/278
> > @@ -0,0 +1,218 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2022 BingJing Chang.
> > +#
> > +# FS QA Test No. btrfs/278
> > +#
> > +# Regression test for btrfs incremental send issue when processing inodes
> > +# with no links
> > +#
> > +# This issue is fixed by the following linux kernel btrfs patch:
> > +#
> > +#   commit 9ed0a72e5b355d ("btrfs: send: fix failures when processing
> > +#   inodes with no links")
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto quick send
> > +
> > +# real QA test starts here
> > +_supported_fs btrfs
> > +_fixed_by_kernel_commit 9ed0a72e5b355d \
> > +       "btrfs: send: fix failures when processing inodes with no links"
> > +_require_test
> > +_require_scratch
> > +_require_btrfs_command "property"
> > +_require_fssum
> > +
> > +send_files_dir=$TEST_DIR/btrfs-test-$seq
> > +
> > +rm -fr $send_files_dir
> > +mkdir $send_files_dir
> > +
> > +_scratch_mkfs >>$seqres.full 2>&1
> > +_scratch_mount
> > +_run_btrfs_util_prog subvolume create $SCRATCH_MNT/vol
> > +
> > +# Creating the first snapshot looks like:
> > +#
> > +# .                                                                  (ino 256)
> > +# |--- deleted.file                                                  (ino 257)
> > +# |--- deleted.dir/                                                  (ino 258)
> > +# |--- changed_subcase1.file                                         (ino 259)
> > +# |--- changed_subcase2.file                                         (ino 260)
> > +# |--- changed_subcase1.dir/                                         (ino 261)
> > +# |    |---- foo                                                     (ino 262)
> > +# |--- changed_subcase2.dir/                                         (ino 263)
> > +# |    |---- foo                                                     (ino 264)
> > +#
> > +touch $SCRATCH_MNT/vol/deleted.file
> > +mkdir $SCRATCH_MNT/vol/deleted.dir
> > +touch $SCRATCH_MNT/vol/changed_subcase1.file
> > +touch $SCRATCH_MNT/vol/changed_subcase2.file
> > +mkdir $SCRATCH_MNT/vol/changed_subcase1.dir
> > +touch $SCRATCH_MNT/vol/changed_subcase1.dir/foo
> > +mkdir $SCRATCH_MNT/vol/changed_subcase2.dir
> > +touch $SCRATCH_MNT/vol/changed_subcase2.dir/foo
> > +_run_btrfs_util_prog subvolume snapshot -r $SCRATCH_MNT/vol $SCRATCH_MNT/snap1
> > +
> > +# Delete the deleted.*, create a new file and a new directory, and then
> > +# take the second snapshot looks like:
> > +#
> > +# .                                                                  (ino 256)
> > +# |--- changed_subcase1.file                                         (ino 259)
> > +# |--- changed_subcase2.file                                         (ino 260)
> > +# |--- changed_subcase1.dir/                                         (ino 261)
> > +# |    |---- foo                                                     (ino 262)
> > +# |--- changed_subcase2.dir/                                         (ino 263)
> > +# |    |---- foo                                                     (ino 264)
> > +# |--- new.file                                                      (ino 265)
> > +# |--- new.dir/                                                      (ino 266)
> > +#
> > +unlink $SCRATCH_MNT/vol/deleted.file
> > +rmdir $SCRATCH_MNT/vol/deleted.dir
> > +touch $SCRATCH_MNT/vol/new.file
> > +mkdir $SCRATCH_MNT/vol/new.dir
> > +_run_btrfs_util_prog subvolume snapshot -r $SCRATCH_MNT/vol $SCRATCH_MNT/snap2
> > +
> > +# Set the subvolume back to read-write mode to make orphans of the first
> > +# snapshot to look like:
>
> This confused me. You mention setting the subvolume to RW, but what
> happens is we are
> setting the snapshot "snap1" to RW. So, something like:
>
> "Set the snapshot "snap1" to read-write mode and turn several inodes
> to orphans, so
> that the snapshot will look like this:"
>
> > +#
> > +# .                                                                  (ino 256)
> > +# |--- (orphan) deleted.file                                         (ino 257)
> > +# |--- (orphan) deleted.dir/                                         (ino 258)
> > +# |--- (orphan) changed_subcase1.file                                (ino 259)
> > +# |--- changed_subcase2.file                                         (ino 260)
> > +# |--- (orphan) changed_subcase1.dir/                                (ino 261)
> > +# |--- changed_subcase2.dir/                                         (ino 263)
> > +# |    |---- foo                                                     (ino 264)
> > +#
> > +# Note: To make an easy illustration, I just put a tag "(orphan)" in front of
> > +# their original names to indicate that they're deleted, but their inodes can
> > +# not be removed because of open file descriptors on them. Mention that orphan
> > +# inodes don't have names(paths).
> > +#
> > +$BTRFS_UTIL_PROG property set $SCRATCH_MNT/snap1 ro false
> > +exec 71<$SCRATCH_MNT/snap1/deleted.file
> > +exec 72<$SCRATCH_MNT/snap1/deleted.dir
> > +exec 73<$SCRATCH_MNT/snap1/changed_subcase1.file
> > +exec 74<$SCRATCH_MNT/snap1/changed_subcase1.dir
> > +unlink $SCRATCH_MNT/snap1/deleted.file
> > +rmdir $SCRATCH_MNT/snap1/deleted.dir
> > +unlink $SCRATCH_MNT/snap1/changed_subcase1.file
> > +unlink $SCRATCH_MNT/snap1/changed_subcase1.dir/foo
> > +rmdir $SCRATCH_MNT/snap1/changed_subcase1.dir
> > +
> > +# Turn the subvolume back to read-only mode as snapshots
>
> Same here, and the sentence also seems unfinished. So:
>
> "Turn the snapshot "snap1" back to read-only mode."
>
> > +$BTRFS_UTIL_PROG property set $SCRATCH_MNT/snap1 ro true
> > +
> > +# Set the subvolume back to read-write mode to make orphans of the second
> > +# snapshot to look like:
>
> Same here, it's the snapshot "snap2" and not the subvolume.
>
> "Set the snapshot "snap2" to read-write mode and turn several inodes
> to orphans, so
> that the snapshot will look like this:"
>
> > +#
> > +# .                                                                  (ino 256)
> > +# |--- (orphan) changed_subcase1.file                                (ino 259)
> > +# |--- (orphan) changed_subcase2.file                                (ino 260)
> > +# |--- (orphan) changed_subcase1.dir/                                (ino 261)
> > +# |--- (orphan) changed_subcase2.dir/                                (ino 263)
> > +# |--- (orphan) new.file                                             (ino 265)
> > +# |--- (orphan) new.dir/                                             (ino 266)
> > +#
> > +# Note: Same notice as above. Mention that orphan inodes don't have
> > +# names(paths).
> > +#
> > +$BTRFS_UTIL_PROG property set $SCRATCH_MNT/snap2 ro false
> > +exec 81<$SCRATCH_MNT/snap2/changed_subcase1.file
> > +exec 82<$SCRATCH_MNT/snap2/changed_subcase1.dir
> > +exec 83<$SCRATCH_MNT/snap2/changed_subcase2.file
> > +exec 84<$SCRATCH_MNT/snap2/changed_subcase2.dir
> > +exec 85<$SCRATCH_MNT/snap2/new.file
> > +exec 86<$SCRATCH_MNT/snap2/new.dir
> > +unlink $SCRATCH_MNT/snap2/changed_subcase1.file
> > +unlink $SCRATCH_MNT/snap2/changed_subcase1.dir/foo
> > +rmdir $SCRATCH_MNT/snap2/changed_subcase1.dir
> > +unlink $SCRATCH_MNT/snap2/changed_subcase2.file
> > +unlink $SCRATCH_MNT/snap2/changed_subcase2.dir/foo
> > +rmdir $SCRATCH_MNT/snap2/changed_subcase2.dir
> > +unlink $SCRATCH_MNT/snap2/new.file
> > +rmdir $SCRATCH_MNT/snap2/new.dir
> > +
> > +# Turn the subvolume back to read-only mode as snapshots
>
> Same here, it's snapshot "snap2" and not the subvolume.
> The sentence also seems unfinished.
>
> "Turn the snapshot "snap2" back to read-only mode."
>
> > +$BTRFS_UTIL_PROG property set $SCRATCH_MNT/snap2 ro true
> > +
> > +# Test that a full send operation can handle orphans with no paths
> > +_run_btrfs_util_prog send -f $send_files_dir/1.snap $SCRATCH_MNT/snap1
> > +
> > +# Test that an incremental send operation can handle orphans.
> > +#
> > +# Here're descriptions for the details:
> > +#
> > +# Case 1: new.file and new.dir (BTRFS_COMPARE_TREE_NEW)
> > +#        |  send snapshot  | action
> > +#  --------------------------------
> > +#  nlink |        0        | ignore
> > +#
> > +# They're new inodes in the send snapshots, while they don't have paths.
>
> snapshots -> snapshot
>
> "They are new inodes in the send snapshot ("snap2"), but they don't
> have paths because they have no links".
>
> > +# Test that the send operation can ignore them in order not to generate
> > +# the creation commands for them. Or it will fail in retrieving their
> > +# current paths.
>
> Mentioning the error would be useful:
>
>  "... Or it will fail, with -ENOENT, when trying to generate paths for them."
>
> Other than those minor things, it looks good to me.
> Test passes on a patched kernel and fails on an unpatched kernel.
>
> Thanks so much for doing this!
>
> Reviewed-by: Filipe Manana <fdmanana@xxxxxxxx>
>

Thank you for the review. I revised the confusing sentences in patch v2.

>
> > +#
> > +#
> > +# Case 2: deleted.file and deleted.dir (BTRFS_COMPARE_TREE_DELETED)
> > +#       | parent snapshot | action
> > +# ----------------------------------
> > +# nlink |        0        | as usual
> > +#
> > +# They're deleted in the parent snapshots but become orphans with no
> > +# paths. Test that no deletion commands will be generated as usual.
> > +# This case didn't fail before.
> > +#
> > +#
> > +# Case 3: changed_*.file and changed_*.dir (BTRFS_COMPARE_TREE_CHANGED)
> > +#           |       | parent snapshot | send snapshot | action
> > +# -----------------------------------------------------------------------
> > +# subcase 1 | nlink |        0        |       0       | ignore
> > +# subcase 2 | nlink |       >0        |       0       | new_gen(deletion)
> > +#
> > +# In subcase 1, test that the send operation can ignore them without trying
> > +# to generate any commands.
> > +#
> > +# In subcase 2, test that the send operation can generate an unlink command
> > +# for that file and test that it can generate a rename command for the
> > +# non-empty directory first and a rmdir command to remove it finally. Or
> > +# the receive operation will fail with a wrong unlink on a non-empty
> > +# directory.
> > +#
> > +_run_btrfs_util_prog send -p $SCRATCH_MNT/snap1 -f $send_files_dir/2.snap \
> > +                    $SCRATCH_MNT/snap2
> > +
> > +$FSSUM_PROG -A -f -w $send_files_dir/1.fssum $SCRATCH_MNT/snap1
> > +$FSSUM_PROG -A -f -w $send_files_dir/2.fssum \
> > +       -x $SCRATCH_MNT/snap2/snap1 $SCRATCH_MNT/snap2
> > +
> > +# Recreate the filesystem by receiving both send streams and verify we get
> > +# the same content that the original filesystem had.
> > +exec 71>&-
> > +exec 72>&-
> > +exec 73>&-
> > +exec 74>&-
> > +exec 81>&-
> > +exec 82>&-
> > +exec 83>&-
> > +exec 84>&-
> > +exec 85>&-
> > +exec 86>&-
> > +_scratch_unmount
> > +_scratch_mkfs >>$seqres.full 2>&1
> > +_scratch_mount
> > +
> > +# Add the first snapshot to the new filesystem by applying the first send
> > +# stream.
> > +_run_btrfs_util_prog receive -f $send_files_dir/1.snap $SCRATCH_MNT
> > +
> > +# Test the incremental send stream
> > +_run_btrfs_util_prog receive -f $send_files_dir/2.snap $SCRATCH_MNT
> > +
> > +$FSSUM_PROG -r $send_files_dir/1.fssum $SCRATCH_MNT/snap1
> > +$FSSUM_PROG -r $send_files_dir/2.fssum $SCRATCH_MNT/snap2
> > +
> > +status=0
> > +exit
> > diff --git a/tests/btrfs/278.out b/tests/btrfs/278.out
> > new file mode 100644
> > index 00000000..82b93b4e
> > --- /dev/null
> > +++ b/tests/btrfs/278.out
> > @@ -0,0 +1,3 @@
> > +QA output created by 278
> > +OK
> > +OK
> > --
> > 2.37.1
> >




[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux