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@xxxxxxxxxx> > >>>> --- > >>>> 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list