| Message ID | 1552988980-25710-1-git-send-email-anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fstests: btrfs try use forget to unregister device | expand |
On Tue, Mar 19, 2019 at 05:49:40PM +0800, Anand Jain wrote: > btrfs module reload was introduced to unregister devices in the btrfs > kernel module. > > The problem with the module reload approach is that you can't run btrfs > test cases 124, 125, 154 and 164 on the system with btrfs as root fs. > > Patches [1] introduced btrfs forget feature which lets to cleanup the > kernel device list without kernel module reload. > > [1] > btrfs-progs: add cli to forget one or all scanned devices > btrfs: introduce new ioctl to unregister a btrfs device > > So this patch uses forget feature instead of kernel module reload, if > the forget feature is available. > > Signed-off-by: Anand Jain <anand.jain@oracle.com> Looks fine to me. But I'd like an explicit review from btrfs folks too. Thanks, Eryu > --- > common/btrfs | 20 ++++++++++++++++++++ > tests/btrfs/124 | 6 +++--- > tests/btrfs/125 | 6 +++--- > tests/btrfs/154 | 6 +++--- > tests/btrfs/164 | 4 ++-- > 5 files changed, 31 insertions(+), 11 deletions(-) > > diff --git a/common/btrfs b/common/btrfs > index f6513c06f95f..e94e011db04e 100644 > --- a/common/btrfs > +++ b/common/btrfs > @@ -382,3 +382,23 @@ _scratch_btrfs_sectorsize() > $BTRFS_UTIL_PROG inspect-internal dump-super $SCRATCH_DEV |\ > grep sectorsize | awk '{print $2}' > } > + > +_btrfs_supports_forget() > +{ > + $BTRFS_UTIL_PROG device scan --help | grep -wq forget && \ > + $BTRFS_UTIL_PROG device scan --forget > /dev/null 2>&1 > +} > + > +_require_btrfs_forget_if_not_fs_loadable() > +{ > + _btrfs_supports_forget && return > + > + _require_loadable_fs_module "btrfs" > +} > + > +_btrfs_forget_if_not_fs_reload() > +{ > + _btrfs_supports_forget && return > + > + _reload_fs_module "btrfs" > +} > diff --git a/tests/btrfs/124 b/tests/btrfs/124 > index a52c65f608ff..9341dcea8896 100755 > --- a/tests/btrfs/124 > +++ b/tests/btrfs/124 > @@ -51,7 +51,7 @@ _supported_fs btrfs > _supported_os Linux > _require_scratch_dev_pool 2 > _test_unmount > -_require_loadable_fs_module "btrfs" > +_require_btrfs_forget_if_not_fs_loadable > > _scratch_dev_pool_get 2 > > @@ -86,7 +86,7 @@ echo "clean btrfs ko" >> $seqres.full > _scratch_unmount > > # un-scan the btrfs devices > -_reload_fs_module "btrfs" > +_btrfs_forget_if_not_fs_reload > > echo >> $seqres.full > echo "-----Write degraded mount fill upto $max_fs_sz bytes-----" >> $seqres.full > @@ -125,7 +125,7 @@ echo > echo "Mount degraded with the other dev" > _scratch_unmount > # un-scan the btrfs devices > -_reload_fs_module "btrfs" > +_btrfs_forget_if_not_fs_reload > _mount -o degraded $dev2 $SCRATCH_MNT >>$seqres.full 2>&1 > _run_btrfs_util_prog filesystem show > checkpoint3=`md5sum $SCRATCH_MNT/tf2` > diff --git a/tests/btrfs/125 b/tests/btrfs/125 > index 847fa62ad25f..3d847033f1a2 100755 > --- a/tests/btrfs/125 > +++ b/tests/btrfs/125 > @@ -50,7 +50,7 @@ _supported_fs btrfs > _supported_os Linux > _require_scratch_dev_pool 3 > _test_unmount > -_require_loadable_fs_module "btrfs" > +_require_btrfs_forget_if_not_fs_loadable > _require_btrfs_fs_feature raid56 > > _scratch_dev_pool_get 3 > @@ -103,7 +103,7 @@ echo "unmount" >> $seqres.full > _scratch_unmount > echo "clean btrfs ko" >> $seqres.full > # un-scan the btrfs devices > -_reload_fs_module "btrfs" > +_btrfs_forget_if_not_fs_reload > _mount -o degraded,device=$dev2 $dev1 $SCRATCH_MNT >>$seqres.full 2>&1 > dd if=/dev/zero of="$SCRATCH_MNT"/tf2 bs=$bs count=$count \ > >>$seqres.full 2>&1 > @@ -139,7 +139,7 @@ echo "Mount degraded but with other dev" > > _scratch_unmount > # un-scan the btrfs devices > -_reload_fs_module "btrfs" > +_btrfs_forget_if_not_fs_reload > > _mount -o degraded,device=${dev2} $dev3 $SCRATCH_MNT >>$seqres.full 2>&1 > > diff --git a/tests/btrfs/154 b/tests/btrfs/154 > index cd6c688fb9fe..e39f54ac6ab8 100755 > --- a/tests/btrfs/154 > +++ b/tests/btrfs/154 > @@ -36,7 +36,7 @@ rm -f $seqres.full > _supported_fs btrfs > _supported_os Linux > _require_scratch_dev_pool 2 > -_require_loadable_fs_module "btrfs" > +_require_btrfs_forget_if_not_fs_loadable > > _scratch_dev_pool_get 2 > > @@ -90,7 +90,7 @@ degrade_mount_write() > > echo "clean btrfs ko" >> $seqres.full > # un-scan the btrfs devices > - _reload_fs_module "btrfs" > + _btrfs_forget_if_not_fs_reload > _mount -o degraded $DEV1 $SCRATCH_MNT >>$seqres.full 2>&1 > cnt=$(( $COUNT/10 )) > dd if=/dev/urandom of="$SCRATCH_MNT"/tf1 bs=$bs count=$cnt \ > @@ -142,7 +142,7 @@ verify() > echo "unmount" >> $seqres.full > > _scratch_unmount > - _reload_fs_module "btrfs" > + _btrfs_forget_if_not_fs_reload > _mount -o degraded $DEV2 $SCRATCH_MNT >>$seqres.full 2>&1 > verify_checkpoint1=`md5sum $SCRATCH_MNT/tf1` > verify_checkpoint2=`md5sum $SCRATCH_MNT/tf2` > diff --git a/tests/btrfs/164 b/tests/btrfs/164 > index 097191a0e493..55042c4035e0 100755 > --- a/tests/btrfs/164 > +++ b/tests/btrfs/164 > @@ -36,7 +36,7 @@ rm -f $seqres.full > # Modify as appropriate. > _supported_fs btrfs > _supported_os Linux > -_require_loadable_fs_module "btrfs" > +_require_btrfs_forget_if_not_fs_loadable > _require_scratch_dev_pool 2 > > _scratch_dev_pool_get 2 > @@ -69,7 +69,7 @@ delete_seed() > { > _run_btrfs_util_prog device delete $dev_seed $SCRATCH_MNT > _scratch_unmount > - _reload_fs_module "btrfs" > + _btrfs_forget_if_not_fs_reload > run_check _mount $dev_sprout $SCRATCH_MNT > _run_btrfs_util_prog filesystem show -m $SCRATCH_MNT > echo -- sprout -- > -- > 1.8.3.1 >
On Tue, Mar 19, 2019 at 05:49:40PM +0800, Anand Jain wrote: > btrfs module reload was introduced to unregister devices in the btrfs > kernel module. > > The problem with the module reload approach is that you can't run btrfs > test cases 124, 125, 154 and 164 on the system with btrfs as root fs. > > Patches [1] introduced btrfs forget feature which lets to cleanup the > kernel device list without kernel module reload. > > [1] > btrfs-progs: add cli to forget one or all scanned devices The subject lines was changed to "btrfs-progs: device scan: add new option to forget one or all scanned devices" > btrfs: introduce new ioctl to unregister a btrfs device > So this patch uses forget feature instead of kernel module reload, if > the forget feature is available. > > Signed-off-by: Anand Jain <anand.jain@oracle.com> > --- > common/btrfs | 20 ++++++++++++++++++++ > tests/btrfs/124 | 6 +++--- > tests/btrfs/125 | 6 +++--- > tests/btrfs/154 | 6 +++--- > tests/btrfs/164 | 4 ++-- > 5 files changed, 31 insertions(+), 11 deletions(-) > > diff --git a/common/btrfs b/common/btrfs > index f6513c06f95f..e94e011db04e 100644 > --- a/common/btrfs > +++ b/common/btrfs > @@ -382,3 +382,23 @@ _scratch_btrfs_sectorsize() > $BTRFS_UTIL_PROG inspect-internal dump-super $SCRATCH_DEV |\ > grep sectorsize | awk '{print $2}' > } > + > +_btrfs_supports_forget() > +{ > + $BTRFS_UTIL_PROG device scan --help | grep -wq forget && \ > + $BTRFS_UTIL_PROG device scan --forget > /dev/null 2>&1 The second part actually unregisters all devices, is there some more lightweight way to detect the support? Like providing a valid block device but without btrfs. If the ioctl is supported, then it returns -ENOENT and if not supported then -EOPNOTSUPP. > +} > + > +_require_btrfs_forget_if_not_fs_loadable() _require_btrfs_forget_or_module_loadable We don't want to load the filesystem but the kernel module. > +{ > + _btrfs_supports_forget && return Why is the 'return' here? If the first command succeeds, then && proceeds to return that does implicitli returns 0. So it's redundant, or I'm missing something subtle here. > + > + _require_loadable_fs_module "btrfs" > +} > + > +_btrfs_forget_if_not_fs_reload() Same naming > +{ > + _btrfs_supports_forget && return > + > + _reload_fs_module "btrfs" > +}
On 3/29/19 1:50 AM, David Sterba wrote: > On Tue, Mar 19, 2019 at 05:49:40PM +0800, Anand Jain wrote: >> btrfs module reload was introduced to unregister devices in the btrfs >> kernel module. >> >> The problem with the module reload approach is that you can't run btrfs >> test cases 124, 125, 154 and 164 on the system with btrfs as root fs. >> >> Patches [1] introduced btrfs forget feature which lets to cleanup the >> kernel device list without kernel module reload. >> >> [1] >> btrfs-progs: add cli to forget one or all scanned devices > > The subject lines was changed to "btrfs-progs: device scan: add new > option to forget one or all scanned devices" ok thanks will fix. >> btrfs: introduce new ioctl to unregister a btrfs device > >> So this patch uses forget feature instead of kernel module reload, if >> the forget feature is available. >> >> Signed-off-by: Anand Jain <anand.jain@oracle.com> >> --- >> common/btrfs | 20 ++++++++++++++++++++ >> tests/btrfs/124 | 6 +++--- >> tests/btrfs/125 | 6 +++--- >> tests/btrfs/154 | 6 +++--- >> tests/btrfs/164 | 4 ++-- >> 5 files changed, 31 insertions(+), 11 deletions(-) >> >> diff --git a/common/btrfs b/common/btrfs >> index f6513c06f95f..e94e011db04e 100644 >> --- a/common/btrfs >> +++ b/common/btrfs >> @@ -382,3 +382,23 @@ _scratch_btrfs_sectorsize() >> $BTRFS_UTIL_PROG inspect-internal dump-super $SCRATCH_DEV |\ >> grep sectorsize | awk '{print $2}' >> } >> + >> +_btrfs_supports_forget() >> +{ >> + $BTRFS_UTIL_PROG device scan --help | grep -wq forget && \ >> + $BTRFS_UTIL_PROG device scan --forget > /dev/null 2>&1 > > The second part actually unregisters all devices, is there some more > lightweight way to detect the support? > Like providing a valid block > device but without btrfs. If the ioctl is supported, then it returns > -ENOENT and if not supported then -EOPNOTSUPP. _btrfs_supports_forget() has two different usage _require_btrfs_forget_if_not_fs_loadable which is called in the beginning of the test so its fine to clean all the (unmounted) devices in the kernel. _btrfs_forget_if_not_fs_reload Earlier we were reloading the btrfs.ko in the middle of the test case, so this is certainly more lightweight. Looked into the return values if I could use any better [1]. So _require_btrfs_forget_if_not_fs_loadable() need a btrfs on a loop device to verify if we need to avoid unregister all devices. Bit messy though. [1] With btrfs patches: We still get error if the device is not in the kernel dev_list. -------- # mkfs.btrfs -fq /dev/sdb # wipefs -a /dev/sdb /dev/sdb: 8 bytes were erased at offset 0x00010040 (btrfs): 5f 42 48 52 66 53 5f 4d # mkfs.xfs -fq /dev/sdb # btrfs dev scan --forget /dev/sdb; echo $? 0 # btrfs dev scan --forget /dev/sdb; echo $? ERROR: Can't forget '/dev/sdb': No such file or directory 1 ---------- With out any btrfs forget patches: Usage error and error code = 1 (it should rather be EINVAL). -------- # btrfs dev scan --forget /dev/sda usage: btrfs device scan [(-d|--all-devices)|<device> [<device>...]] Scan devices for a btrfs filesystem -d|--all-devices (deprecated) 1 ----------- With out the btrfs kernel patch: Returns error code 1. (After the patch sent to the ML it will return EOPNOTSUPP). --------------- # btrfs dev scan --forget /dev/loop0; echo $? ERROR: Can't forget '/dev/loop0': Inappropriate ioctl for device 1 --------------- >> +} >> + >> +_require_btrfs_forget_if_not_fs_loadable() > > _require_btrfs_forget_or_module_loadable > > We don't want to load the filesystem but the kernel module. Will fix. >> +{ >> + _btrfs_supports_forget && return > > Why is the 'return' here? If the first command succeeds, then && > proceeds to return that does implicitli returns 0. So it's redundant, or > I'm missing something subtle here. Hmm. Not sure if I understood. Do you mean we could _btrfs_supports_forget || _require_loadable_fs_module "btrfs" >> + >> + _require_loadable_fs_module "btrfs" >> +} >> + >> +_btrfs_forget_if_not_fs_reload() > > Same naming Ok Thanks, Anand > >> +{ >> + _btrfs_supports_forget && return >> + >> + _reload_fs_module "btrfs" >> +}
diff --git a/common/btrfs b/common/btrfs index f6513c06f95f..e94e011db04e 100644 --- a/common/btrfs +++ b/common/btrfs @@ -382,3 +382,23 @@ _scratch_btrfs_sectorsize() $BTRFS_UTIL_PROG inspect-internal dump-super $SCRATCH_DEV |\ grep sectorsize | awk '{print $2}' } + +_btrfs_supports_forget() +{ + $BTRFS_UTIL_PROG device scan --help | grep -wq forget && \ + $BTRFS_UTIL_PROG device scan --forget > /dev/null 2>&1 +} + +_require_btrfs_forget_if_not_fs_loadable() +{ + _btrfs_supports_forget && return + + _require_loadable_fs_module "btrfs" +} + +_btrfs_forget_if_not_fs_reload() +{ + _btrfs_supports_forget && return + + _reload_fs_module "btrfs" +} diff --git a/tests/btrfs/124 b/tests/btrfs/124 index a52c65f608ff..9341dcea8896 100755 --- a/tests/btrfs/124 +++ b/tests/btrfs/124 @@ -51,7 +51,7 @@ _supported_fs btrfs _supported_os Linux _require_scratch_dev_pool 2 _test_unmount -_require_loadable_fs_module "btrfs" +_require_btrfs_forget_if_not_fs_loadable _scratch_dev_pool_get 2 @@ -86,7 +86,7 @@ echo "clean btrfs ko" >> $seqres.full _scratch_unmount # un-scan the btrfs devices -_reload_fs_module "btrfs" +_btrfs_forget_if_not_fs_reload echo >> $seqres.full echo "-----Write degraded mount fill upto $max_fs_sz bytes-----" >> $seqres.full @@ -125,7 +125,7 @@ echo echo "Mount degraded with the other dev" _scratch_unmount # un-scan the btrfs devices -_reload_fs_module "btrfs" +_btrfs_forget_if_not_fs_reload _mount -o degraded $dev2 $SCRATCH_MNT >>$seqres.full 2>&1 _run_btrfs_util_prog filesystem show checkpoint3=`md5sum $SCRATCH_MNT/tf2` diff --git a/tests/btrfs/125 b/tests/btrfs/125 index 847fa62ad25f..3d847033f1a2 100755 --- a/tests/btrfs/125 +++ b/tests/btrfs/125 @@ -50,7 +50,7 @@ _supported_fs btrfs _supported_os Linux _require_scratch_dev_pool 3 _test_unmount -_require_loadable_fs_module "btrfs" +_require_btrfs_forget_if_not_fs_loadable _require_btrfs_fs_feature raid56 _scratch_dev_pool_get 3 @@ -103,7 +103,7 @@ echo "unmount" >> $seqres.full _scratch_unmount echo "clean btrfs ko" >> $seqres.full # un-scan the btrfs devices -_reload_fs_module "btrfs" +_btrfs_forget_if_not_fs_reload _mount -o degraded,device=$dev2 $dev1 $SCRATCH_MNT >>$seqres.full 2>&1 dd if=/dev/zero of="$SCRATCH_MNT"/tf2 bs=$bs count=$count \ >>$seqres.full 2>&1 @@ -139,7 +139,7 @@ echo "Mount degraded but with other dev" _scratch_unmount # un-scan the btrfs devices -_reload_fs_module "btrfs" +_btrfs_forget_if_not_fs_reload _mount -o degraded,device=${dev2} $dev3 $SCRATCH_MNT >>$seqres.full 2>&1 diff --git a/tests/btrfs/154 b/tests/btrfs/154 index cd6c688fb9fe..e39f54ac6ab8 100755 --- a/tests/btrfs/154 +++ b/tests/btrfs/154 @@ -36,7 +36,7 @@ rm -f $seqres.full _supported_fs btrfs _supported_os Linux _require_scratch_dev_pool 2 -_require_loadable_fs_module "btrfs" +_require_btrfs_forget_if_not_fs_loadable _scratch_dev_pool_get 2 @@ -90,7 +90,7 @@ degrade_mount_write() echo "clean btrfs ko" >> $seqres.full # un-scan the btrfs devices - _reload_fs_module "btrfs" + _btrfs_forget_if_not_fs_reload _mount -o degraded $DEV1 $SCRATCH_MNT >>$seqres.full 2>&1 cnt=$(( $COUNT/10 )) dd if=/dev/urandom of="$SCRATCH_MNT"/tf1 bs=$bs count=$cnt \ @@ -142,7 +142,7 @@ verify() echo "unmount" >> $seqres.full _scratch_unmount - _reload_fs_module "btrfs" + _btrfs_forget_if_not_fs_reload _mount -o degraded $DEV2 $SCRATCH_MNT >>$seqres.full 2>&1 verify_checkpoint1=`md5sum $SCRATCH_MNT/tf1` verify_checkpoint2=`md5sum $SCRATCH_MNT/tf2` diff --git a/tests/btrfs/164 b/tests/btrfs/164 index 097191a0e493..55042c4035e0 100755 --- a/tests/btrfs/164 +++ b/tests/btrfs/164 @@ -36,7 +36,7 @@ rm -f $seqres.full # Modify as appropriate. _supported_fs btrfs _supported_os Linux -_require_loadable_fs_module "btrfs" +_require_btrfs_forget_if_not_fs_loadable _require_scratch_dev_pool 2 _scratch_dev_pool_get 2 @@ -69,7 +69,7 @@ delete_seed() { _run_btrfs_util_prog device delete $dev_seed $SCRATCH_MNT _scratch_unmount - _reload_fs_module "btrfs" + _btrfs_forget_if_not_fs_reload run_check _mount $dev_sprout $SCRATCH_MNT _run_btrfs_util_prog filesystem show -m $SCRATCH_MNT echo -- sprout --
btrfs module reload was introduced to unregister devices in the btrfs kernel module. The problem with the module reload approach is that you can't run btrfs test cases 124, 125, 154 and 164 on the system with btrfs as root fs. Patches [1] introduced btrfs forget feature which lets to cleanup the kernel device list without kernel module reload. [1] btrfs-progs: add cli to forget one or all scanned devices btrfs: introduce new ioctl to unregister a btrfs device So this patch uses forget feature instead of kernel module reload, if the forget feature is available. Signed-off-by: Anand Jain <anand.jain@oracle.com> --- common/btrfs | 20 ++++++++++++++++++++ tests/btrfs/124 | 6 +++--- tests/btrfs/125 | 6 +++--- tests/btrfs/154 | 6 +++--- tests/btrfs/164 | 4 ++-- 5 files changed, 31 insertions(+), 11 deletions(-)