Re: [PATCH 2/2] btrfs: fix btrfs_find_device unused arg seed

[Nikolay Borisov <nborisov@suse.com>]

On 24.09.20 г. 13:21 ч., Nikolay Borisov wrote:
> 
> 
> On 24.09.20 г. 13:11 ч., Anand Jain wrote:
>> commit 343694eee8d8 (btrfs: switch seed device to list api), missed to
>> check if the last arg (seed) is true in the function btrfs_find_device().
>> This arg tells whether to traverse through the seed device list or not.
>>
>> This means after the above commit the arg is always true, and the parent
>> function which set this arg to false aren't effective.
>>
>> So we don't worry about the parent functions which set the last arg to
>> true, instead there is only one parent with calling btrfs_find_device
>> with the last arg false in device_list_add().
>>
>> But in fact, even the device_list_add() has no purpose that it has to set
>> the last arg to false. Because the fs_devices always points to the
>> device's fs_devices. So with the devid+uuid matching, it shall find the
>> btrfs_device and returns. So naturally, it won't traverse through the
>> seed fs_devices (if) present.
>>
>> So this patch makes it official that we don't need the last arg in the
>> function btrfs_find_device() and it shall always traverse through the
>> seed device list.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> 
> IMO the changelog could be vastly simplified by stating that following
> commit 343694eee8d8 the seed argument is no longer used and can simply
> be removed.
> 
> In any case
> 
> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> 
> 


Actually I went back and it seems that I accidentally removed the
portion of btrfs_find_device() which was predicated on seed bool
parameter being true or false. So the correct thing to do is really to
send a patch which adds it back and consider it a fix (i.e adding a
Fixes: tag as well).


OTOH - does it really make any difference? SO what if btrfs_find_device
returns a device from fs_devices->seed_list  in device_list_add (which
is called only from device scan)?