Re: [libvirt] [PATCH 2/4] interface_backend_udev: Implement link speed & state

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(a)redhat.com&gt; > --- > > 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)