Re: [PATCH 6/6] btrfs: refactor btrfs_find_device() return error code

[David Sterba <dsterba@suse.cz>]

On Fri, Jan 18, 2019 at 02:13:26PM +0800, Anand Jain wrote:
> 
> 
> On 01/17/2019 11:49 PM, David Sterba wrote:
> > On Thu, Jan 17, 2019 at 11:32:33PM +0800, Anand Jain wrote:
> >> Refactor btrfs_find_device() to return standard error code.
> > 
> > Do you intend to add more error codes?
> 
>   No actually.
> 
>   code like this ...
>      if (PTR_ERR(device) == -ENOENT)
>   can be
>      if (IS_ERR(device))
> 
> 
> > A NULL is valid in case it's
> > clear that it always means 'device not found'.
> > 
> 
>   Oh. I am not too particular, I am ok to drop this patch.
> 
>   The thought process was.. having btrfs_find_device() to return standard
>   err code made code at @@ -2404,8 +2404,6 @@; @@ -2419,13 +2417,9 @@;
>   simpler.

Well, that piece of code is simpler but when compared to the amount of
other changes in the rest it's not making it simpler.

>   And as ioctls already return -ENODEV (though it might not be correct
>   one),

Unrelated to this patch, but I tend to agree that ENODEV is wrong.
However it's been in use for a long time so we can't change easily.

>   so if btrfs_find_device() could return -ENODEV (instead of
>   -ENOENT) then there are few more places where code can be just ..
>      if (IS_ERR(device))
> 	return PTR_ERR(device);

I've seen a lot return values of btrfs_find_device are for internal
purposes so the translation from NULL pointer to ENODEV/ENOENT does not
happen that often. In some contexts it's even something different like
EUCLEAN, but I maybe see the motivation to change it to ERR_PTR, as
btrfs_find_device_by_devspec and btrfs_find_device_missing_or_by_path
return that - btrfs_find_device is different here.