Re: [PATCH] md/raid0: Fail BIOs if their underlying block device is gone

[Song Liu <songliubraving@fb.com>]

> On Aug 1, 2019, at 1:28 PM, Guilherme G. Piccoli <gpiccoli@canonical.com> wrote:
> 
> 
> 
> On 31/07/2019 16:56, Song Liu wrote:
>> On Wed, Jul 31, 2019 at 12:54 PM Song Liu <liu.song.a23@gmail.com> wrote:
>>> 
>>> On Tue, Jul 30, 2019 at 5:31 AM Guilherme G. Piccoli
>>> <gpiccoli@canonical.com> wrote:
>>>> 
>>>> On 29/07/2019 21:08, NeilBrown wrote:
>>>>> [...]
>>>>>> +    if (unlikely(test_bit(MD_BROKEN, &mddev->flags))) {
>>>>>> +            bio_io_error(bio);
>>>>>> +            return BLK_QC_T_NONE;
>>>>>> +    }
>>>>> 
>>>>> I think this should only fail WRITE requests, not READ requests.
>>>>> 
>>>>> Otherwise the patch is probably reasonable.
>>>>> 
>>>>> NeilBrown
>>>> 
>>>> Thanks for the feedback Neil! I thought about it; it seemed to me better
>>>> to deny/fail the reads instead of returning "wrong" reads, since a file
>>>> read in a raid0 will be incomplete if one member is missing.
>>>> But it's fine for me to change that in the next iteration of this patch.
>>> 
>>> For reads at block/page level, we will either get EIO or valid data, right?
>>> 
>>> If that's not the case, we should fail all writes.
>> 
>> Oops, I meant all _reads_.
> 
> Hi Song, thanks for the feedback! After changing the patch and testing a
> bit, it behaves exactly as you said, we got either valid data read from
> the healthy devices or -EIO for the data tentatively read from the
> failed/missing array members.

Thanks for testing this out. 

> 
> So, I'll resubmit with that change. Also, I've noticed clearing the
> BROKEN flag seem unnecessary, if user stops the array in order to fix
> the missing member, it'll require a re-assembly and the array is gonna
> work again.
> 
> Do you / Neil considers this fix relevant to md/linear too? If so, I can
> also include that in the V2.

Yes, please also include fix for md/linear. 

Song