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