[libvirt] [PATCH 08/12] interface_backend_netcf: Implement link speed & state

[Laine Stump]

On 05/29/2014 11:32 AM, Michal Privoznik wrote:
> While the previous commit was pretty straightforward, things are
> different with netcf as it doesn't exposed the bits we need yet.

Rather than tacking these onto the XML returned from netcf after the
fact, we should instead modify netcf to directly provide the desired
information in the XML returned from ncf_if_xml_state(). I can help with
that.

Also, note that the interface XML only returns items that can be
configured when the VIR_INTERFACE_XML_INACTIVE flag is set, or when the
interface isn't active. If these values are read-only, I think they
should only be present when the interface is active and the INACTIVE
flag isn't set (i.e. they are part of the interface status rather than
config).

Of course netcf has a separate API (ncf_if_status) that checks the
interface IFF_UP and IFF_RUNNING flags, and uses that to determine what
is returned from libvirt's virInterfaceIsActive() API. We should
probably try to at least be sure that what is returned from that is
enough differentiated in documentation from what is returned in this new
<state> element that they aren't confused with each other (if they are
in fact different in practice).

And that brings up another possible issue (depending on exactly how
"<link state='up'> meshes with IFF_UP|IFF_RUNNING) - within the current
virInterfaceGetXMLDesc() paradigm of "interface *status* is only
returned when an interface is 'active'" (which was directly copied from
libvirt's behavior in the case of domains and networks), 1) it will
never be possible to learn about the state of the interface when the
interface is inactive (and if the cable is unplugged, it is considered
inactive), and 2) it will then be a bit pointless to report the
active/inactive status of the interface in the XML since it will by
definition always report "active" (state='up') if the element is present
at all. In order to make it useful, we'll need to modify the semantics
of virInterfaceGetXMLDesc() in some way such that it is always possible
to get the current status even when the interface is down; for example,
possibly we could add a VIR_INTERFACE_XML_STATUS flag that would
indicate to always return the status of the interface, even when it is
inactive.

> However, we can work around it by fetching the info we need from
> SYSFS.
>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/interface/interface_backend_netcf.c | 99 +++++++++++++++++++++++++++++++++
>  1 file changed, 99 insertions(+)
>
> diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c
> index 1b9ace5..40181ef 100644
> --- a/src/interface/interface_backend_netcf.c
> +++ b/src/interface/interface_backend_netcf.c
> @@ -33,6 +33,7 @@
>  #include "virlog.h"
>  #include "virstring.h"
>  #include "viraccessapicheck.h"
> +#include "virfile.h"
>  
>  #define VIR_FROM_THIS VIR_FROM_INTERFACE
>  
> @@ -240,6 +241,96 @@ static struct netcf_if *interfaceDriverGetNetcfIF(struct netcf *ncf, virInterfac
>      return iface;
>  }
>  
> +/* Okay, the following two doesn't really belong here as they dump the info
> + * from SYSFS rather than netcf. But netcf is not yet exposing the info we
> + * need, so what. */
> +#define SYSFS_PREFIX "/sys/class/net/"
> +
> +static int
> +interfaceGetState(const char *name,
> +                  virInterfaceState *state)
> +{
> +    int ret = -1;
> +    char *path = NULL;
> +    char *buf = NULL;
> +    char *tmp;
> +
> +    if (virAsprintf(&path, SYSFS_PREFIX "%s/operstate", name) < 0)
> +        goto cleanup;
> +
> +    if (virFileReadAll(path, 1024, &buf) < 0) {
> +        virReportSystemError(errno,
> +                             _("unable to read: %s"),
> +                             path);
> +        goto cleanup;
> +    }
> +
> +    if (!(tmp = strchr(buf, '\n'))) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Unable to parse: %s"),
> +                       buf);
> +        goto cleanup;
> +    }
> +
> +    *tmp = '\0';
> +
> +    /* We shouldn't allow 0 here, because
> +     * virInterfaceState enum starts from 1. */
> +    if ((*state = virInterfaceStateTypeFromString(buf)) <= 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Unable to parse: %s"),
> +                       buf);
> +        goto cleanup;
> +    }
> +
> +    ret = 0;
> + cleanup:
> +    VIR_FREE(buf);
> +    VIR_FREE(path);
> +    return ret;
> +}
> +
> +static int
> +interfaceGetSpeed(const char *name,
> +                  unsigned long *speed)
> +{
> +    int ret = -1;
> +    char *path = NULL;
> +    char *buf = NULL;
> +    char *tmp;
> +
> +    if (virAsprintf(&path, SYSFS_PREFIX "%s/speed", name) < 0)
> +        goto cleanup;
> +
> +    if (virFileReadAll(path, 1024, &buf) < 0) {
> +        /* Some devices doesn't report speed, in which case we get EINVAL */
> +        if (errno == EINVAL) {
> +            ret = 0;
> +            goto cleanup;
> +        }
> +        virReportSystemError(errno,
> +                             _("unable to read: %s"),
> +                             path);
> +        goto cleanup;
> +    }
> +
> +    if (virStrToLong_ul(buf, &tmp, 10, speed) < 0 ||
> +        *tmp != '\n') {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Unable to parse: %s"),
> +                       buf);
> +        goto cleanup;
> +    }
> +
> +    ret = 1;
> + cleanup:
> +    VIR_FREE(buf);
> +    VIR_FREE(path);
> +    return ret;
> +}
> +
> +/* Ond of sysfs hacks */
> +
>  static int
>  netcfInterfaceObjIsActive(struct netcf_if *iface,
>                            bool *active)
> @@ -840,6 +931,14 @@ static char *netcfInterfaceGetXMLDesc(virInterfacePtr ifinfo,
>      if (virInterfaceGetXMLDescEnsureACL(ifinfo->conn, ifacedef) < 0)
>          goto cleanup;
>  
> +    if (interfaceGetState(ifacedef->name, &ifacedef->state) < 0)
> +        goto cleanup;
> +
> +    /* querying inteface speed makes sense only sometimes */
> +    if (ifacedef->state == VIR_INTERFACE_STATE_UP &&
> +        interfaceGetSpeed(ifacedef->name, &ifacedef->speed) < 0)
> +        goto cleanup;
> +
>      ret = virInterfaceDefFormat(ifacedef);
>      if (!ret) {
>          /* error was already reported */