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 :)

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/1893661

Title:
  Support for Intel VROC (Virtual RAID On CPU)

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/curtin/+bug/1893661/+subscriptions

-- 
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

Reply via email to