Re: [libvirt] [PATCH v2 1/4] virInterface: Expose link state & speed
On 11.06.2014 03:13, John Ferlan wrote:
On 06/05/2014 11:39 AM, Michal Privoznik wrote:
> Currently it is not possible to determine the speed of an interface
> and whether a link is actually detected from the API. Orchestrating
> platforms want to be able to determine when the link has failed and
> where multiple speeds may be available which one the interface is
> actually connected at. This commit introduces an extension to our
> interface XML (without implementation to interface driver backends):
>
> <interface type='ethernet' name='eth0'>
> <start mode='none'/>
> <mac address='aa:bb:cc:dd:ee:ff'/>
> <link speed='1000' state='up'/>
> <mtu size='1492'/>
> ...
> </interface>
>
> Where @speed is negotiated link speed in Mbits per second, and state
> is the current NIC state (can be one of the following: "unknown",
> "notpresent", "down",
"lowerlayerdown","testing", "dormant", "up").
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> docs/schemas/basictypes.rng | 25 ++++++++++
> docs/schemas/interface.rng | 1 +
> src/conf/device_conf.c | 62 +++++++++++++++++++++++++
> src/conf/device_conf.h | 27 ++++++++++-
> src/conf/interface_conf.c | 11 ++++-
> src/conf/interface_conf.h | 2 +
> src/libvirt_private.syms | 4 ++
> tests/interfaceschemadata/bridge-no-address.xml | 1 +
> tests/interfaceschemadata/bridge.xml | 1 +
> tests/interfaceschemadata/ethernet-dhcp.xml | 1 +
> 10 files changed, 133 insertions(+), 2 deletions(-)
>
Already been ACK'd, but I will point out the interface.html.in hasn't
been updated - later you update the nodedev.html.in file - so probably
reasonable to do so again. While at it the consistency of using "Mbits"
vs. "Mb" in the comment in device_conf.h
The first issue has simple explanation - there's no interface.html.in
yet. Yes we lack the virInterface XML documentation. The second one I'm
fixing prior to push.
Just so I understand - the reality is regardless of what the XML is on
input - the code will still check/get/set the link state/speed - so this
is mostly an output thing.
Yes. So far this is only for getting the link speed & state. Which
brings up an interesting question. With recent issue with QoS on
bridgeless networks on mind - should we make virInterface XML parsing
actually reject any <link/> since it's output element only? One could
argue that we usually ignore unknown elements, but then <link/> is not
unknown element anymore. One way or another, it can be done in a
separate patch.
It's not like if on input someone had "down" for the state that we'd
attempt to set the link down... or if the speed on input was 500, but
the file had 1000 - there's no changing of the file.
John
Michal