Joao Martins wrote:
Introduce support for connectGetAllDomainStats call that
allow us to _all_ domain(s) statistics including network, block,
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(a)oracle.com>
---
src/libxl/libxl_driver.c | 267 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 267 insertions(+)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index fd952a3..01e97fd 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -5284,6 +5284,272 @@ 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);
+ virDomainStatsRecordPtr tmp;
+ libxl_dominfo d_info;
+ libxl_vcpuinfo *vcpuinfo = NULL;
+ char *path;
+ 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);
+ unsigned int vcpuflags = stats & VIR_DOMAIN_STATS_VCPU;
+ unsigned int netflags = stats & VIR_DOMAIN_STATS_INTERFACE;
+ unsigned int blkflags = stats & VIR_DOMAIN_STATS_BLOCK;
+
+ if (VIR_ALLOC(tmp) < 0)
+ return ret;
+
+ 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 (vcpuflags)
+ vcpuinfo = libxl_list_vcpu(cfg->ctx, dom->def->id, &maxcpu,
&hostcpus);
+
+ for (i = 0; i < dom->def->vcpus && vcpuflags; 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);
I found this a bit difficult to read. IMO, something like
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)
libxl_vcpuinfo_list_free(vcpuinfo, maxcpu);
}
is a little clearer. What do you think?
+
+ for (i = 0; i < dom->def->nnets && netflags; i++) {
+ struct _virDomainInterfaceStats netstats;
+
+ if (!virDomainObjIsActive(dom))
+ continue;
+
+ if (virAsprintf(&path, "vif%d.%ld", dom->def->id, i) <
0)
+ goto cleanup;
+
+ ret = virNetInterfaceStats(path, &netstats);
+
+ 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", path, i);
+ }
+
+ if (netflags)
+ LIBXL_RECORD_UINT(cleanup, "net.count", dom->def->nnets);
+
+ for (i = 0; i < dom->def->ndisks && blkflags; 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);
+ }
+
+ if (blkflags)
+ LIBXL_RECORD_UINT(cleanup, "block.count", dom->def->ndisks);
Same comment for VIR_DOMAIN_STATS_INTERFACE and VIR_DOMAIN_STATS_BLOCK.
+
+ if (!(tmp->dom = virGetDomain(conn, dom->def->name,
dom->def->uuid)))
+ goto cleanup;
+
+ *record = tmp;
+ return 0;
+
+ cleanup_vcpu:
+ if (vcpuinfo)
+ libxl_vcpuinfo_list_free(vcpuinfo, maxcpu);
+ cleanup:
+ if (path)
+ VIR_FREE(path);
'make syntax-check' complained here
src/libxl/libxl_driver.c: if (path)
VIR_FREE(path);
maint.mk: found useless "if" before "free" above
Also, it looks like you could declare path in a local block handling the
interface stats. E.g.
if (stats & VIR_DOMAIN_STATS_INTERFACE {
char *path = NULL;
...
VIR_FREE(path);
}
+ 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,
@@ -5882,6 +6148,7 @@ static virHypervisorDriver libxlHypervisorDriver = {
.domainInterfaceStats = libxlDomainInterfaceStats, /* 1.2.20 */
.domainMemoryStats = libxlDomainMemoryStats, /* 1.2.20 */
.domainGetCPUStats = libxlDomainGetCPUStats, /* 1.2.20 */
+ .connectGetAllDomainStats = libxlConnectGetAllDomainStats, /* 1.2.20 */
Now 1.2.21 due to my review delay :-/.
Regards,
Jim