[libvirt] [PATCH 3/3] lxc: implement connectGetAllDomainStats
LXC containers can also provide some statistics. Allow users to fetch them using the existing API. --- src/lxc/lxc_driver.c | 203 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 203 insertions(+) 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; } +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) + +static int +lxcDomainGetStatsBlock(virLXCDriverPtr driver, + virDomainObjPtr dom, + virDomainStatsRecordPtr record, + int *maxparams) +{ + long long rd_req, rd_bytes, wr_req, wr_bytes; + + 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); + + 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) +{ + 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 */ + + 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 (lxcDomainGetStats(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; +} /* 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 = { -- 2.16.1
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 = {
participants (2)
-
Cédric Bosdonnat -
John Ferlan