On 13.06.2014 09:31, Laine Stump wrote:
On 06/13/2014 10:10 AM, Martin Kletzander wrote:
On Fri, Jun 13, 2014 at 08:46:59AM +0200, Michal Privoznik wrote:
The kernel's more broken than one would think. Various drivers report
various (usually spurious) values if the interface is down. While on
some we experience -EINVAL when read()-ing the speed sysfs file, with
other drivers we might get anything from 0 to UINT_MAX. If that's the
case it's better to not report link speed. Well, the interface is down
anyway.
Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
src/util/virnetdev.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 6f3a202..80ef572 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1843,7 +1843,7 @@ virNetDevGetLinkInfo(const char *ifname,
char *buf = NULL;
char *tmp;
int tmp_state;
- unsigned int tmp_speed;
+ unsigned int tmp_speed; /* virInterfaceState */
You probably wanted to put this comment next to the line with
tmp_state and not tmp_speed.
if (virNetDevSysfsFile(&path, ifname, "operstate") < 0)
goto cleanup;
@@ -1875,6 +1875,16 @@ virNetDevGetLinkInfo(const char *ifname,
lnk->state = tmp_state;
+ /* Shortcut to avoid some kernel issues. If link is down (and
possibly in
+ * other states too) several drivers report several values.
While igb
+ * reports 65535, realtek goes with 10. To avoid muddying XML
with insane
+ * values, don't report link speed */
+ if (lnk->state == VIR_INTERFACE_STATE_DOWN) {
Also for VIR_INTERFACE_LOWER_LAYER_DOWN (verified by looking at the
speed reported by a macvtap device when its physdev is down). And I'm
not sure how to get an interface into "NOT_PRESENT" or "DORMANT" state,
but I would imagine that the speed should be 0 in those cases too.
If you use a wireless network that requires you to type in password each
time you sign in, the wlan is in dormant state from the time it
associates with an AP till you type in password and WiFi lets you in.
ACK with LOWER_LAYER_DOWN added (I won't insist on the others
until/unless I see experimental evidence that they need it).
Okay.
BTW, thinking more about bridge devices - maybe they should be given
state "up" if the device has been ifup'ed. In other words, in their case
you could call the functional equivalent of if_is_active() in netcf
(which does an SIOCGIFFLAGS ioctl and checks for the IFF_UP flag). (in
any case, bridges should probably just always report a speed of 0).
I'm trying to keep the state libvirt reports in sync with state that the
kernel reports.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list