[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 6/8] libxl: implement virConnectGetAllDomainStats
On 11/18/2015 10:03 PM, Jim Fehlig wrote: > On 11/13/2015 06:14 AM, Joao Martins wrote: >> Introduce support for connectGetAllDomainStats call that >> allow us to _all_ domain(s) statistics including network, block, > > allows us to get > >> cpus and memory. Changes are rather mechanical and mostly >> take care of the format to export the data. >> >> Signed-off-by: Joao Martins <joao.m.martins@xxxxxxxxxx> >> --- >> Changes since v1: >> - Rework flags checking on libxlDomainGetStats >> for VIR_DOMAIN_STATS_{VCPU,INTERFACE,BLOCK} >> - Removed path since we are reusing <virDomainNetDef>.ifname >> - Init dominfo and dispose it on cleanup. >> - Fixed VIR_FREE issue that was reported with make syntax-check" >> - Bump version to 1.2.22 >> --- >> src/libxl/libxl_driver.c | 266 >> +++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 266 insertions(+) >> >> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c >> index ba1d67b..8db6536 100644 >> --- a/src/libxl/libxl_driver.c >> +++ b/src/libxl/libxl_driver.c >> @@ -5238,6 +5238,271 @@ libxlDomainMemoryStats(virDomainPtr dom, >> >> #undef LIBXL_SET_MEMSTAT >> >> +#define LIBXL_RECORD_UINT(error, key, value, ...) \ >> +do { \ >> + char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \ >> + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \ >> + key, ##__VA_ARGS__); \ >> + if (virTypedParamsAddUInt(&tmp->params, \ >> + &tmp->nparams, \ >> + &maxparams, \ >> + param_name, \ >> + value) < 0) \ >> + goto error; \ >> +} while (0) >> + >> +#define LIBXL_RECORD_LL(error, key, value, ...) \ >> +do { \ >> + char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \ >> + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \ >> + key, ##__VA_ARGS__); \ >> + if (virTypedParamsAddLLong(&tmp->params, \ >> + &tmp->nparams, \ >> + &maxparams, \ >> + param_name, \ >> + value) < 0) \ >> + goto error; \ >> +} while (0) >> + >> +#define LIBXL_RECORD_ULL(error, key, value, ...) \ >> +do { \ >> + char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \ >> + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \ >> + key, ##__VA_ARGS__); \ >> + if (virTypedParamsAddULLong(&tmp->params, \ >> + &tmp->nparams, \ >> + &maxparams, \ >> + param_name, \ >> + value) < 0) \ >> + goto error; \ >> +} while (0) >> + >> +#define LIBXL_RECORD_STR(error, key, value, ...) \ >> +do { \ >> + char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \ >> + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \ >> + key, ##__VA_ARGS__); \ >> + if (virTypedParamsAddString(&tmp->params, \ >> + &tmp->nparams, \ >> + &maxparams, \ >> + param_name, \ >> + value) < 0) \ >> + goto error; \ >> +} while (0) >> + >> +static int >> +libxlDomainGetStats(virConnectPtr conn, >> + virDomainObjPtr dom, >> + unsigned int stats, >> + virDomainStatsRecordPtr *record) >> +{ >> + libxlDriverPrivatePtr driver = conn->privateData; >> + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); > > cfg needs to be unref'ed when this function terminates. Sadly I noticed this > only after pushing some of the other patches with a similar flaw. > OK, will fix that. I saw and tested your fix series, and it appears both of them are Acked already. >> + virDomainStatsRecordPtr tmp; >> + libxl_dominfo d_info; >> + libxl_vcpuinfo *vcpuinfo = NULL; >> + int maxcpu, hostcpus; >> + unsigned long long mem, maxmem; >> + int maxparams = 0; >> + int ret = -1; >> + size_t i, state; >> + unsigned int domflags = stats & (VIR_DOMAIN_STATS_BALLOON | >> + VIR_DOMAIN_STATS_CPU_TOTAL); >> + >> + if (VIR_ALLOC(tmp) < 0) >> + return ret; >> + >> + libxl_dominfo_init(&d_info); >> + >> + mem = virDomainDefGetMemoryInitial(dom->def); >> + maxmem = virDomainDefGetMemoryActual(dom->def); >> + d_info.cpu_time = 0; >> + >> + if (domflags && virDomainObjIsActive(dom) && >> + !libxl_domain_info(cfg->ctx, &d_info, dom->def->id)) { >> + mem = d_info.current_memkb; >> + maxmem = d_info.max_memkb; >> + } >> + >> + if (stats & VIR_DOMAIN_STATS_STATE) { >> + LIBXL_RECORD_UINT(cleanup, "state.reason", dom->state.reason); >> + LIBXL_RECORD_UINT(cleanup, "state.state", dom->state.state); >> + } >> + >> + if (stats & VIR_DOMAIN_STATS_BALLOON) { >> + LIBXL_RECORD_ULL(cleanup, "balloon.current", mem); >> + LIBXL_RECORD_ULL(cleanup, "balloon.maximum", maxmem); >> + } >> + >> + if (stats & VIR_DOMAIN_STATS_CPU_TOTAL) >> + LIBXL_RECORD_ULL(cleanup, "cpu.time", d_info.cpu_time); >> + >> + if (stats & VIR_DOMAIN_STATS_VCPU) { >> + vcpuinfo = libxl_list_vcpu(cfg->ctx, dom->def->id, &maxcpu, >> &hostcpus); >> + >> + for (i = 0; i < dom->def->vcpus; i++) { >> + if (!vcpuinfo) >> + state = VIR_VCPU_OFFLINE; >> + else if (vcpuinfo[i].running) >> + state = VIR_VCPU_RUNNING; >> + else if (vcpuinfo[i].blocked) >> + state = VIR_VCPU_BLOCKED; >> + else >> + state = VIR_VCPU_OFFLINE; >> + >> + LIBXL_RECORD_UINT(cleanup_vcpu, "vcpu.%zu.state", state, i); >> + >> + /* vcputime is shown only if the VM is active */ >> + if (!virDomainObjIsActive(dom)) >> + continue; >> + >> + LIBXL_RECORD_ULL(cleanup_vcpu, "vcpu.%zu.time", >> vcpuinfo[i].vcpu_time, i); >> + } >> + >> + if (vcpuinfo) >> + libxl_vcpuinfo_list_free(vcpuinfo, maxcpu); >> + } >> + >> + if (stats & VIR_DOMAIN_STATS_INTERFACE) { >> + for (i = 0; i < dom->def->nnets; i++) { >> + virDomainNetDefPtr net = dom->def->nets[i]; >> + struct _virDomainInterfaceStats netstats; >> + >> + if (!virDomainObjIsActive(dom) || !net->ifname) >> + continue; >> + >> + if ((ret = virNetInterfaceStats(net->ifname, &netstats)) < 0) >> + continue; >> + >> + LIBXL_RECORD_ULL(cleanup, "net.%zu.rx.bytes", >> netstats.rx_bytes, i); >> + LIBXL_RECORD_ULL(cleanup, "net.%zu.rx.pkts", >> netstats.rx_packets, i); >> + LIBXL_RECORD_ULL(cleanup, "net.%zu.rx.drop", netstats.rx_drop, >> i); >> + LIBXL_RECORD_ULL(cleanup, "net.%zu.rx.errs", netstats.rx_errs, >> i); >> + LIBXL_RECORD_ULL(cleanup, "net.%zu.tx.bytes", >> netstats.tx_bytes, i); >> + LIBXL_RECORD_ULL(cleanup, "net.%zu.tx.pkts", >> netstats.tx_packets, i); >> + LIBXL_RECORD_ULL(cleanup, "net.%zu.tx.drop", netstats.tx_drop, >> i); >> + LIBXL_RECORD_ULL(cleanup, "net.%zu.tx.errs", netstats.tx_errs, >> i); >> + LIBXL_RECORD_STR(cleanup, "net.%zu.name", net->ifname, i); >> + } >> + >> + LIBXL_RECORD_UINT(cleanup, "net.count", dom->def->nnets); >> + } >> + >> + if (stats & VIR_DOMAIN_STATS_BLOCK) { >> + for (i = 0; i < dom->def->ndisks; i++) { >> + virDomainDiskDefPtr disk = dom->def->disks[i]; >> + struct _libxlBlockStats blkstats; >> + >> + if (!virDomainObjIsActive(dom)) >> + continue; >> + >> + memset(&blkstats, 0, sizeof(libxlBlockStats)); >> + if ((ret = libxlDomainBlockStatsGather(dom, disk->dst, >> &blkstats)) < 0) >> + continue; >> + >> + LIBXL_RECORD_LL(cleanup, "block.%zu.rd.reqs", blkstats.rd_req, >> i); >> + LIBXL_RECORD_LL(cleanup, "block.%zu.rd.bytes", >> blkstats.rd_bytes, i); >> + LIBXL_RECORD_LL(cleanup, "block.%zu.wr.reqs", blkstats.wr_req, >> i); >> + LIBXL_RECORD_LL(cleanup, "block.%zu.wr.bytes", >> blkstats.wr_bytes, i); >> + LIBXL_RECORD_LL(cleanup, "block.%zu.fl.reqs", blkstats.f_req, >> i); >> + >> + if (STREQ_NULLABLE(blkstats.backend, "vbd")) { >> + LIBXL_RECORD_LL(cleanup, "block.%zu.discard.reqs", >> blkstats.u.vbd.ds_req, i); >> + LIBXL_RECORD_LL(cleanup, "block.%zu.errs", >> blkstats.u.vbd.oo_req, i); >> + } >> + LIBXL_RECORD_STR(cleanup, "block.%zu.name", disk->dst, i); >> + LIBXL_RECORD_STR(cleanup, "block.%zu.path", disk->src->path, i); >> + } >> + >> + LIBXL_RECORD_UINT(cleanup, "block.count", dom->def->ndisks); >> + } >> + >> + if (!(tmp->dom = virGetDomain(conn, dom->def->name, dom->def->uuid))) >> + goto cleanup; >> + >> + *record = tmp; > > tmp = NULL; > ret = 0; > > then fall-though to cleanup? Otherwise looks like there are some leaks. > Ah, yes. Will need to do this too: if (tmp) virTypedParamsFree(tmp->params, tmp->nparams); And perhaps set vcpuinfo to NULL on (stats & VIR_DOMAIN_STATS_VCPU) after libxl_vcpuinfo_list_free() gets called so that I can just remove the "return 0" there. That way return path is clear and also with no leaks. Thanks! Joao > Regards, > Jim > >> + return 0; >> + >> + cleanup_vcpu: >> + if (vcpuinfo) >> + libxl_vcpuinfo_list_free(vcpuinfo, maxcpu); >> + cleanup: >> + libxl_dominfo_dispose(&d_info); >> + virTypedParamsFree(tmp->params, tmp->nparams); >> + VIR_FREE(tmp); >> + return ret; >> +} >> + >> +static int >> +libxlConnectGetAllDomainStats(virConnectPtr conn, >> + virDomainPtr *doms, >> + unsigned int ndoms, >> + unsigned int stats, >> + virDomainStatsRecordPtr **retStats, >> + unsigned int flags) >> +{ >> + libxlDriverPrivatePtr driver = conn->privateData; >> + virDomainObjPtr *vms = NULL; >> + virDomainObjPtr vm; >> + size_t nvms; >> + virDomainStatsRecordPtr *tmpstats = NULL; >> + int nstats = 0; >> + size_t i; >> + int ret = -1; >> + unsigned int lflags = flags & (VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE | >> + >> VIR_CONNECT_LIST_DOMAINS_FILTERS_PERSISTENT | >> + VIR_CONNECT_LIST_DOMAINS_FILTERS_STATE); >> + >> + virCheckFlags(VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE | >> + VIR_CONNECT_LIST_DOMAINS_FILTERS_PERSISTENT | >> + VIR_CONNECT_LIST_DOMAINS_FILTERS_STATE, -1); >> + >> + if (virConnectGetAllDomainStatsEnsureACL(conn) < 0) >> + return -1; >> + >> + if (ndoms) { >> + if (virDomainObjListConvert(driver->domains, conn, doms, ndoms, >> &vms, >> + &nvms, >> virConnectGetAllDomainStatsCheckACL, >> + lflags, true) < 0) >> + return -1; >> + } else { >> + if (virDomainObjListCollect(driver->domains, conn, &vms, &nvms, >> + virConnectGetAllDomainStatsCheckACL, >> + lflags) < 0) >> + return -1; >> + } >> + >> + if (VIR_ALLOC_N(tmpstats, nvms + 1) < 0) >> + return -1; >> + >> + for (i = 0; i < nvms; i++) { >> + virDomainStatsRecordPtr tmp = NULL; >> + vm = vms[i]; >> + >> + virObjectLock(vm); >> + >> + if (libxlDomainGetStats(conn, vm, stats, &tmp) < 0) { >> + virObjectUnlock(vm); >> + goto cleanup; >> + } >> + >> + if (tmp) >> + tmpstats[nstats++] = tmp; >> + >> + virObjectUnlock(vm); >> + } >> + >> + *retStats = tmpstats; >> + tmpstats = NULL; >> + >> + ret = nstats; >> + >> + cleanup: >> + virDomainStatsRecordListFree(tmpstats); >> + virObjectListFreeCount(vms, nvms); >> + return ret; >> +} >> + >> static int >> libxlConnectDomainEventRegisterAny(virConnectPtr conn, virDomainPtr dom, >> int eventID, >> virConnectDomainEventGenericCallback >> callback, >> @@ -5836,6 +6101,7 @@ static virHypervisorDriver libxlHypervisorDriver = { >> .domainInterfaceStats = libxlDomainInterfaceStats, /* 1.2.22 */ >> .domainMemoryStats = libxlDomainMemoryStats, /* 1.2.22 */ >> .domainGetCPUStats = libxlDomainGetCPUStats, /* 1.2.22 */ >> + .connectGetAllDomainStats = libxlConnectGetAllDomainStats, /* 1.2.22 */ >> .connectDomainEventRegister = libxlConnectDomainEventRegister, /* 0.9.0 >> */ >> .connectDomainEventDeregister = libxlConnectDomainEventDeregister, /* >> 0.9.0 */ >> .domainManagedSave = libxlDomainManagedSave, /* 0.9.2 */ > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |