| Message ID | 20211027135754.20101-1-nborisov@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: Test proper interaction between skip_balance and paused balance | expand |
On Wed, Oct 27, 2021 at 10:25 PM Nikolay Borisov <nborisov@suse.com> wrote: > > Ensure a device can be added to a filesystem that has a paused balance > operation and has been mounted with the 'skip_balance' mount option Please mention which patch the test relates to. > > Signed-off-by: Nikolay Borisov <nborisov@suse.com> > --- > tests/btrfs/049 | 47 +++++++++++++++++++++++++++++++++++++++++++++ > tests/btrfs/049.out | 1 + > 2 files changed, 48 insertions(+) > create mode 100755 tests/btrfs/049 > > diff --git a/tests/btrfs/049 b/tests/btrfs/049 > new file mode 100755 > index 000000000000..7f566ee112f1 > --- /dev/null > +++ b/tests/btrfs/049 > @@ -0,0 +1,47 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (c) 2021 SUSE Linux Products GmbH. All Rights Reserved. > +# > +# FS QA Test 049 > +# > +# Ensure that it's possible to add a device when we have a paused balance > +# and the filesystem is mounted with skip_balance. > +# > +. ./common/preamble > +_begin_fstest quick balance auto > + > +# real QA test starts here > + > +_supported_fs btrfs > +_require_scratch_dev_pool > + > +_scratch_mkfs >/dev/null > +_scratch_mount > + > +uuid=$(findmnt -n -o UUID $SCRATCH_MNT) > + > +if [[ ! -e /sys/fs/btrfs/$uuid/exclusive_operation ]]; then > + _notrun "Requires btrfs exclusive operation support" > +fi Why is it required to have the sysfs export file for the exclusive operations? The test doesn't use the file at all, and exclusive operations exist for many years, unlike the sysfs file which is recent. > + > +dev1="`echo $SCRATCH_DEV_POOL | $AWK_PROG '{print $2}'`" > + > +# Create some files on the so that balance doesn't complete instantly > +args=`_scale_fsstress_args -z \ > + -f write=10 -f creat=10 \ > + -n 1000 -p 2 -d $SCRATCH_MNT/stress_dir` > +echo "Run fsstress $args" >>$seqres.full > +$FSSTRESS_PROG $args >/dev/null 2>&1 > + > +# Start and pause balance to ensure it will be restored on remount > +echo "Start balance" >>$seqres.full > +_run_btrfs_balance_start --full-balance --bg "$SCRATCH_MNT" There's no need to pass --full-balance, _run_btrfs_balance_start already does that internally. Further, explicitly passing --full-balance will make the test fail on older versions of btrfs-progs that don't have that flag. With those fixed, you can add: Reviewed-by: Filipe Manana <fdmanana@suse.com> Thanks. > +$BTRFS_UTIL_PROG balance pause "$SCRATCH_MNT" > + > +_scratch_cycle_mount "skip_balance" > + > +$BTRFS_UTIL_PROG device add -K -f $dev1 "$SCRATCH_MNT" > + > +echo "Silence is golden" > +status=0 > +exit > diff --git a/tests/btrfs/049.out b/tests/btrfs/049.out > index cb0061b33ff0..c69568ad9323 100644 > --- a/tests/btrfs/049.out > +++ b/tests/btrfs/049.out > @@ -1 +1,2 @@ > QA output created by 049 > +Silence is golden > -- > 2.17.1 >
On 28.10.21 г. 12:01, Filipe Manana wrote: > On Wed, Oct 27, 2021 at 10:25 PM Nikolay Borisov <nborisov@suse.com> wrote: <snip> >> +if [[ ! -e /sys/fs/btrfs/$uuid/exclusive_operation ]]; then >> + _notrun "Requires btrfs exclusive operation support" >> +fi > > Why is it required to have the sysfs export file for the exclusive operations? > The test doesn't use the file at all, and exclusive operations exist > for many years, unlike the sysfs file which is recent. Because the report mentioned the following error message being printed: ERROR: unable to start device add, another exclusive operation 'balance' in progress And this comes from check_running_fs_exclop which got added as part of the sysfs interface for exclusive ops. > <snip>
On Thu, Oct 28, 2021 at 10:17 AM Nikolay Borisov <nborisov@suse.com> wrote: > > > > On 28.10.21 г. 12:01, Filipe Manana wrote: > > On Wed, Oct 27, 2021 at 10:25 PM Nikolay Borisov <nborisov@suse.com> wrote: > > <snip> > > >> +if [[ ! -e /sys/fs/btrfs/$uuid/exclusive_operation ]]; then > >> + _notrun "Requires btrfs exclusive operation support" > >> +fi > > > > Why is it required to have the sysfs export file for the exclusive operations? > > The test doesn't use the file at all, and exclusive operations exist > > for many years, unlike the sysfs file which is recent. > > Because the report mentioned the following error message being printed: > > ERROR: unable to start device add, another exclusive operation 'balance' > in progress > > And this comes from check_running_fs_exclop which got added as part of > the sysfs interface for exclusive ops. Ok, so check_running_fs_exclop() is a btrfs-progs function. But even before that, and the sysfs file was added, it was not possible to do a device add while there's stopped balance, no? > > > > > <snip>
On 28.10.21 г. 12:22, Filipe Manana wrote: > On Thu, Oct 28, 2021 at 10:17 AM Nikolay Borisov <nborisov@suse.com> wrote: >> >> >> >> On 28.10.21 г. 12:01, Filipe Manana wrote: >>> On Wed, Oct 27, 2021 at 10:25 PM Nikolay Borisov <nborisov@suse.com> wrote: >> >> <snip> >> >>>> +if [[ ! -e /sys/fs/btrfs/$uuid/exclusive_operation ]]; then >>>> + _notrun "Requires btrfs exclusive operation support" >>>> +fi >>> >>> Why is it required to have the sysfs export file for the exclusive operations? >>> The test doesn't use the file at all, and exclusive operations exist >>> for many years, unlike the sysfs file which is recent. >> >> Because the report mentioned the following error message being printed: >> >> ERROR: unable to start device add, another exclusive operation 'balance' >> in progress >> >> And this comes from check_running_fs_exclop which got added as part of >> the sysfs interface for exclusive ops. > > Ok, so check_running_fs_exclop() is a btrfs-progs function. > > But even before that, and the sysfs file was added, it was not > possible to do a device add while there's stopped balance, no? Most probably yes. Ok I will remove this check then. > >> >>> >> >> <snip> > > >
diff --git a/tests/btrfs/049 b/tests/btrfs/049 new file mode 100755 index 000000000000..7f566ee112f1 --- /dev/null +++ b/tests/btrfs/049 @@ -0,0 +1,47 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2021 SUSE Linux Products GmbH. All Rights Reserved. +# +# FS QA Test 049 +# +# Ensure that it's possible to add a device when we have a paused balance +# and the filesystem is mounted with skip_balance. +# +. ./common/preamble +_begin_fstest quick balance auto + +# real QA test starts here + +_supported_fs btrfs +_require_scratch_dev_pool + +_scratch_mkfs >/dev/null +_scratch_mount + +uuid=$(findmnt -n -o UUID $SCRATCH_MNT) + +if [[ ! -e /sys/fs/btrfs/$uuid/exclusive_operation ]]; then + _notrun "Requires btrfs exclusive operation support" +fi + +dev1="`echo $SCRATCH_DEV_POOL | $AWK_PROG '{print $2}'`" + +# Create some files on the so that balance doesn't complete instantly +args=`_scale_fsstress_args -z \ + -f write=10 -f creat=10 \ + -n 1000 -p 2 -d $SCRATCH_MNT/stress_dir` +echo "Run fsstress $args" >>$seqres.full +$FSSTRESS_PROG $args >/dev/null 2>&1 + +# Start and pause balance to ensure it will be restored on remount +echo "Start balance" >>$seqres.full +_run_btrfs_balance_start --full-balance --bg "$SCRATCH_MNT" +$BTRFS_UTIL_PROG balance pause "$SCRATCH_MNT" + +_scratch_cycle_mount "skip_balance" + +$BTRFS_UTIL_PROG device add -K -f $dev1 "$SCRATCH_MNT" + +echo "Silence is golden" +status=0 +exit diff --git a/tests/btrfs/049.out b/tests/btrfs/049.out index cb0061b33ff0..c69568ad9323 100644 --- a/tests/btrfs/049.out +++ b/tests/btrfs/049.out @@ -1 +1,2 @@ QA output created by 049 +Silence is golden
Ensure a device can be added to a filesystem that has a paused balance operation and has been mounted with the 'skip_balance' mount option Signed-off-by: Nikolay Borisov <nborisov@suse.com> --- tests/btrfs/049 | 47 +++++++++++++++++++++++++++++++++++++++++++++ tests/btrfs/049.out | 1 + 2 files changed, 48 insertions(+) create mode 100755 tests/btrfs/049