Hi Ryan,
I am the author of the PR (https://github.com/canonical/subiquity/pull/929) as 
well as the originating bug report 
(https://bugs.launchpad.net/subiquity/+bug/1880023).
Thanks for considering the changes. A few comments from my side:
I think this change is first of all a fix for a bug. The bug is that the code 
and the comment above state these values are "minimum number of devices for a 
given raid level". The original numbers are incorrect in this regard (see 
https://github.com/canonical/subiquity/pull/929 for pointers to the relevant 
information sources).

Also there are valid use-cases for both reductions. For (linux) RAID10 it is 
obviously single stream read speed and for RAID5 the main reason is 
expandability.

Regarding your RAID5 comment: a RAID5 with two components is not missing 
parity. The parity is just coincidentally the data itself which means the data 
on the device is arranged (parity distribution on equal data will not have an 
effect) like a RAID1. Nevertheless the metadata is RAID5 and also the upgrade 
path using "grow" is more obvious (RAID5 two component to RAID5 three 
component). Of course it is possible to do the intermediate step (RAID1 two 
components to RAID5 two components) before.

As a side-note, "mdadm" complains if the creation is missing some devices and 
it is not explicitly told "missing", so I am generally wondering why it is 
necessary to repeat the check in the installer (due to missing dry-run feature 
of mdadm?).

Regarding the layout support, I think this is a much appreciated feature but it 
is not a dependency to fix the current issue.
Since we are at the layout topic: I am not sure if there is much interest for 
the layout feature beyond RAID10. Thus it might also be an option to have 
two(/three) different RAID10 modes like RAID10near and RAID10far. The main 
question then would be if it is safe to assume the number of copies to be 2 
(from mdadm man page: "2 is normal, 3 can be useful").

BR
-- 
https://code.launchpad.net/~mwhudson/curtin/+git/curtin/+merge/400931
Your team curtin developers is requested to review the proposed merge of 
~mwhudson/curtin:raid-min-levels into curtin:master.

-- 
Mailing list: https://launchpad.net/~curtin-dev
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~curtin-dev
More help   : https://help.launchpad.net/ListHelp

Reply via email to