Design review: FCoE capable NICs

[robhoes]

@sharady

I had already added some comments to the code review at xapi-project/xen-api#2232, which still apply to the current design. I'll paraphrase:

When adding the is_fcoe_supported call into the metrics/monitor loop, the command would be run every 5 seconds, while I'd expect the FCoE capabilities of a NIC to not change that often, or even never. I assume that the field means that the field refers to the inherent capabilities of the NIC, rather than whether FCoE is currently "on" (it is not a status field).

Therefore, it would be sufficient to handle fcoe_supported in the same way as we handle the MAC address of a PIF: we set the field when the PIF is created, and update it each time xapi starts (in case the hardware was changed/replaced when xapi wasn't running). Code for the former can go around here: https://github.com/xapi-project/xen-api/blob/master/ocaml/xapi/xapi_pif.ml#L327 for PIF.introduce/scan; the latter here: https://github.com/xapi-project/xen-api/blob/master/ocaml/xapi/xapi_pif.ml#L69 which is called by the dbsync startup code. There is no need to do anything on PIF.plug (Nm.bring_pif_up).

The above also implies that the field should be on the PIF class directly, rather than the PIF_metrics class. Perhaps this deserves a more general PIF.capabilities field where we can put various (offload) capabilities of the NIC.

[robhoes]

@sharady Feedback on v2:

• I'd prefer to make PIF.capabilities of type string set (rather than string map). Then define the capability "fcoe" (I'd prefer all lowercase), which can be present or absent. This avoids having to decide whether to use values such as "supported"/"unsupported" or "true"/"false" etc.
• The interface to xcp-networkd should be made in terms of capabilities as well: Interface.get_capabilities.
• "PIF.capabilties" -> "PIF.capabilities"
• refresh_all is not called in PIF.scan (introduce_internal is). It is called from dbsync as part of xapi's startup sequence (see dbsync_slave.ml).
• The bit about the CLI should be changed to "capabilities".
• Cosmetic: some of the bullet lists are not displaying correctly. Adding a newline above them would probably fix that.