Comment 16 for bug 1893661

On 9/2/20 4:51 PM, Ryan Harper wrote:
>> I think your suggestion is a good YAML scheme. I think size_kb:
>> should be optional to fill the whole array with one volume if
>> it's omitted.
>
> Well, it's not that simple. What do we do if it's omitted and
> the config includes multiple volumes from the same container?
>
> Curtin config is typically constructed from an "Oracle"; either
> MAAS or Subiquity probe storage and then provide complete
> configuration from user-input.
>
> So if either added support for VROC, they would know in advance
> how many volumes per container and could specify the exact
> size.
>
> I believe we want to require size_kb if metadata == 'imsm'
>
Well, I would not complicate the most common case (at least for us).
If some external tool provides the size to all arrays, then OK. But if a
hand-constructed yaml doesn't, then it should go with that.

>
>> The number of devices can be looked up by pairing
>> 'container' with 'id'.
>
> Yes, I put the id anchor there so that the volumes created with
> in a container will be able to lookup the correct value rather
> than having to repeat.
>
>
>> Maybe it would be possible to abstract out the container, like:
>> - type: raid
>
> Everything needs and id.
>
> id: raid_container_0
>
>> metadata: imsm
>> container: /dev/md/imsm0
>
> This isn't needed, IIUC, metadata=imsm implies a container
> raid, no?
>
Yeah, container: is now in the members.

>> name: imsm0
>
> name: /dev/md/imsm0
>
>> devices:
>> - /dev/nvme0n1p1
>> - /dev/nvme1n1p1
>>
>> - type: raid
>> devices:
>> - /dev/md/imsm0 # but need to get the number of real devices for -n
>
> Yes, this is nice, and what we will do instead is:
>
> devices:
> - raid_container_0
>
I did
   container: raid_container_0
Otherwise it wouldn't obvious that it's a backreference.

>> name: mirror0
>> level: 1
>
>
>> I suggest to add a level: container to the top-level, as it would
>> imply to use the -e switch to mdadm, and also would be consistent
>> to the query output.
>>
>> - type: raid
>> id: disk_raid_container0
>> level: container
>> metadata: imsm
>
> I think we can skip that if it's true that imsm is always a
> container.
>
True. But then level would be undefined. Allowing level as None when
metadata=imsm - not sure if it's simpler.

>> name: /dev/md/imsm0
>> devices:
>> - /dev/nvme0n1p1
>> - /dev/nvme1n1p1
>
>
>> Another buglet: metadata is not passed to mdadm_create() in
>> raid_handler()
>
> Good catch! And in that case we can always pass --metadata=
> to mdadm, if metadata is not provided, we use the default.
>
As it wasn't reported, I think nobody used it :)