[libvirt] [PATCH 0/6] lxc: Add suport to virConnectGetAllDomainStats().
This series has a collection of new methods to enable support for virConnectGetAllDomainStats() to LXC driver. The only difference is Disk Stats. It needs to consider only the host file system operations or any other block device as a external disk or partition. Julio Faracco (6): lxc: Introduce method lxcConnectGetAllDomainStats(). lxc: Introduce method lxcDomainGetStats(). lxc: Introduce method lxcDomainGetStatsCpu(). lxc: Introduce method lxcDomainGetStatsState(). lxc: Introduce method lxcDomainGetBlockStats(). lxc: Introduce method lxcDomainGetBalloonStats(). src/lxc/lxc_driver.c | 357 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 357 insertions(+) -- 2.19.1
This is an implementation of method used by driver to retrieve stats from all domain. Right now, this is a simple implementation considering only State, CPU, Disks and Balloon. Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/lxc/lxc_driver.c | 83 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 760f9f8bdf..1ab6179572 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -5277,6 +5277,88 @@ lxcDomainGetMetadata(virDomainPtr dom, 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); + unsigned int supported = VIR_DOMAIN_STATS_STATE | + VIR_DOMAIN_STATS_CPU_TOTAL | + VIR_DOMAIN_STATS_BLOCK | + VIR_DOMAIN_STATS_BALLOON; + + virCheckFlags(VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE | + VIR_CONNECT_LIST_DOMAINS_FILTERS_PERSISTENT | + VIR_CONNECT_LIST_DOMAINS_FILTERS_STATE | + VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS, -1); + + if (virConnectGetAllDomainStatsEnsureACL(conn) < 0) + return -1; + + if (!stats) { + stats = supported; + } else if ((flags & VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS) && + (stats & ~supported)) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, + _("Stats types bits 0x%x are not supported by this daemon"), + stats & ~supported); + 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, ndoms + 1) < 0) + goto cleanup; + + for (i = 0; i < nvms; i++) { + virDomainStatsRecordPtr tmp; + vm = vms[i]; + + virObjectLock(vm); + tmp = lxcDomainGetStats(conn, vm); + virObjectUnlock(vm); + + if (!tmp) + goto cleanup; + + tmpstats[nstats++] = tmp; + + } + + *retStats = tmpstats; + tmpstats = NULL; + ret = nstats; + + cleanup: + virDomainStatsRecordListFree(tmpstats); + virObjectListFreeCount(vms, nvms); + + return ret; +} static int lxcDomainGetCPUStats(virDomainPtr dom, @@ -5448,6 +5530,7 @@ static virHypervisorDriver lxcHypervisorDriver = { .domainMemoryStats = lxcDomainMemoryStats, /* 1.2.2 */ .nodeGetCPUStats = lxcNodeGetCPUStats, /* 0.9.3 */ .nodeGetMemoryStats = lxcNodeGetMemoryStats, /* 0.9.3 */ + .connectGetAllDomainStats = lxcConnectGetAllDomainStats, /* 5.1.0 */ .nodeGetCellsFreeMemory = lxcNodeGetCellsFreeMemory, /* 0.6.5 */ .nodeGetFreeMemory = lxcNodeGetFreeMemory, /* 0.6.5 */ .nodeGetCPUMap = lxcNodeGetCPUMap, /* 1.0.0 */ -- 2.19.1
On Mon, Mar 11, 2019 at 17:56:13 -0300, Julio Faracco wrote:
This is an implementation of method used by driver to retrieve stats from all domain. Right now, this is a simple implementation considering only State, CPU, Disks and Balloon.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/lxc/lxc_driver.c | 83 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 760f9f8bdf..1ab6179572 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -5277,6 +5277,88 @@ lxcDomainGetMetadata(virDomainPtr dom, 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);
Flags should not be silently ignored. You need to use virCheckFlags and reject unsupported ones.
+ unsigned int supported = VIR_DOMAIN_STATS_STATE | + VIR_DOMAIN_STATS_CPU_TOTAL | + VIR_DOMAIN_STATS_BLOCK | + VIR_DOMAIN_STATS_BALLOON; + + virCheckFlags(VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE | + VIR_CONNECT_LIST_DOMAINS_FILTERS_PERSISTENT | + VIR_CONNECT_LIST_DOMAINS_FILTERS_STATE | + VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS, -1); + + if (virConnectGetAllDomainStatsEnsureACL(conn) < 0) + return -1; + + if (!stats) { + stats = supported; + } else if ((flags & VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS) && + (stats & ~supported)) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, + _("Stats types bits 0x%x are not supported by this daemon"), + stats & ~supported); + 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, ndoms + 1) < 0) + goto cleanup; + + for (i = 0; i < nvms; i++) { + virDomainStatsRecordPtr tmp; + vm = vms[i]; + + virObjectLock(vm); + tmp = lxcDomainGetStats(conn, vm); + virObjectUnlock(vm); + + if (!tmp) + goto cleanup; + + tmpstats[nstats++] = tmp; + + } + + *retStats = tmpstats; + tmpstats = NULL;
VIR_STEAL_PTR(*retStats, tmpstats);
+ ret = nstats; + + cleanup: + virDomainStatsRecordListFree(tmpstats); + virObjectListFreeCount(vms, nvms); + + return ret; +}
static int lxcDomainGetCPUStats(virDomainPtr dom, @@ -5448,6 +5530,7 @@ static virHypervisorDriver lxcHypervisorDriver = { .domainMemoryStats = lxcDomainMemoryStats, /* 1.2.2 */ .nodeGetCPUStats = lxcNodeGetCPUStats, /* 0.9.3 */ .nodeGetMemoryStats = lxcNodeGetMemoryStats, /* 0.9.3 */ + .connectGetAllDomainStats = lxcConnectGetAllDomainStats, /* 5.1.0 */
5.2.0
.nodeGetCellsFreeMemory = lxcNodeGetCellsFreeMemory, /* 0.6.5 */ .nodeGetFreeMemory = lxcNodeGetFreeMemory, /* 0.6.5 */ .nodeGetCPUMap = lxcNodeGetCPUMap, /* 1.0.0 */ -- 2.19.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Please drop the '.' at the end of the commit message. (Possibly the parentheses as well - you alredy called it a method and having a symbol with 'lxc' prefix and 'Get' in its name should be enough of a hint) Jano On Mon, Mar 11, 2019 at 05:56:13PM -0300, Julio Faracco wrote:
This is an implementation of method used by driver to retrieve stats from all domain. Right now, this is a simple implementation considering only State, CPU, Disks and Balloon.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/lxc/lxc_driver.c | 83 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+)
This method is responsible to fetch data like State, CPU, Disk and Balloon info (all single domain info) from each domain 'virDomainObjPtr'. Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/lxc/lxc_driver.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 1ab6179572..72639e0d7d 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -5277,6 +5277,48 @@ lxcDomainGetMetadata(virDomainPtr dom, return ret; } +static virDomainStatsRecordPtr +lxcDomainGetStats(virConnectPtr conn, + virDomainObjPtr dom) +{ + virLXCDriverPtr driver = conn->privateData; + virDomainStatsRecordPtr stat; + int maxparams = 0; + + if (VIR_ALLOC(stat) < 0) + return NULL; + + if (!(stat->dom = virGetDomain(conn, dom->def->name, dom->def->uuid, dom->def->id))) + goto error; + + if (virLXCDomainObjBeginJob(driver, dom, LXC_JOB_QUERY) < 0) + goto error; + + if (lxcDomainGetStatsState(dom, stat, &maxparams) < 0) + goto endjob; + + if (lxcDomainGetStatsCpu(dom, stat, &maxparams) < 0) + goto endjob; + + if (lxcDomainGetBlockStats(dom, stat, &maxparams) < 0) + goto endjob; + + if (lxcDomainGetBalloonStats(dom, stat, &maxparams) < 0) + goto endjob; + + virLXCDomainObjEndJob(driver, dom); + return stat; + + endjob: + virLXCDomainObjEndJob(driver, dom); + + error: + virTypedParamsFree(stat->params, stat->nparams); + virObjectUnref(stat->dom); + VIR_FREE(stat); + return NULL; +} + static int lxcConnectGetAllDomainStats(virConnectPtr conn, virDomainPtr *doms, -- 2.19.1
On Mon, Mar 11, 2019 at 17:56:14 -0300, Julio Faracco wrote:
This method is responsible to fetch data like State, CPU, Disk and Balloon info (all single domain info) from each domain 'virDomainObjPtr'.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/lxc/lxc_driver.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 1ab6179572..72639e0d7d 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -5277,6 +5277,48 @@ lxcDomainGetMetadata(virDomainPtr dom, return ret; }
+static virDomainStatsRecordPtr +lxcDomainGetStats(virConnectPtr conn, + virDomainObjPtr dom) +{ + virLXCDriverPtr driver = conn->privateData; + virDomainStatsRecordPtr stat; + int maxparams = 0; + + if (VIR_ALLOC(stat) < 0) + return NULL; + + if (!(stat->dom = virGetDomain(conn, dom->def->name, dom->def->uuid, dom->def->id))) + goto error; + + if (virLXCDomainObjBeginJob(driver, dom, LXC_JOB_QUERY) < 0) + goto error; + + if (lxcDomainGetStatsState(dom, stat, &maxparams) < 0) + goto endjob;
This does not select the distinct groups of stats according to the 'stats' variable passed to the caller of this. Also as said in cover letter this will not compile.
+ + if (lxcDomainGetStatsCpu(dom, stat, &maxparams) < 0) + goto endjob; + + if (lxcDomainGetBlockStats(dom, stat, &maxparams) < 0) + goto endjob; + + if (lxcDomainGetBalloonStats(dom, stat, &maxparams) < 0) + goto endjob; + + virLXCDomainObjEndJob(driver, dom); + return stat; + + endjob: + virLXCDomainObjEndJob(driver, dom); + + error: + virTypedParamsFree(stat->params, stat->nparams); + virObjectUnref(stat->dom); + VIR_FREE(stat); + return NULL; +} + static int lxcConnectGetAllDomainStats(virConnectPtr conn, virDomainPtr *doms, -- 2.19.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
This method is responsible to fetch all CPU Cgroup Stats and store data into virDomainStatsRecordPtr structure. Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/lxc/lxc_driver.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 72639e0d7d..86c98517d2 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -5277,6 +5277,45 @@ lxcDomainGetMetadata(virDomainPtr dom, return ret; } +static int +lxcDomainGetStatsCpu(virDomainObjPtr dom, + virDomainStatsRecordPtr record, + int *maxparams) +{ + virLXCDomainObjPrivatePtr priv = dom->privateData; + unsigned long long cpu_time = 0; + unsigned long long user_time = 0; + unsigned long long sys_time = 0; + int err = 0; + + if (!priv->cgroup) + return 0; + + err = virCgroupGetCpuacctUsage(priv->cgroup, &cpu_time); + if (!err && virTypedParamsAddULLong(&record->params, + &record->nparams, + maxparams, + "cpu.time", + cpu_time) < 0) + return -1; + + err = virCgroupGetCpuacctStat(priv->cgroup, &user_time, &sys_time); + if (!err && virTypedParamsAddULLong(&record->params, + &record->nparams, + maxparams, + "cpu.user", + user_time) < 0) + return -1; + if (!err && virTypedParamsAddULLong(&record->params, + &record->nparams, + maxparams, + "cpu.system", + sys_time) < 0) + return -1; + + return 0; +} + static virDomainStatsRecordPtr lxcDomainGetStats(virConnectPtr conn, virDomainObjPtr dom) -- 2.19.1
This method is responsible to fetch all State Stats and store data into virDomainStatsRecordPtr structure. Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/lxc/lxc_driver.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 86c98517d2..5fd3bdc5ec 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -5277,6 +5277,28 @@ lxcDomainGetMetadata(virDomainPtr dom, return ret; } +static int +lxcDomainGetStatsState(virDomainObjPtr dom, + virDomainStatsRecordPtr record, + int *maxparams) +{ + if (virTypedParamsAddInt(&record->params, + &record->nparams, + maxparams, + "state.state", + dom->state.state) < 0) + return -1; + + if (virTypedParamsAddInt(&record->params, + &record->nparams, + maxparams, + "state.reason", + dom->state.reason) < 0) + return -1; + + return 0; +} + static int lxcDomainGetStatsCpu(virDomainObjPtr dom, virDomainStatsRecordPtr record, -- 2.19.1
This method is responsible to fetch all Block Stats and store data into virDomainStatsRecordPtr structure. This particular method checks for LXC filesystem in host disk/partition and check for block disks like physical disks and partition that are considered block types. If the block type does not have a CGroup associated, it returns an error. Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/lxc/lxc_driver.c | 100 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 5fd3bdc5ec..1190d67a81 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2482,6 +2482,106 @@ lxcDomainBlockStatsFlags(virDomainPtr dom, return ret; } +#define LXC_STORE_DISK_RECORD(FIELD, COUNTER) \ +do { \ + if (stats.FIELD != -1) { \ + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \ + "block.%zu." COUNTER, i); \ + if (virTypedParamsAddULLong(&record->params, \ + &record->nparams, \ + maxparams, \ + param_name, \ + stats.FIELD) < 0) \ + return -1; \ + } \ +} while (0) + +static int +lxcDomainGetBlockStats(virDomainObjPtr dom, + virDomainStatsRecordPtr record, + int *maxparams) +{ + virLXCDomainObjPrivatePtr priv = dom->privateData; + virDomainBlockStatsStruct stats; + size_t i = 0; + size_t ndisks = 1; + char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; + + if (virCgroupGetBlkioIoServiced(priv->cgroup, + &stats.rd_bytes, + &stats.wr_bytes, + &stats.rd_req, + &stats.wr_req) < 0) + return -1; + + LXC_STORE_DISK_RECORD(rd_req, "rd.reqs"); + LXC_STORE_DISK_RECORD(rd_bytes, "rd.bytes"); + LXC_STORE_DISK_RECORD(wr_req, "wr.reqs"); + LXC_STORE_DISK_RECORD(wr_bytes, "wr.bytes"); + + for (i = 0; i < dom->def->ndisks; i++) { + virDomainDiskDefPtr disk = dom->def->disks[i]; + + if (virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_BLOCK) { + if (virCgroupGetBlkioIoDeviceServiced(priv->cgroup, + disk->src->path, + &stats.rd_bytes, + &stats.wr_bytes, + &stats.rd_req, + &stats.wr_req) < 0) + return -1; + + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, + "block.%zu.name", i); + if (virTypedParamsAddString(&record->params, + &record->nparams, + maxparams, + param_name, + disk->dst) < 0) + return -1; + + if (virStorageSourceIsLocalStorage(disk->src) && disk->src->path) { + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, + "block.%zu.path", i); + if (virTypedParamsAddString(&record->params, + &record->nparams, + maxparams, + param_name, + disk->src->path) < 0) + return -1; + } + + LXC_STORE_DISK_RECORD(rd_req, "rd.reqs"); + LXC_STORE_DISK_RECORD(rd_bytes, "rd.bytes"); + LXC_STORE_DISK_RECORD(wr_req, "wr.reqs"); + LXC_STORE_DISK_RECORD(wr_bytes, "wr.bytes"); + + if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, + "block.%zu.capacity", i); + if (virTypedParamsAddULLong(&record->params, + &record->nparams, + maxparams, + param_name, + disk->src->capacity) < 0) + return -1; + } + + ndisks++; + } + } + + if (virTypedParamsAddUInt(&record->params, + &record->nparams, + maxparams, + "block.count", + ndisks) < 0) + return -1; + + return 0; +} + +#undef LXC_STORE_DISK_RECORD static int lxcDomainSetBlkioParameters(virDomainPtr dom, -- 2.19.1
On Mon, Mar 11, 2019 at 17:56:17 -0300, Julio Faracco wrote:
This method is responsible to fetch all Block Stats and store data into virDomainStatsRecordPtr structure. This particular method checks for LXC filesystem in host disk/partition and check for block disks like physical disks and partition that are considered block types. If the block type does not have a CGroup associated, it returns an error.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/lxc/lxc_driver.c | 100 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 5fd3bdc5ec..1190d67a81 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2482,6 +2482,106 @@ lxcDomainBlockStatsFlags(virDomainPtr dom, return ret; }
+#define LXC_STORE_DISK_RECORD(FIELD, COUNTER) \ +do { \ + if (stats.FIELD != -1) { \ + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \ + "block.%zu." COUNTER, i); \
COUNTER is not counter but field name. This macro also expands to code which tries to access variables which are not passed in. That is usually not a good idea because it obfuscates what's happening.
+ if (virTypedParamsAddULLong(&record->params, \ + &record->nparams, \ + maxparams, \ + param_name, \ + stats.FIELD) < 0) \ + return -1; \ + } \ +} while (0) + +static int +lxcDomainGetBlockStats(virDomainObjPtr dom, + virDomainStatsRecordPtr record, + int *maxparams) +{ + virLXCDomainObjPrivatePtr priv = dom->privateData; + virDomainBlockStatsStruct stats; + size_t i = 0; + size_t ndisks = 1; + char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; + + if (virCgroupGetBlkioIoServiced(priv->cgroup, + &stats.rd_bytes, + &stats.wr_bytes, + &stats.rd_req, + &stats.wr_req) < 0) + return -1; + + LXC_STORE_DISK_RECORD(rd_req, "rd.reqs"); + LXC_STORE_DISK_RECORD(rd_bytes, "rd.bytes"); + LXC_STORE_DISK_RECORD(wr_req, "wr.reqs"); + LXC_STORE_DISK_RECORD(wr_bytes, "wr.bytes");
This adds records for "block.0.rd.reqs", "block.0.rd.bytes", etc ...
+ + for (i = 0; i < dom->def->ndisks; i++) { + virDomainDiskDefPtr disk = dom->def->disks[i]; + + if (virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_BLOCK) { + if (virCgroupGetBlkioIoDeviceServiced(priv->cgroup, + disk->src->path, + &stats.rd_bytes, + &stats.wr_bytes, + &stats.rd_req, + &stats.wr_req) < 0) + return -1; + + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, + "block.%zu.name", i); + if (virTypedParamsAddString(&record->params, + &record->nparams, + maxparams, + param_name, + disk->dst) < 0) + return -1; + + if (virStorageSourceIsLocalStorage(disk->src) && disk->src->path) {
This is always true since everything is guarded by: virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_BLOCK Since we should report also other disks the top condition seems wrong.
+ snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, + "block.%zu.path", i); + if (virTypedParamsAddString(&record->params, + &record->nparams, + maxparams, + param_name, + disk->src->path) < 0) + return -1; + } + + LXC_STORE_DISK_RECORD(rd_req, "rd.reqs"); + LXC_STORE_DISK_RECORD(rd_bytes, "rd.bytes"); + LXC_STORE_DISK_RECORD(wr_req, "wr.reqs"); + LXC_STORE_DISK_RECORD(wr_bytes, "wr.bytes");
... and in the first iteration this also adds "block.0.rd.reqs", "block.0.rd.bytes".
+ + if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
Capacity seems relevant for all styles of disks.
+ snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, + "block.%zu.capacity", i); + if (virTypedParamsAddULLong(&record->params, + &record->nparams, + maxparams, + param_name, + disk->src->capacity) < 0) + return -1; + } + + ndisks++; + } + } + + if (virTypedParamsAddUInt(&record->params, + &record->nparams, + maxparams, + "block.count", + ndisks) < 0) + return -1; + + return 0; +} + +#undef LXC_STORE_DISK_RECORD
static int lxcDomainSetBlkioParameters(virDomainPtr dom, -- 2.19.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
This method is responsible to fetch all Balloon Memory Stats and store data into virDomainStatsRecordPtr structure. Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/lxc/lxc_driver.c | 71 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 1190d67a81..ea48214073 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2876,6 +2876,77 @@ lxcDomainGetBlkioParameters(virDomainPtr dom, return ret; } +static int +lxcDomainGetBalloonStats(virDomainObjPtr dom, + virDomainStatsRecordPtr record, + int *maxparams) +{ + virLXCDomainObjPrivatePtr priv = dom->privateData; + virDomainMemoryStatStruct stats[VIR_DOMAIN_MEMORY_STAT_NR]; + unsigned long long swap_usage; + unsigned long mem_usage; + size_t i; + int ret = -1; + + if (virTypedParamsAddULLong(&record->params, + &record->nparams, + maxparams, + "balloon.maximum", + virDomainDefGetMemoryTotal(dom->def)) < 0) + return -1; + + if (virTypedParamsAddULLong(&record->params, + &record->nparams, + maxparams, + "balloon.current", + virDomainDefGetMemoryTotal(dom->def)) < 0) + return -1; + + if (virCgroupGetMemSwapUsage(priv->cgroup, &swap_usage) < 0) + return -1; + + if (virCgroupGetMemoryUsage(priv->cgroup, &mem_usage) < 0) + return -1; + + ret = 0; + if (ret < *maxparams) { + stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON; + stats[ret].val = dom->def->mem.cur_balloon; + ret++; + } + if (ret < *maxparams) { + stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_SWAP_IN; + stats[ret].val = swap_usage; + ret++; + } + if (ret < *maxparams) { + stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_RSS; + stats[ret].val = mem_usage; + ret++; + } + +#define LXC_STORE_MEM_RECORD(TAG, NAME) \ + if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_ ##TAG) \ + if (virTypedParamsAddULLong(&record->params, \ + &record->nparams, \ + maxparams, \ + "balloon." NAME, \ + stats[i].val) < 0) \ + return -1; + + for (i = 0; i < ret; i++) { + LXC_STORE_MEM_RECORD(SWAP_IN, "swap_in") + LXC_STORE_MEM_RECORD(SWAP_OUT, "swap_out") + LXC_STORE_MEM_RECORD(MAJOR_FAULT, "major_fault") + LXC_STORE_MEM_RECORD(MINOR_FAULT, "minor_fault") + LXC_STORE_MEM_RECORD(AVAILABLE, "available") + LXC_STORE_MEM_RECORD(UNUSED, "unused") + } + +#undef LXC_STORE_MEM_RECORD + + return 0; +} static int lxcDomainInterfaceStats(virDomainPtr dom, -- 2.19.1
On Mon, Mar 11, 2019 at 17:56:18 -0300, Julio Faracco wrote:
This method is responsible to fetch all Balloon Memory Stats and store data into virDomainStatsRecordPtr structure.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/lxc/lxc_driver.c | 71 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 1190d67a81..ea48214073 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2876,6 +2876,77 @@ lxcDomainGetBlkioParameters(virDomainPtr dom, return ret; }
+static int +lxcDomainGetBalloonStats(virDomainObjPtr dom, + virDomainStatsRecordPtr record, + int *maxparams) +{ + virLXCDomainObjPrivatePtr priv = dom->privateData; + virDomainMemoryStatStruct stats[VIR_DOMAIN_MEMORY_STAT_NR]; + unsigned long long swap_usage; + unsigned long mem_usage; + size_t i; + int ret = -1; + + if (virTypedParamsAddULLong(&record->params, + &record->nparams, + maxparams, + "balloon.maximum", + virDomainDefGetMemoryTotal(dom->def)) < 0) + return -1; + + if (virTypedParamsAddULLong(&record->params, + &record->nparams, + maxparams, + "balloon.current", + virDomainDefGetMemoryTotal(dom->def)) < 0) + return -1; + + if (virCgroupGetMemSwapUsage(priv->cgroup, &swap_usage) < 0) + return -1; + + if (virCgroupGetMemoryUsage(priv->cgroup, &mem_usage) < 0) + return -1; + + ret = 0; + if (ret < *maxparams) { + stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON; + stats[ret].val = dom->def->mem.cur_balloon; + ret++;
Erm, why do you put this into the temporary 'stats' variable ...
+ } + if (ret < *maxparams) { + stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_SWAP_IN; + stats[ret].val = swap_usage; + ret++; + } + if (ret < *maxparams) { + stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_RSS; + stats[ret].val = mem_usage; + ret++; + } + +#define LXC_STORE_MEM_RECORD(TAG, NAME) \ + if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_ ##TAG) \ + if (virTypedParamsAddULLong(&record->params, \ + &record->nparams, \ + maxparams, \ + "balloon." NAME, \
... if you then convert it right away into typed parameters? That does not make much sense. You should fill the parameters right away.
+ stats[i].val) < 0) \ + return -1; + + for (i = 0; i < ret; i++) { + LXC_STORE_MEM_RECORD(SWAP_IN, "swap_in") + LXC_STORE_MEM_RECORD(SWAP_OUT, "swap_out") + LXC_STORE_MEM_RECORD(MAJOR_FAULT, "major_fault") + LXC_STORE_MEM_RECORD(MINOR_FAULT, "minor_fault") + LXC_STORE_MEM_RECORD(AVAILABLE, "available") + LXC_STORE_MEM_RECORD(UNUSED, "unused") + } + +#undef LXC_STORE_MEM_RECORD + + return 0; +}
static int lxcDomainInterfaceStats(virDomainPtr dom, -- 2.19.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Mar 11, 2019 at 17:56:12 -0300, Julio Faracco wrote:
This series has a collection of new methods to enable support for virConnectGetAllDomainStats() to LXC driver. The only difference is Disk Stats. It needs to consider only the host file system operations or any other block device as a external disk or partition.
Julio Faracco (6): lxc: Introduce method lxcConnectGetAllDomainStats(). lxc: Introduce method lxcDomainGetStats(). lxc: Introduce method lxcDomainGetStatsCpu(). lxc: Introduce method lxcDomainGetStatsState(). lxc: Introduce method lxcDomainGetBlockStats(). lxc: Introduce method lxcDomainGetBalloonStats().
I've looked through and this series will not compile between commits, which is a requirement for pushing it. You'll need to fix the function calls so that they don't reference code which does not exist at given time. Other comments will be inline.
participants (3)
-
Julio Faracco -
Ján Tomko -
Peter Krempa