Re: [PATCH] fstests: btrfs try use forget to unregister device

[David Sterba <dsterba@suse.cz>]

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"
> +}