On 04.03.2016 03:38, Mathieu Trudel-Lapierre wrote: > The work is ready for partman-iscsi here: > http://anonscm.debian.org/cgit/d-i/partman-iscsi.git/log/?h=people/cyphermox/initiatorname. > Thoughts? From the PoV of an open-iscsi co-maintainer I'd like to note that I like the general approach (and the prompt should be critical IMHO), but there are some details where I'd like to see some changes: 1. I wouldn't name the shell function iscsi_start: it's not a good idea to distinguish the script from the shell function by just underscore vs. hyphen. I'd rather go for something like do_iscsi_start for the shell function, to make the distinction clear. 2. I don't like the formulation of the debconf template: > iSCSI targets usually require that the initiatorName from the > initiator is registered on the target side or the discovery/login > steps will fail. If that's the case, please enter the initiatorName > to be used in this installation, or leave it blank to use a random > generated initiatorName. s/usually/may/. While this may be quite common, 'usually' will encourage administrators to always enter something there, even if they don't need it. I don't think that's a particularly good idea - because people who do need it will know about it, but people who don't need it should not be made to think about this. Especially since the name must be unique and they run into the risk of choosing the same name for different systems if they aren't that well informed about this aspect of iSCSI. I'd put the instructions to leave it blank in an extra paragraph at the end, just to make sure that this information is displayed more prominently. Also: s/initiatorName/initiator name/ everywhere in prose. > iSCSI initiatorName is usually of the form: > iqn.1993-08.org.debian:01:deadbeef I don't like the 'deadbeef' here, I'd rather replace that with XXXXXXXX and explicit instructions. So my suggestion would be: | iSCSI targets may require that the initiator name is registered on the | target side or the discovery and/or login steps will fail. If that's | the case, please enter the initiator name to be used in this | installation. | | iSCSI initiator names are usually of the form | iqn.1993.08.org.debian:01:XXXXXXXX, where XXXXXXXX should be replaced | by a unique string. | | To have a random initiator name generated for you, leave this field | blank. 3. Should there be a method of changing the initiator name? In most parts of d-i you can go back and change things - but in this case, once the prompt was answered, you'll have to restart the installer to change it (or fiddle manually on a console). At least as long as there are no active sessions, maybe there should be a way to stop iscsid, change the name and start iscsid again. For example, if an admin mistypes, it would be very unfortunate for them to have to restart the entire installer once they notice that login doesn't work. (If that requires additional changes to the udeb package, I'd gladly help out there.) Regards, Christian
Attachment:
signature.asc
Description: OpenPGP digital signature