On 04.06.2014 23:40, Daniel P. Berrange wrote:
On Tue, Jun 03, 2014 at 02:22:10PM +0200, Michal Privoznik wrote:
In previous commit the interface XML is prepared for exporting
information on NIC link speed and state. This commit implement
actual logic for getting such info and writing it into XML.
Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
Notes:
For the kernel issue I've posted patch here:
http://patchwork.ozlabs.org/patch/354928/
But it wasn't accepted as developers are afraid of breaking backward
compatibility. Which is weird as in 2.6.X the kernel was reporting -1
instead of unsigned -1.
src/interface/interface_backend_udev.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c
index c5353ea..80ba93e 100644
--- a/src/interface/interface_backend_udev.c
+++ b/src/interface/interface_backend_udev.c
@@ -1036,6 +1036,8 @@ udevGetIfaceDef(struct udev *udev, const char *name)
const char *mtu_str;
char *vlan_parent_dev = NULL;
const char *devtype;
+ const char *operstate;
+ const char *link_speed;
/* Allocate our interface definition structure */
if (VIR_ALLOC(ifacedef) < 0)
@@ -1059,6 +1061,31 @@ udevGetIfaceDef(struct udev *udev, const char *name)
udev_device_get_sysattr_value(dev, "address")) < 0)
goto error;
+ operstate = udev_device_get_sysattr_value(dev, "operstate");
+ if ((ifacedef->lnk.state = virInterfaceStateTypeFromString(operstate)) <= 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unable to parse operstate: %s"),
+ operstate);
+ goto error;
+ }
+
+ link_speed = udev_device_get_sysattr_value(dev, "speed");
+ if (link_speed) {
+ if (virStrToLong_ul(link_speed, NULL, 10, &ifacedef->lnk.speed) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unable to parse speed: %s"),
+ link_speed);
+ goto error;
+ }
+
+ /* Workaround broken kernel API. If the link is unplugged then
+ * depending on the NIC driver, link speed can be reported as -1.
+ * However, the value is printed out as unsigned integer instead of
+ * signed one. Terrifying but true. */
+ if ((int) ifacedef->lnk.speed == -1)
+ ifacedef->lnk.speed = 0;
+ }
This block of code is duplicated in two other places in your series.
I think it'd be worth having a function in virnetdev
virNetDevGetLinkInfo(const char *ifname, int *speed, int *state)
Yes and no. One place that adds these lines is in netcf backend. This is
probably not going to be applied anyway since netcf is gonna expose the
info itself. That leaves us with the second place, which is udev backend
to node_device driver. So the function can be:
udevNetDevGetLinkInfo(struct udev_device *device,
int *speed, int *state);
Okay, I'll rework and repost. I'm not sure if it makes any sense to push
the 1/4 now. Does it?
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list