On 2021/11/16 21:52, David Sterba wrote:

On Tue, Nov 16, 2021 at 09:10:51PM +0800, Qu Wenruo wrote:

quoted

There is a bug in raid56_recov() which doesn't properly repair data and
P case corruption:

	/* Data and P*/
	if (dest2 == nr_devs - 1)
		return raid6_recov_datap(nr_devs, stripe_len, dest1, data);

Note that, dest1/2 is to indicate which slot has corruption.

For RAID6 cases:

[0, nr_devs - 2) is for data stripes,
@data_devs - 2 is for P,
@data_devs - 1 is for Q.

For above code, the comment is correct, but the check condition is
wrong, and leads to the only project, btrfs-fuse, to report raid6
recovery error for 2 devices missing case.

Fix it by using correct condition.

Signed-off-by: Qu Wenruo <redacted>
---
But I'm more interested in why this function is still there, as there
seems to be no caller of this function in btrfs-progs anyway.

The file is there from old times when the radi56 implementation landed
and the file was a copy of something in the lib/raid6 directory, but the
sources have diverged.

The function was not used as there was no repair code in userspace, so
the question is if w still want it there or remove it.

But then the problem is, how could userspace doesn't have any RAID56
recovery mechanism?

Things like btrfs check still needs RAID56 recovery to read fs with
missing devices, this doesn't make any sense to me.

The only good news is now we have another project which would do a full
coverage test for btrfs-progs...

Thanks,
Qu