Dan, Dan Groves wrote: > I've posted a new version of the link ID management API document to: > > http://www.opensolaris.org/os/project/clearview/uv/link_id_management.pdf
This is looking good. A few comments: Section 2: * The daemon is called linkmgmtd, but the service should probably have a more friendly name or one that is not equal to the daemon's name (which is an implementation detail of the service). I don't really feel strongly about this, but there seems to be some precedent behind separating the service name from the names of the programs used to implement it. Section 2.2: * The table on page 3 still lists the link id as a property Section 2.3: * 2nd paragraph: The warning about it being a dangerous operation is a bit understated, I think. It's not just that administrators can't create links, but devices can't attach either, so opening links for devices that haven't attached (such as plumbing an IP interface over a device that hasn't been used since the system has booted) won't work either. I know this still implicitly constitutes "creating a link", but readers might not read it that way. * The sentence that starts with "This way, temporary ..." is a bit awkward. I know what you meant, but as written, one could interpret this as, "In the event of a system crash or reboot, temporary link information is restored". Section 3.1.1: * I'm a confused between the distinction between PHYS, ETHER, and WIFI. To me, both WIFI and ETHER are physical classes. I believe the code was recently modified to represent this relationship by getting rid of PHYS from the enum, and having PHYS simply be a macro consisting of the classes that represent physical classes. It's possible that this document simply needs to be updated to reflect what's been done with the implementation. Section 3.2.1: * If the mapping isn't persistent until a configuration is persisted using dladm_*_conf(), then what is the purpose of DLADM_OPT_PERSIST in dladm_create_datalink_id()? * Setting both DLADM_OPT_PERSIST and DLADM_OPT_TEMP is acceptable and does not result in DLADM_STATUS_BADARG in the implementation. Does the document reflect what will be true in the future for the implementation in this case, or does the document need to be changed to reflect the implementation? That's all, good stuff. -Seb
