Re: [PATCH 4/4] btrfs: use btrfs_raid_array[].ncopies in btrfs_num_copies()

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

On 2022/5/13 17:15, Johannes Thumshirn wrote:
> On 13/05/2022 10:34, Qu Wenruo wrote:
>>   	map = em->map_lookup;
>> -	if (map->type & (BTRFS_BLOCK_GROUP_DUP | BTRFS_BLOCK_GROUP_RAID1_MASK))
>> -		ret = map->num_stripes;
>> -	else if (map->type & BTRFS_BLOCK_GROUP_RAID10)
>> -		ret = map->sub_stripes;
>> +	index = btrfs_bg_flags_to_raid_index(map->type);
>> +
>> +	if (!(map->type & BTRFS_BLOCK_GROUP_RAID56_MASK))
>> +		/* Non-raid56, use their ncopies from btrfs_raid_array[]. */
>> +		ret = btrfs_raid_array[index].ncopies;
>>   	else if (map->type & BTRFS_BLOCK_GROUP_RAID5)
>>   		ret = 2;
>>   	else if (map->type & BTRFS_BLOCK_GROUP_RAID6)
>
> Here I'm not 100% sure. RAID10 used sub_stripes and now it's using ncopies.
> The code still produces the same result, as for RAID10 ncopies == sub_stripes,
> but semantically it's different (I think).

Since the objective of the code is the get the number of mirrors for the
chunk, sub_stripes is definitely not correct here.

Just imagine if we have RAID1C30 (sounds ugly I know), which is RAID1C3
first, then RAID0.

We should still have sub_stripes == 2, but ncopies == 3.

And the number of mirrors is definitely 3, not 2.

So it's the old code not correctly semantically.

Thanks,
Qu