On 11/19/2015 04:45 PM, 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 succesful 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].
> The devid is taken from domain config which is accessible in
> libxlDomainStartCallback(). 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.
>
> 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@xxxxxxxxxx>
> ---
> Changes since v3:
> - Do not unlock vm if libxlDomainObjEndJob() returns false
> - Set vm->def->net[i]->ifname on DomainStartCallback instead of
> DomainStart.
> - Change commit message reflecting the changes on the previous
> item and mention correct interface names when doing domiflist.
>
> Changes since v2:
> - Clear ifname if it's autogenerated, since otherwise will persist
> on successive domain starts. Change commit message reflecting this
> change.
>
> Changes since v1:
> - Fill <virDomainNetDef>.ifname after domain start with generated
> name from libxl based on domain id and devid returned by libxl.
> After that path validation don interfaceStats is enterily based
> on ifname pretty much like the other drivers.
> - Modify commit message reflecting the changes mentioned in
> the previous item.
> - Bump version to 1.2.22
> ---
> src/libxl/libxl_domain.c | 27 +++++++++++++++++++++++++++
> src/libxl/libxl_driver.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 73 insertions(+)
>
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index f0746f8..5533677 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -733,6 +733,17 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver,
> }
> }
>
> + if ((vm->def->nnets)) {
> + ssize_t i;
> +
> + for (i = 0; i < vm->def->nnets; i++) {
> + virDomainNetDefPtr net = vm->def->nets[i];
> +
> + if (STRPREFIX(net->ifname, "vif"))
> + 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);
> @@ -862,6 +873,8 @@ static void
> libxlDomainStartCallback(libxl_ctx *ctx, libxl_event *ev, void *for_callback)
> {
> virDomainObjPtr vm = for_callback;
> + libxlDomainObjPrivatePtr priv = vm->privateData;
> + libxl_domain_config *d_config = priv->config;
> size_t i;
>
> virObjectLock(vm);
> @@ -888,6 +901,20 @@ libxlDomainStartCallback(libxl_ctx *ctx, libxl_event *ev, void *for_callback)
> VIR_FREE(console);
> }
> }
> +
> + for (i = 0; i < vm->def->nnets; i++) {
> + virDomainNetDefPtr net = vm->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;
> +
> + if (virAsprintf(&net->ifname, "vif%d.%d%s",
> + ev->domid, x_nic->devid, suffix) < 0)
> + continue;
> + }
Is it possible to get the interfaces with libxl_device_nic_list(), and use those
to generate ifname in the corresponding virDomainNetDef? According to the libxl
docs, it is permitted to call libxl from within the callback.
When I first read this patch, I thought this hunk was in the
libxlDomainInterfaceStats() function below. If that was the case, you can
probably see my concern that vm->def->nets could have grown or shrunk over time
while d_config->nics remained static. Such use of the potentially stale
libxl_domain_config object is why I'm now questioning whether we should add it
to the libxlDomainObjPrivate struct. Sorry for not realizing this when making
the suggestion :-(.
Regards,
Jim
> virObjectUnlock(vm);
> libxl_event_free(ctx, ev);
> }
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index d77a0e4..fe9c357 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
>
> @@ -4643,6 +4644,50 @@ libxlDomainIsUpdated(virDomainPtr dom)
> }
>
> static int
> +libxlDomainInterfaceStats(virDomainPtr dom,
> + const char *path,
> + virDomainInterfaceStatsPtr stats)
> +{
> + libxlDriverPrivatePtr driver = dom->conn->privateData;
> + virDomainObjPtr vm;
> + ssize_t i;
> + int ret = -1;
> +
> + if (!(vm = libxlDomObjFromDomain(dom)))
> + goto cleanup;
> +
> + if (virDomainInterfaceStatsEnsureACL(dom->conn, vm->def) < 0)
> + goto cleanup;
> +
> + if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_QUERY) < 0)
> + goto cleanup;
> +
> + if (!virDomainObjIsActive(vm)) {
> + virReportError(VIR_ERR_OPERATION_INVALID,
> + "%s", _("domain is not running"));
> + goto endjob;
> + }
> +
> + /* Check the path is one of the domain's network interfaces. */
> + for (i = 0; i < vm->def->nnets; i++) {
> + if (vm->def->nets[i]->ifname &&
> + STREQ(vm->def->nets[i]->ifname, path)) {
> + ret = virNetInterfaceStats(path, stats);
> + break;
> + }
> + }
> +
> + endjob:
> + if (!libxlDomainObjEndJob(driver, vm))
> + vm = NULL;
> +
> + cleanup:
> + if (vm)
> + virObjectUnlock(vm);
> + return ret;
> +}
> +
> +static int
> libxlDomainGetTotalCPUStats(libxlDriverPrivatePtr driver,
> virDomainObjPtr vm,
> virTypedParameterPtr params,
> @@ -5421,6 +5466,7 @@ static virHypervisorDriver libxlHypervisorDriver = {
> #endif
> .nodeGetFreeMemory = libxlNodeGetFreeMemory, /* 0.9.0 */
> .nodeGetCellsFreeMemory = libxlNodeGetCellsFreeMemory, /* 1.1.1 */
> + .domainInterfaceStats = libxlDomainInterfaceStats, /* 1.2.22 */
> .domainMemoryStats = libxlDomainMemoryStats, /* 1.2.22 */
> .domainGetCPUStats = libxlDomainGetCPUStats, /* 1.2.22 */
> .connectDomainEventRegister = libxlConnectDomainEventRegister, /* 0.9.0 */
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list