[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 3/3] libxl: implement virDomainInterfaceStats
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 > */ _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |