Re: [PATCH] btrfs: Create sysfs entries only for non-stale devices

[Goffredo Baroncelli <kreijack@libero.it>]

On 12/15/21 23:02, Nikolay Borisov wrote:
> 
> 
> On 15.12.21 г. 20:55, Goffredo Baroncelli wrote:
>> Hi Nikolay,
>>
>> On 12/15/21 15:46, Nikolay Borisov wrote:
>>> Currently /sys/fs/btrfs/<uuid>/devinfo/<devid> entry is always created
>>> for a device present in btrfs_fs_devices on the other hand
>>> /sys/fs/btrfs/<uuid>/devices/<devname> sysfs link is created only when
>>> the given btrfs_device::bdisk member is populated. This can lead to
>>> cases where a filesystem consisting of 2 device, one of which is stale
>>> ends up having 2 entries under /sys/fs/btrfs/<uuid>/devinfo/<devid>
>>> but only one under /sys/fs/btrfs/<uuid>/devices/<devname>.
>>
>> What happened in case of a degraded mode ? Is correct to not show a
>> missing devices ?
> 
> Good question, now I'm thinking if 'devices' show the currently
> available devices to the filesystem, whilst 'devinfo' is supposed to
> show what devices the fs knows about. So in the case of degraded mount
> it should really have 2 devices under devinfo and only 1 under device.
> But this also means the case you reported shouldn't be handled by the
> devinfo sysfs code but rather the admin should do 'btrfs device scan -u'
> to remove the stale device.
> 
> 
> I'd say this is all pretty open to interpretation so I'd like to also
> see David's and Josef's opinions on this.

After some thinking, I reach the conclusion that devices/ shows the correct
values by mistake :-)

My understanding of a btrfs filesystem bootstrap is the following:

- each time a new block device appears, if this is a valid BTRFS device,
it is registered in a list
- at mount time, BTRFS groups all the devices with the same FS-UUID
- during the mount, it is assumed that if a device has a valid FS-UUID it
is a valid block devices for the filesystem

BTRFS has the following information to detect "foreign" block devices:
1) the UUID of each block devices should match the ones stored in DEV_ITEM (in the metadata)
2) the generation number should match
3) the "num_devices" field of superblock should match the total number of devices

In this case I see two problems:

a) the device was registered but it was unavailable at the time of mounting.
I don't think that it should be considered valid at all and it should be
removed from the list of the available devices when the first access for
check 1) and 2) is tried.
I know that a device can disappear after, but this case is different: if we
can perform 1) and 2) we can classify the device as valid/invalid
If we cannot do (as this case) we can consider it an artifact and discard enterly

b) in any case the check 3) ( 1) and 1) cannot be performed without the device)
should prevent the filesystem to be mounted if num_devices < number of the
listed devices (which is different from the available device). Looking at the
code it seems that only the case of a missing devices is handled










> 
>>
>>
> <snip>


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5