On 03/05/2018 12:21 PM, Cédric Bosdonnat wrote:
LXC containers can also provide some statistics. Allow users to
fetch
them using the existing API.
I think you're fetching more than "some" ;-)!
---
src/lxc/lxc_driver.c | 203 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 203 insertions(+)
Please don't forget to add a docs/news.xml update for the new functionality
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index a16dbcc96..c357df927 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -80,6 +80,7 @@
#include "viraccessapichecklxc.h"
#include "virhostdev.h"
#include "netdev_bandwidth_conf.h"
+#include "domain_stats.h"
#define VIR_FROM_THIS VIR_FROM_LXC
@@ -5483,6 +5484,207 @@ lxcDomainHasManagedSaveImage(virDomainPtr dom, unsigned int
flags)
return ret;
}
2 blank lines for each...
+static int
+lxcDomainGetStatsState(virLXCDriverPtr driver ATTRIBUTE_UNUSED,
+ virDomainObjPtr dom,
+ virDomainStatsRecordPtr record,
+ int *maxparams)
+{
+ return virDomainStatsGetState(dom, record, maxparams);
+}
+
+static int
+lxcDomainGetStatsCpu(virLXCDriverPtr driver ATTRIBUTE_UNUSED,
+ virDomainObjPtr dom,
+ virDomainStatsRecordPtr record,
+ int *maxparams)
+{
+ virLXCDomainObjPrivatePtr priv = dom->privateData;
+ return virCgroupGetStatsCpu(priv->cgroup, record, maxparams);
+}
+
+static int
+lxcDomainGetStatsInterface(virLXCDriverPtr driver ATTRIBUTE_UNUSED,
+ virDomainObjPtr dom,
+ virDomainStatsRecordPtr record,
+ int *maxparams)
+{
+ return virDomainStatsGetInterface(dom, record, maxparams);
+}
+
+/* expects a LL, but typed parameter must be ULL */
+#define LXC_ADD_BLOCK_PARAM_LL(record, maxparams, name, value) \
+do { \
+ char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \
+ snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \
+ "block.cgroup.%s", name); \
+ if (virTypedParamsAddULLong(&(record)->params, \
+ &(record)->nparams, \
+ maxparams, \
+ param_name, \
+ value) < 0) \
+ goto cleanup; \
+} while (0)
I think it'll look cleaner if the do { ... } while gets 4 char indent.
Although I see you've essentially copied QEMU_ADD_BLOCK_PARAM_LL
+
+static int
+lxcDomainGetStatsBlock(virLXCDriverPtr driver,
+ virDomainObjPtr dom,
+ virDomainStatsRecordPtr record,
+ int *maxparams)
+{
+ long long rd_req, rd_bytes, wr_req, wr_bytes;
Here could be virDomainBlockStats stats = { 0 }; and of course
subsequent adjustments.
+
+ int ret = lxcDomainBlockStatsInternal(driver, dom, "",
+ &rd_bytes,
+ &wr_bytes,
+ &rd_req,
+ &wr_req);
+
+ LXC_ADD_BLOCK_PARAM_LL(record, maxparams,
+ "rd.reqs", rd_req);
+ LXC_ADD_BLOCK_PARAM_LL(record, maxparams,
+ "rd.bytes", rd_bytes);
+ LXC_ADD_BLOCK_PARAM_LL(record, maxparams,
+ "wr.reqs", wr_req);
+ LXC_ADD_BLOCK_PARAM_LL(record, maxparams,
+ "wr.bytes", wr_bytes);
^^
The above all are off by one...
+
+ cleanup:
+ return ret;
+}
+
+#undef LXC_ADD_BLOCK_PARAM_ULL
+
+typedef int
+(*lxcDomainGetStatsFunc)(virLXCDriverPtr driver,
+ virDomainObjPtr dom,
+ virDomainStatsRecordPtr record,
+ int *maxparams);
+
+struct lxcDomainGetStatsWorker {
+ lxcDomainGetStatsFunc func;
+ unsigned int stats;
+};
+
+static struct lxcDomainGetStatsWorker lxcDomainGetStatsWorkers[] = {
+ { lxcDomainGetStatsState, VIR_DOMAIN_STATS_STATE },
+ { lxcDomainGetStatsCpu, VIR_DOMAIN_STATS_CPU_TOTAL },
+ { lxcDomainGetStatsInterface, VIR_DOMAIN_STATS_INTERFACE },
+ { lxcDomainGetStatsBlock, VIR_DOMAIN_STATS_BLOCK },
+ { NULL, 0 }
+};
+
+static int
+lxcDomainGetStats(virConnectPtr conn,
+ virDomainObjPtr dom,
+ unsigned int stats,
+ virDomainStatsRecordPtr *record)
I see qemu doesn't use the @flags anywhere - your call if you think it
could be important in the future - right now it's not could add @flags
here so that if it is ever used
+{
+ virLXCDriverPtr driver = conn->privateData;
+ int maxparams = 0;
+ virDomainStatsRecordPtr tmp;
+ size_t i;
+ int ret = -1;
+
+ if (VIR_ALLOC(tmp) < 0)
+ goto cleanup;
+
+ for (i = 0; lxcDomainGetStatsWorkers[i].func; i++) {
+ if (stats & lxcDomainGetStatsWorkers[i].stats) {
+ if (lxcDomainGetStatsWorkers[i].func(driver, dom, tmp, &maxparams) <
0)
+ goto cleanup;
+ }
+ }
+
+ if (!(tmp->dom = virGetDomain(conn, dom->def->name,
+ dom->def->uuid, dom->def->id)))
+ goto cleanup;
+
+ *record = tmp;
+ tmp = NULL;
+ ret = 0;
+
+ cleanup:
+ if (tmp) {
+ virTypedParamsFree(tmp->params, tmp->nparams);
+ VIR_FREE(tmp);
+ }
+
+ return ret;
+}
+
+static int
+lxcConnectGetAllDomainStats(virConnectPtr conn,
+ virDomainPtr *doms,
+ unsigned int ndoms,
+ unsigned int stats,
+ virDomainStatsRecordPtr **retStats,
+ unsigned int flags)
+{
+ virLXCDriverPtr 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;
+
+ /* TODO Check stats support */
Is this a stray or work still to be done? Seems this is important for
the VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS flag in qemu, which
isn't being supported here.
+
+ 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;
+ }
Ironically if nvms == 0, we return an empty tmpstats with ret = 1...
Same with qemu. I'll post a patch.
+
+ if (VIR_ALLOC_N(tmpstats, nvms + 1) < 0)
+ return -1;
This would leak vms... same bug exists in qemu.
+
+ for (i = 0; i < nvms; i++) {
+ virDomainStatsRecordPtr tmp = NULL;
+ vm = vms[i];
+
+ virObjectLock(vm);
+
+ if (lxcDomainGetStats(conn, vm, stats, &tmp) < 0) {
+ virObjectUnlock(vm);
+ goto cleanup;
+ }
+
+ if (tmp)
+ tmpstats[nstats++] = tmp;
+
+ virObjectUnlock(vm);
+ }
+
+ *retStats = tmpstats;
+ tmpstats = NULL;
VIR_STEAL_PTR(*retStats, tmpstats);
John
+
+ ret = nstats;
+
+ cleanup:
+ virDomainStatsRecordListFree(tmpstats);
+ virObjectListFreeCount(vms, nvms);
+
+ return ret;
+}
/* Function Tables */
static virHypervisorDriver lxcHypervisorDriver = {
@@ -5577,6 +5779,7 @@ static virHypervisorDriver lxcHypervisorDriver = {
.nodeGetFreePages = lxcNodeGetFreePages, /* 1.2.6 */
.nodeAllocPages = lxcNodeAllocPages, /* 1.2.9 */
.domainHasManagedSaveImage = lxcDomainHasManagedSaveImage, /* 1.2.13 */
+ .connectGetAllDomainStats = lxcConnectGetAllDomainStats, /* 4.2.0 */
};
static virConnectDriver lxcConnectDriver = {