On 2019/10/7 下午9:30, Nikolay Borisov wrote:
>
>
> On 7.10.19 г. 12:45 ч., Anand Jain wrote:
>> Following test case explains it all, even though the degraded mount is
>> successful the btrfs-progs fails to report the missing device.
>>
>> mkfs.btrfs -fq -draid1 -mraid1 /dev/sdc /dev/sdd && \
>> wipefs -a /dev/sdd && mount -o degraded /dev/sdc /btrfs && \
>> btrfs fi show -m /btrfs
>>
>> Label: none uuid: 2b3b8d92-572b-4d37-b4ee-046d3a538495
>> Total devices 2 FS bytes used 128.00KiB
>> devid 1 size 1.09TiB used 2.01GiB path /dev/sdc
>> devid 2 size 1.09TiB used 2.01GiB path /dev/sdd
>>
>> This is because btrfs-progs does it fundamentally wrong way that
>> it deduces the missing device status in the user land instead of
>> refuting from the kernel.
>>
>> At the same time in the kernel when we know that there is device
>> with non-btrfs magic, then remove that device from the list so
>> that btrfs-progs or someother userland utility won't be confused.
>>
>> Signed-off-by: Anand Jain <anand.j...@oracle.com>
>> ---
>> fs/btrfs/disk-io.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index 326d5281ad93..e05856432456 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -3417,7 +3417,7 @@ int btrfs_read_dev_one_super(struct block_device
>> *bdev, int copy_num,
>> if (btrfs_super_bytenr(super) != bytenr ||
>> btrfs_super_magic(super) != BTRFS_MAGIC) {
>> brelse(bh);
>> - return -EINVAL;
>> + return -EUCLEAN;
>
> This is really non-obvious and you are propagating the special-meaning
> of EUCLEAN waaaaaaaay beyond btrfs_open_one_device. In fact what this
> patch does is make the following call chain return EUCLAN:
>
> btrfs_open_one_device <-- finally removing the device in this function
> btrfs_get_bdev_and_sb <-- propagating it to here
> btrfs_read_dev_super
> btrfs_read_dev_one_super <-- you return the EUCLEAN
>
>
> And your commit log doesn't mention anything about that. EUCLEAN
> warrants a comment in this case since it changes behavior in
> higher-level layers.
And, for most case, EUCLEAN should have a proper kernel message for
what's going wrong, the value we hit and the value we expect.
And for EUCLEAN, it's more like graceful error out for impossible thing.
This is definitely not the case, and I believe the original EINVAL makes
more sense than EUCLEAN.
Thanks,
Qu
>
>> }
>>
>> *bh_ret = bh;
>>
