[libvirt] [PATCH v2 1/4] virInterface: Expose link state & speed

[Daniel P. Berrange]

On Wed, Jun 11, 2014 at 01:04:02PM +0300, Laine Stump wrote:
> On 06/11/2014 12:28 PM, Daniel P. Berrange wrote:
> > On Wed, Jun 11, 2014 at 11:19:14AM +0200, Michal Privoznik wrote:
> >> 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 at 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.
> > Apps must always be able to round-trip XML. ie they should be able to
> > do DumpXML and then feed that back into DefineXML and have no functional
> > change. Thus we must not reject <link/> with an error when parsing, we
> > must ignore it or honour it as appropriate for the desired semantics.
> 
> I agree that <link> should just be ignored when defining a domain. But
> as a general response to that last paragraph, there are cases where
> doing a virDomainGetXMLDesc() on an active domain (without the INACTIVE
> flag), followed by a virDomainDefine using that output, will *not*
> produce the same domain. The example at the front of my mind is what
> happens with an <interface type='network'>, as related in my RFC email
> yesterday:
> 
>   https://www.redhat.com/archives/libvir-list/2014-June/msg00416.html
> 
> but a more general problem is that if you run virDomainGetXMLDesc() with
> no INACTIVE flag, you will miss any previous edits to the domain
> definition since the last time it was started. This had actually been
> the cause of a long-running bug with "virsh net-edit" as a matter of fact.
> 
> I had thought that what was required was the ability to roundtrip the
> *inactive* xml back into a define, not the status XML. If it's really a
> requirement that we be able to feed back the output of the status XML to
> get the exact same domain, then I think I've made a big mistake :-/

Yes, you're right actually. I should have clarified I meant inactive
XML.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|