On Thu, Oct 31 2019, dann frazier wrote:
> On Thu, Oct 31, 2019 at 10:56:04AM +1100, NeilBrown wrote:
>> Since Linux 5.4 a layout is needed for RAID0 arrays with
>> varying device sizes.
>
> Thanks for working on this. It will be key to backport to distros
> along w/ the kernel side.
>
>> This patch makes the layout of an array visible (via --examine)
>> and sets the layout on newly created arrays.
>
> Probably a n00b question - but why is this visible in --examine but not
> --detail? Seems like a feature of the array vs. the members.
I've added the information to --detail.
>> --- a/maps.c
>> +++ b/maps.c
>> @@ -73,6 +73,18 @@ mapping_t r6layout[] = {
>> { NULL, UnSet }
>> };
>>
>> +/* raid0 layout is only needed because of a bug in 3.13 which changed
>
> s/3.13/3.14/
Fixed.
>
>> + * the effective layout of raid0 arrays with varying device sizes.
>> + */
>> +mapping_t r0layout[] = {
>> + { "original", RAID0_ORIG_LAYOUT},
>> + { "alternate", RAID0_ALT_MULTIZONE_LAYOUT},
>> + { "1", 1},
>> + { "2", 2},
>
> Nit - why not use the RAID0_ macros here as well, so it is clear which
> that e.g. "1" is an alias for "original"?
If I did that, it wouldn't be obvious from the code that the mapping
was correct. I've added comments.
>> --- a/mdadm.8.in
>> +++ b/mdadm.8.in
>> @@ -593,6 +593,8 @@ to change the RAID level in some cases. See LEVEL CHANGES below.
>> This option configures the fine details of data layout for RAID5, RAID6,
>> and RAID10 arrays, and controls the failure modes for
>
> Should this list now include RAID0?
No, I don't think so. But the line below should definitely mention
RAID0 - so I've added it there.
>
>> .IR faulty .
>> +It can also be used for working around a kernel bug, but generaly
>
> s/generaly/generally/
Fixed.
>> .I 'n'
>> @@ -677,6 +679,32 @@ devices in the array. It does not need to divide evenly into that
>> number (e.g. it is perfectly legal to have an 'n2' layout for an array
>> with an odd number of devices).
>>
>> +A bug in Linux 3.14 means that RAID0 arrays
>
> s/bug in/bug introduced in/
>
Fixed.
>> +An array created for either
>> +.RB ' original '
>> +or
>> +.RB ' alternate '
>> +with not be recognized by an (unpatched) kernel prior to 5.4. To create
>
> s/with not/will not/
>
> I noticed this in my testing - definitely a nice feature :)
Fixed.
>
>> --- a/super1.c
>> +++ b/super1.c
>> @@ -43,7 +43,7 @@ struct mdp_superblock_1 {
>>
>> __u64 ctime; /* lo 40 bits are seconds, top 24 are microseconds or 0*/
>> __u32 level; /* -4 (multipath), -1 (linear), 0,1,4,5 */
>> - __u32 layout; /* only for raid5 currently */
>> + __u32 layout; /* use for raid5, raid6, raid10, and raid0 */
>
> s/use/used/
Fixed.
>
>> __u64 size; /* used size of component devices, in 512byte sectors */
>>
>> __u32 chunksize; /* in 512byte sectors */
>> @@ -144,6 +144,7 @@ struct misc_dev_info {
>> #define MD_FEATURE_JOURNAL 512 /* support write journal */
>> #define MD_FEATURE_PPL 1024 /* support PPL */
>> #define MD_FEATURE_MUTLIPLE_PPLS 2048 /* support for multiple PPLs */
>> +#define MD_FEATURE_RAID0_LAYOUT 4096 /* layout is meaninful in RAID0 */
>
> s/meaninful/meaningful/
Fixed.
>
> Those minor fixes aside, the code worked well for me. I was able to
> create a new multi-zone array w/ and w/o an explicit layout, and was
> able to change the layout of an existing array on assemble. I verified
> I could view the layout w/ -E, and that it persisted across reboots.
>
> Tested-by: dann frazier <dann.frazier@xxxxxxxxxxxxx>
Thanks for your excellent review and testing. I'll resend shortly.
Thanks,
NeilBrown
Attachment:
signature.asc
Description: PGP signature
