[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 3/3] libxl: implement virDomainInterfaceStats
On 11/20/2015 07:25 PM, Jim Fehlig wrote: > 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. Ah, I didn't know that routine in the libxl API. Then we don't need the config at all in libxlDomainObjPrivate. > 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 :-(. No problem! I think the config there might actually be redundant and with not much use after domain create as I pointed out in the Patch 1/3 comment. Regards, Joao > > 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 |