Re: [PATCH v4 2/4] btrfs: redeclare btrfs_stale_devices arg1 to dev_t

[Nikolay Borisov <nborisov@suse.com>]

On 10.01.22 г. 15:23, Anand Jain wrote:
> After the commit (btrfs: harden identification of the stale device), we
> don't have to match the device path anymore. Instead, we match the dev_t.
> So pass in the dev_t instead of the device-path, in the call chain
> btrfs_forget_devices()->btrfs_free_stale_devices().
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> v4: Change log updated drop commit id
>     Use name[0] instead of strlen()
>     Change logic from !lookup_bdev() to lookup_bdev == 0
> v3: -
> 
>  fs/btrfs/super.c   |  8 +++++++-
>  fs/btrfs/volumes.c | 51 +++++++++++++++++++++++++---------------------
>  fs/btrfs/volumes.h |  2 +-
>  3 files changed, 36 insertions(+), 25 deletions(-)
> 
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 1ff03fb6c64a..6eca93096ca5 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -2386,6 +2386,7 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
>  {
>  	struct btrfs_ioctl_vol_args *vol;
>  	struct btrfs_device *device = NULL;
> +	dev_t devt = 0;
>  	int ret = -ENOTTY;
>  
>  	if (!capable(CAP_SYS_ADMIN))
> @@ -2405,7 +2406,12 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
>  		mutex_unlock(&uuid_mutex);
>  		break;
>  	case BTRFS_IOC_FORGET_DEV:
> -		ret = btrfs_forget_devices(vol->name);
> +		if (vol->name[0] != '\0') {
> +			ret = lookup_bdev(vol->name, &devt);
> +			if (ret)
> +				break;
> +		}
> +		ret = btrfs_forget_devices(devt);
>  		break;
>  	case BTRFS_IOC_DEVICES_READY:
>  		mutex_lock(&uuid_mutex);

<snip>

> @@ -577,16 +572,16 @@ static int device_matched(struct btrfs_device *device, const char *path)
>  /*
>   *  Search and remove all stale (devices which are not mounted) devices.
>   *  When both inputs are NULL, it will search and release all stale devices.
> - *  path:	Optional. When provided will it release all unmounted devices
> - *		matching this path only.
> + *  devt:	Optional. When provided will it release all unmounted devices
> + *		matching this devt only.
>   *  skip_dev:	Optional. Will skip this device when searching for the stale
>   *		devices.
> - *  Return:	0 for success or if @path is NULL.
> - * 		-EBUSY if @path is a mounted device.
> - * 		-ENOENT if @path does not match any device in the list.
> + *  Return:	0 for success or if @devt is 0.
> + *		-EBUSY if @devt is a mounted device.
> + *		-ENOENT if @devt does not match any device in the list.
>   */
> -static int btrfs_free_stale_devices(const char *path,
> -				     struct btrfs_device *skip_device)
> +static int btrfs_free_stale_devices(dev_t devt,
> +				    struct btrfs_device *skip_device)
>  {
>  	struct btrfs_fs_devices *fs_devices, *tmp_fs_devices;
>  	struct btrfs_device *device, *tmp_device;
> @@ -594,7 +589,7 @@ static int btrfs_free_stale_devices(const char *path,
>  
>  	lockdep_assert_held(&uuid_mutex);
>  
> -	if (path)
> +	if (devt)
>  		ret = -ENOENT;
>  
>  	list_for_each_entry_safe(fs_devices, tmp_fs_devices, &fs_uuids, fs_list) {
> @@ -604,14 +599,14 @@ static int btrfs_free_stale_devices(const char *path,
>  					 &fs_devices->devices, dev_list) {
>  			if (skip_device && skip_device == device)
>  				continue;
> -			if (path && !device->name)
> +			if (devt && !device->name)

This check is now rendered obsolete since ->name is used iff we have
passed a patch to match against it, but since your series removes the
path altogether having device->name becomes obsolete, hence it can be
removed.

<snip>