CP-12093: Add fcoe_supported param to PIF_metrics

[sharady]

No description provided.

[robhoes]

The code that checks whether fcoe is supported should be in the monitor loop of xcp-networkd (https://github.com/xapi-project/xcp-networkd/blob/master/networkd/network_monitor_thread.ml#L127), rather than in xapi's Monitor_master.

However, in either case, 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. That is, 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 it is 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; the latter here: https://github.com/xapi-project/xen-api/blob/master/ocaml/xapi/xapi_pif.ml#L69.

The above also implies that the field should be on the PIF class directly, rather than the PIF_metrics class.

[robhoes]

Perhaps this deserves a more general PIF.capabilities field where we can put various (offload) capabilities of the NIC.

[robhoes]

Note that half of this review was actually a design review rather than a code review... A design page on xapi-project.github.io would be the right place for such a design discussion.

[sharady]

Thanks Rob, I will update the xapi-project.github.io for toolstack design.

[robhoes]

I'm not sure I understand this bit. I was expecting something like the following, at the end of the function:

maybe_update_database "capabilities"
        (Db.PIF.get_capabilities)
        (Db.PIF.set_capabilities)
        (fun () -> Net.Interface.get_capabilities dbg ~name:device)
        (String.concat "; ")

There is no need to do anything "fcoe" specific.

[robhoes]

Also, please squash the patches.

[sharady]

Thanks @robhoes , for the review.
I have squashed the commits.