Comment 8 for bug 1847924

Revision history for this message
Guilherme G. Piccoli (gpiccoli) wrote :

Hi Robie, thanks for you concerns! They are valid and make sense. Let me first give you some context on why this is needed in stable releases: the mdadm patch is a counter-part of a kernel modification which performs 2 things: (a) 'broken' state when raid0/linear arrays have a member missing; (b) refuse write I/O to such broken arrays.

Without this kernel change, we can continue writing to a broken md device until FS notices some error, which may take a while. And due to writeback/page-cache mechanism, this would be transparent to the regular user, only being noticed by users looking logs and stats of disk writing. The kernel page-cache leads to a situation in which commands like 'dd' or even 'sync' succeeds to a broken device, leading to a partial-written/corrupted file.
So, given we are in the process of SRU the kernel change, the mdadm counter-part is highly desirable due to the (a) part above. Or else, we get an incomplete functionality.

Regarding your points:
1) Agreed! The change was well-tested though, and validated by 3 maintainers (2 from kernel md subsystem, one from mdadm itself). Also, despite the patch is a bit large, it basically adds this 'broken' state to places where it's needed, like arrays and defines (hence its size is not so small) and restrict the change clearly to md0/linear, not affecting at all any other levels.

2) You're partially right here, mdadm keeps working *exactly* the same for everything _except_ for raid0/linear status, in '--detail' option. For those 2 levels (raid0/linear), now we read the sysfs state of the array from sysfs. So, it's a behavior change, but a correct (or at least, well-accepted) one. And it's concise/scope-reduced, as it doesn't change any other level or any other functionality in raid0/linear, except state query.

Feel free to comment in case you have more questions, or ping me in IRC also (gpiccoli in freenode).

Now, I have one question for you: do you prefer we do a merge in Focal before accepting this SRU? In order to sync Focal with upstream, I mean.
Thanks,

Guilherme