[libvirt] [PATCH] libxl: implement virDomainInterfaceStats

[Joao Martins]

On 02/24/2016 09:42 PM, Jim Fehlig wrote:
> Joao Martins wrote:
>> Introduce support for domainInterfaceStats API call for querying
>> network interface statistics. Consequently it also enables the use of
>> `virsh domifstat <dom> <interface name>` command plus seeing the
>> interfaces names instead of "-" when doing `virsh domiflist <dom>`.
>>
>> After successful guest creation we fill the network interfaces names
>> based on domain, device id and append suffix if it's emulated in the
>> following form: vif<domid>.<devid>[-emu].  We extract the network
>> interfaces info from the libxl_domain_config object in
>> libxlDomainCreateIfaceNames() to generate ifname. On domain cleanup we
>> also clear ifname, in case it was set by libvirt (i.e.  being prefixed
>> with "vif"). We also skip these two steps in case the name of the
>> interface was manually inserted by the adminstrator. Since the
> 
> s/adminstrator/administrator/
> 
>> introdution of netprefix (commit a040ba9), ifnames with a registered
> 
> s/introdution/introduction/
> 
>> prefix will be freed on virDomain{Obj,Def}Format*, thus eliminating
>> the migration issues observed with the reverted commit d2e5538 whereas
>> source and destination would have the same ifname.
>>
>> For getting the interface statistics we resort to virNetInterfaceStats
>> and let libvirt handle the platform specific nits. Note that the
>> latter is not yet supported in FreeBSD.
>>
>> Signed-off-by: Joao Martins <joao.m.martins at oracle.com>
>> ---
>> Changes since d2e5538:
>>  - Check net->ifname beforehand, preventing a crash on the migration
>>  failure path.
>>  - Use LIBXL_GENERATED_PREFIX_XEN as opposed to hardcoding "vif"
>>  - Bump from 1.3.0 to 1.3.2
>>  - Change commit message to mention the migration issue and netprefix.
>> ---
>>  src/libxl/libxl_domain.c | 39 ++++++++++++++++++++++++++++++++++++
>>  src/libxl/libxl_driver.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 91 insertions(+)
>>
>> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
>> index 50f7eed..9d0da68 100644
>> --- a/src/libxl/libxl_domain.c
>> +++ b/src/libxl/libxl_domain.c
>> @@ -762,6 +762,18 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver,
>>          }
>>      }
>>  
>> +    if ((vm->def->nnets)) {
>> +        size_t i;
>> +
>> +        for (i = 0; i < vm->def->nnets; i++) {
>> +            virDomainNetDefPtr net = vm->def->nets[i];
>> +
>> +            if (net->ifname &&
>> +                STRPREFIX(net->ifname, LIBXL_GENERATED_PREFIX_XEN))
>> +                VIR_FREE(net->ifname);
>> +        }
>> +    }
>> +
>>      if (virAsprintf(&file, "%s/%s.xml", cfg->stateDir, vm->def->name) > 0) {
>>          if (unlink(file) < 0 && errno != ENOENT && errno != ENOTDIR)
>>              VIR_DEBUG("Failed to remove domain XML for %s", vm->def->name);
>> @@ -929,6 +941,32 @@ libxlConsoleCallback(libxl_ctx *ctx, libxl_event *ev, void *for_callback)
>>      libxl_event_free(ctx, ev);
>>  }
>>  
>> +/*
>> + * Create interface names for the network devices in parameter def.
>> + * Names are created with the pattern 'vif<domid>.<devid><suffix>'.
>> + * devid is extracted from the network devices in the d_config
>> + * parameter. User-provided interface names are skipped.
>> + */
>> +static void
>> +libxlDomainCreateIfaceNames(virDomainDefPtr def, libxl_domain_config *d_config)
>> +{
>> +    size_t i;
>> +
>> +    for (i = 0; i < def->nnets && i < d_config->num_nics; i++) {
>> +        virDomainNetDefPtr net = def->nets[i];
>> +        libxl_device_nic *x_nic = &d_config->nics[i];
>> +        const char *suffix =
>> +            x_nic->nictype != LIBXL_NIC_TYPE_VIF ? "-emu" : "";
>> +
>> +        if (net->ifname)
>> +            continue;
>> +
>> +        ignore_value(virAsprintf(&net->ifname,
>> +                                 LIBXL_GENERATED_PREFIX_XEN "%d.%d%s",
>> +                                 def->id, x_nic->devid, suffix));
>> +    }
>> +}
>> +
>>  
>>  /*
>>   * Start a domain through libxenlight.
>> @@ -1073,6 +1111,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
>>      if (libxl_evenable_domain_death(cfg->ctx, vm->def->id, 0, &priv->deathW))
>>          goto cleanup_dom;
>>  
>> +    libxlDomainCreateIfaceNames(vm->def, &d_config);
>>  
>>      if ((dom_xml = virDomainDefFormat(vm->def, cfg->caps, 0)) == NULL)
>>          goto cleanup_dom;
>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>> index 404016e..5479441 100644
>> --- a/src/libxl/libxl_driver.c
>> +++ b/src/libxl/libxl_driver.c
>> @@ -58,6 +58,7 @@
>>  #include "virhostdev.h"
>>  #include "network/bridge_driver.h"
>>  #include "locking/domain_lock.h"
>> +#include "virstats.h"
>>  
>>  #define VIR_FROM_THIS VIR_FROM_LIBXL
>>  
>> @@ -4662,6 +4663,56 @@ libxlDomainIsUpdated(virDomainPtr dom)
>>  }
>>  
>>  static int
>> +libxlDomainInterfaceStats(virDomainPtr dom,
>> +                          const char *path,
>> +                          virDomainInterfaceStatsPtr stats)
>> +{
>> +    libxlDriverPrivatePtr driver = dom->conn->privateData;
>> +    virDomainObjPtr vm;
>> +    ssize_t i;
> 
> s/ssize_t/size_t/
> 
> Otherwise ACK, again :-). Since this patch has been in, then out, followed by
> prerequisite work to get it in again, I think it deserves to make 1.3.2 even
> though we just hit freeze. I've fixed the nits and pushed it.
Cool, Thanks!

> 
> Please be on the lookout for any fallout like Coverity errors, which is
> unlikely, but just in case...
OK :)

Regards,
Joao

> 
> Regards,
> Jim
>