Re: [PATCH v9 03/11] btrfs: add support for inserting raid stripe extents

[Johannes Thumshirn]

On 15.09.23 02:55, Qu Wenruo wrote:
> 
> I found a corncer case that this can be problematic.
> 
> If we have a fs with RST root tree node/leaf corrupted, mounted with
> rescue=ibadroots, then fs_info->stripe_root would be NULL, and in the
> 5th patch inside set_io_stripe() we just fall back to regular non-RST path.
> This would bring us mostly incorrect data (and can be very problematic
> for nodatacsum files).
> 
> Thus stripe_root itself is not a reliable way to determine if we're at
> RST routine, I'd say only super incompat flags is reliable.

Fixed.

> 
> And fs_info->stripe_root should only be checked for functions that do
> RST tree operations, and return -EIO properly if it's not initialized.



>> +
>> +	if (type != BTRFS_BLOCK_GROUP_DATA)
>> +		return false;
>> +
>> +	if (profile & BTRFS_BLOCK_GROUP_RAID1_MASK)
>> +		return true;
> 
> Just a stupid quest, RAID0 DATA doesn't need RST purely because they are
>    the same as SINGLE, thus we only update the file items to the real
> written logical address, and no need for the extra mapping?

Yes but there can still be discrepancies between the assumed physical 
address and the real one due to ZONE_APPEND operations. RST backed file 
systems don't go the "normal" zoned btrfs logical rewrite path but have 
their own.

Also I prefere to keep the stripes together.

> Thus only profiles with duplication relies on RST, right?
> If so, then I guess DUP should also be covered by RST.
> 

Later in this patches, DUP, RAID0 and RAID10 will get added as well.