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

[Anand Jain <anand.jain@oracle.com>]

On 25/9/20 4:22 pm, Nikolay Borisov wrote:
> 
> 
> 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)?
> 

There isn't bug as such. device scan is through the btrfs_control 
interface. So it depends on the device fsid to get fs_devices and it 
could never use seed_list with a genuine (non crafted) disk image.

Thanks, Anand