11:44 a.m.
This patch set adds block io, memory and domain cpu statistics API
slot implementations to the LXC driver, in order to get linux
container monitoring and accounting a bit closer to qemu standards.
The last patch is a tad quirky (happy to hear suggestions on
alternative ways), in that it widens the permissible value set
at the .domainBlockStats slot: for lxc guests, it is relatively
likely to have zero disk devices, since host filesystems can be
used via passthrough bind mounts. Therefore, passing the zero-length
string as device path, is interpreted as 'return summary stats for
the entire domains's block io'.
v3 addresses review comments, and adds unit tests for good measure.
Thorsten Behrens (10):
Add util virCgroupGetBlkioIo*Serviced methods.
Implement domainMemoryStats API slot for LXC driver.
Make qemuGetDomainTotalCPUStats a virCgroup function.
Implement domainGetCPUStats for lxc driver.
Implement lxcDomainBlockStats* for lxc driver
Widening API change - accept empty path for virDomainBlockStats
Add unit test for virCgroupGetBlkioIo*Serviced
Add unit test for virCgroupGetMemoryUsage.
Fix misspelled cpuacct.usage_percpu in cgroup mock.
Add unit test for virCgroupGetPercpuStats.
src/libvirt.c | 8 +-
src/libvirt_private.syms | 4 +
src/lxc/lxc_driver.c | 297 +++++++++++++++++++++++++++++++++
src/qemu/qemu_driver.c | 54 +-----
src/util/vircgroup.c | 382 +++++++++++++++++++++++++++++++++++++++++++
src/util/vircgroup.h | 24 +++
tests/vircgroupmock.c | 109 +++++++++++-
tests/vircgrouptest.c | 234 ++++++++++++++++++++++++++
tools/virsh-domain-monitor.c | 11 +-
tools/virsh.pod | 5 +-
10 files changed, 1066 insertions(+), 62 deletions(-)
--
1.8.4
11:44 a.m.
New subject: [libvirt] [PATCHv3 01/10] Add util virCgroupGetBlkioIo*Serviced methods.
This reads blkio stats from blkio.throttle.io_service_bytes and
blkio.throttle.io_serviced.
---
Notes v3:
- beyond the minor nits from the last review,
virCgroupGetBlkioIoDeviceServiced was rather busted wrt. string
ptr p. Now fixed, and with unit test (separate patch)
src/libvirt_private.syms | 2 +
src/util/vircgroup.c | 254 +++++++++++++++++++++++++++++++++++++++++++++++
src/util/vircgroup.h | 12 +++
3 files changed, 268 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1a8d088..5b40d73 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1005,6 +1005,8 @@ virCgroupDenyDevice;
virCgroupDenyDeviceMajor;
virCgroupDenyDevicePath;
virCgroupFree;
+virCgroupGetBlkioIoDeviceServiced;
+virCgroupGetBlkioIoServiced;
virCgroupGetBlkioWeight;
virCgroupGetCpuacctPercpuUsage;
virCgroupGetCpuacctStat;
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index a6d60c5..867bd26 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -1786,6 +1786,233 @@ virCgroupPathOfController(virCgroupPtr group,
/**
+ * virCgroupGetBlkioIoServiced:
+ *
+ * @group: The cgroup to get throughput for
+ * @bytes_read: Pointer to returned bytes read
+ * @bytes_write: Pointer to returned bytes written
+ * @requests_read: Pointer to returned read io ops
+ * @requests_write: Pointer to returned write io ops
+ *
+ * Returns: 0 on success, -1 on error
+ */
+int
+virCgroupGetBlkioIoServiced(virCgroupPtr group,
+ long long *bytes_read,
+ long long *bytes_write,
+ long long *requests_read,
+ long long *requests_write)
+{
+ long long stats_val;
+ char *str1 = NULL, *str2 = NULL, *p1, *p2;
+ size_t i;
+ int ret = -1;
+
+ const char *value_names[] = {
+ "Read ",
+ "Write "
+ };
+ long long *bytes_ptrs[] = {
+ bytes_read,
+ bytes_write
+ };
+ long long *requests_ptrs[] = {
+ requests_read,
+ requests_write
+ };
+
+ *bytes_read = 0;
+ *bytes_write = 0;
+ *requests_read = 0;
+ *requests_write = 0;
+
+ if (virCgroupGetValueStr(group,
+ VIR_CGROUP_CONTROLLER_BLKIO,
+ "blkio.throttle.io_service_bytes", &str1) <
0)
+ goto cleanup;
+
+ if (virCgroupGetValueStr(group,
+ VIR_CGROUP_CONTROLLER_BLKIO,
+ "blkio.throttle.io_serviced", &str2) < 0)
+ goto cleanup;
+
+ /* sum up all entries of the same kind, from all devices */
+ for (i = 0; i < ARRAY_CARDINALITY(value_names); i++) {
+ p1 = str1;
+ p2 = str2;
+
+ while ((p1 = strstr(p1, value_names[i]))) {
+ p1 += strlen(value_names[i]);
+ if (virStrToLong_ll(p1, &p1, 10, &stats_val) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot parse byte %sstat '%s'"),
+ value_names[i],
+ p1);
+ goto cleanup;
+ }
+
+ if (stats_val < 0 ||
+ (stats_val > 0 && *bytes_ptrs[i] > (LLONG_MAX -
stats_val)))
+ {
+ virReportError(VIR_ERR_OVERFLOW,
+ _("Sum of byte %sstat overflows"),
+ value_names[i]);
+ goto cleanup;
+ }
+ *bytes_ptrs[i] += stats_val;
+ }
+
+ while ((p2 = strstr(p2, value_names[i]))) {
+ p2 += strlen(value_names[i]);
+ if (virStrToLong_ll(p2, &p2, 10, &stats_val) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot parse %srequest stat '%s'"),
+ value_names[i],
+ p2);
+ goto cleanup;
+ }
+
+ if (stats_val < 0 ||
+ (stats_val > 0 && *requests_ptrs[i] > (LLONG_MAX -
stats_val)))
+ {
+ virReportError(VIR_ERR_OVERFLOW,
+ _("Sum of %srequest stat overflows"),
+ value_names[i]);
+ goto cleanup;
+ }
+ *requests_ptrs[i] += stats_val;
+ }
+ }
+
+ ret = 0;
+
+cleanup:
+ VIR_FREE(str2);
+ VIR_FREE(str1);
+ return ret;
+}
+
+
+/**
+ * virCgroupGetBlkioIoDeviceServiced:
+ *
+ * @group: The cgroup to get throughput for
+ * @path: The device to get throughput for
+ * @bytes_read: Pointer to returned bytes read
+ * @bytes_write: Pointer to returned bytes written
+ * @requests_read: Pointer to returned read io ops
+ * @requests_write: Pointer to returned write io ops
+ *
+ * Returns: 0 on success, -1 on error
+ */
+int
+virCgroupGetBlkioIoDeviceServiced(virCgroupPtr group,
+ const char *path,
+ long long *bytes_read,
+ long long *bytes_write,
+ long long *requests_read,
+ long long *requests_write)
+{
+ char *str1 = NULL, *str2 = NULL, *str3 = NULL, *p1, *p2;
+ struct stat sb;
+ size_t i;
+ int ret = -1;
+
+ const char *value_names[] = {
+ "Read ",
+ "Write "
+ };
+ long long *bytes_ptrs[] = {
+ bytes_read,
+ bytes_write
+ };
+ long long *requests_ptrs[] = {
+ requests_read,
+ requests_write
+ };
+
+ if (stat(path, &sb) < 0) {
+ virReportSystemError(errno,
+ _("Path '%s' is not accessible"),
+ path);
+ return -1;
+ }
+
+ if (!S_ISBLK(sb.st_mode)) {
+ virReportSystemError(EINVAL,
+ _("Path '%s' must be a block device"),
+ path);
+ return -1;
+ }
+
+ if (virCgroupGetValueStr(group,
+ VIR_CGROUP_CONTROLLER_BLKIO,
+ "blkio.throttle.io_service_bytes", &str1) <
0)
+ goto cleanup;
+
+ if (virCgroupGetValueStr(group,
+ VIR_CGROUP_CONTROLLER_BLKIO,
+ "blkio.throttle.io_serviced", &str2) < 0)
+ goto cleanup;
+
+ if (virAsprintf(&str3, "%d:%d ", major(sb.st_rdev), minor(sb.st_rdev))
< 0)
+ goto cleanup;
+
+ if (!(p1 = strstr(str1, str3))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot find byte stats for block device
'%s'"),
+ str3);
+ goto cleanup;
+ }
+
+ if (!(p2 = strstr(str2, str3))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot find request stats for block device
'%s'"),
+ str3);
+ goto cleanup;
+ }
+
+ for (i = 0; i < ARRAY_CARDINALITY(value_names); i++) {
+ if (!(p1 = strstr(p1, value_names[i]))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot find byte %sstats for block device
'%s'"),
+ value_names[i], str3);
+ goto cleanup;
+ }
+
+ if (virStrToLong_ll(p1 + strlen(value_names[i]), &p1, 10, bytes_ptrs[i]) <
0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot parse %sstat '%s'"),
+ value_names[i], p1 + strlen(value_names[i]));
+ goto cleanup;
+ }
+
+ if (!(p2 = strstr(p2, value_names[i]))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot find request %sstats for block device
'%s'"),
+ value_names[i], str3);
+ goto cleanup;
+ }
+
+ if (virStrToLong_ll(p2 + strlen(value_names[i]), &p2, 10, requests_ptrs[i])
< 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot parse %sstat '%s'"),
+ value_names[i], p2 + strlen(value_names[i]));
+ goto cleanup;
+ }
+ }
+
+ ret = 0;
+
+cleanup:
+ VIR_FREE(str3);
+ VIR_FREE(str2);
+ VIR_FREE(str1);
+ return ret;
+}
+
+
+/**
* virCgroupSetBlkioWeight:
*
* @group: The cgroup to change io weight for
@@ -3459,6 +3686,33 @@ virCgroupMoveTask(virCgroupPtr src_group ATTRIBUTE_UNUSED,
int
+virCgroupGetBlkioIoServiced(virCgroupPtr group ATTRIBUTE_UNUSED,
+ long long *bytes_read ATTRIBUTE_UNUSED,
+ long long *bytes_write ATTRIBUTE_UNUSED,
+ long long *requests_read ATTRIBUTE_UNUSED,
+ long long *requests_write ATTRIBUTE_UNUSED)
+{
+ virReportSystemError(ENXIO, "%s",
+ _("Control groups not supported on this platform"));
+ return -1;
+}
+
+
+int
+virCgroupGetBlkioIoDeviceServiced(virCgroupPtr group ATTRIBUTE_UNUSED,
+ const char *path ATTRIBUTE_UNUSED,
+ long long *bytes_read ATTRIBUTE_UNUSED,
+ long long *bytes_write ATTRIBUTE_UNUSED,
+ long long *requests_read ATTRIBUTE_UNUSED,
+ long long *requests_write ATTRIBUTE_UNUSED)
+{
+ virReportSystemError(ENXIO, "%s",
+ _("Control groups not supported on this platform"));
+ return -1;
+}
+
+
+int
virCgroupSetBlkioWeight(virCgroupPtr group ATTRIBUTE_UNUSED,
unsigned int weight ATTRIBUTE_UNUSED)
{
diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h
index a70eb18..3159a08 100644
--- a/src/util/vircgroup.h
+++ b/src/util/vircgroup.h
@@ -122,6 +122,18 @@ int virCgroupMoveTask(virCgroupPtr src_group,
int virCgroupSetBlkioWeight(virCgroupPtr group, unsigned int weight);
int virCgroupGetBlkioWeight(virCgroupPtr group, unsigned int *weight);
+int virCgroupGetBlkioIoServiced(virCgroupPtr group,
+ long long *bytes_read,
+ long long *bytes_write,
+ long long *requests_read,
+ long long *requests_write);
+int virCgroupGetBlkioIoDeviceServiced(virCgroupPtr group,
+ const char *path,
+ long long *bytes_read,
+ long long *bytes_write,
+ long long *requests_read,
+ long long *requests_write);
+
int virCgroupSetBlkioDeviceWeight(virCgroupPtr group,
const char *path,
unsigned int weight);
--
1.8.4
5:46 a.m.
New subject: [libvirt] [PATCHv3 01/10] Add util virCgroupGetBlkioIo*Serviced methods.
On 03.02.2014 18:44, Thorsten Behrens wrote:
This reads blkio stats from blkio.throttle.io_service_bytes and
blkio.throttle.io_serviced.
---
Notes v3:
- beyond the minor nits from the last review,
virCgroupGetBlkioIoDeviceServiced was rather busted wrt. string
ptr p. Now fixed, and with unit test (separate patch)
src/libvirt_private.syms | 2 +
src/util/vircgroup.c | 254 +++++++++++++++++++++++++++++++++++++++++++++++
src/util/vircgroup.h | 12 +++
3 files changed, 268 insertions(+)
ACK
Michal
11:44 a.m.
New subject: [libvirt] [PATCHv3 02/10] Implement domainMemoryStats API slot for LXC driver.
---
src/lxc/lxc_driver.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 138c706..02b5cc3 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -5169,6 +5169,55 @@ lxcNodeGetInfo(virConnectPtr conn,
static int
+lxcDomainMemoryStats(virDomainPtr dom,
+ struct _virDomainMemoryStat *stats,
+ unsigned int nr_stats,
+ unsigned int flags)
+{
+ virDomainObjPtr vm;
+ int ret = -1;
+ virLXCDomainObjPrivatePtr priv;
+
+ virCheckFlags(0, -1);
+
+ if (!(vm = lxcDomObjFromDomain(dom)))
+ goto cleanup;
+
+ priv = vm->privateData;
+
+ if (virDomainMemoryStatsEnsureACL(dom->conn, vm->def) < 0)
+ goto cleanup;
+
+ ret = 0;
+ if (!virDomainObjIsActive(vm))
+ goto cleanup;
+
+ if (ret < nr_stats) {
+ stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON;
+ stats[ret].val = vm->def->mem.cur_balloon;
+ ret++;
+ }
+ if (ret < nr_stats) {
+ stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_SWAP_IN;
+ virCgroupGetMemSwapUsage(priv->cgroup, &stats[ret].val);
+ ret++;
+ }
+ if (ret < nr_stats) {
+ unsigned long kb;
+ stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_RSS;
+ virCgroupGetMemoryUsage(priv->cgroup, &kb);
+ stats[ret].val = kb;
+ ret++;
+ }
+
+cleanup:
+ if (vm)
+ virObjectUnlock(vm);
+ return ret;
+}
+
+
+static int
lxcNodeGetCPUStats(virConnectPtr conn,
int cpuNum,
virNodeCPUStatsPtr params,
@@ -5397,6 +5446,7 @@ static virDriver lxcDriver = {
.domainSetSchedulerParameters = lxcDomainSetSchedulerParameters, /* 0.5.0 */
.domainSetSchedulerParametersFlags = lxcDomainSetSchedulerParametersFlags, /* 0.9.2
*/
.domainInterfaceStats = lxcDomainInterfaceStats, /* 0.7.3 */
+ .domainMemoryStats = lxcDomainMemoryStats, /* 1.2.2 */
.nodeGetCPUStats = lxcNodeGetCPUStats, /* 0.9.3 */
.nodeGetMemoryStats = lxcNodeGetMemoryStats, /* 0.9.3 */
.nodeGetCellsFreeMemory = lxcNodeGetCellsFreeMemory, /* 0.6.5 */
--
1.8.4
5:47 a.m.
New subject: [libvirt] [PATCHv3 02/10] Implement domainMemoryStats API slot for LXC driver.
On 03.02.2014 18:44, Thorsten Behrens wrote:
---
src/lxc/lxc_driver.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 138c706..02b5cc3 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -5169,6 +5169,55 @@ lxcNodeGetInfo(virConnectPtr conn,
static int
+lxcDomainMemoryStats(virDomainPtr dom,
+ struct _virDomainMemoryStat *stats,
+ unsigned int nr_stats,
+ unsigned int flags)
+{
+ virDomainObjPtr vm;
+ int ret = -1;
+ virLXCDomainObjPrivatePtr priv;
+
+ virCheckFlags(0, -1);
+
+ if (!(vm = lxcDomObjFromDomain(dom)))
+ goto cleanup;
+
+ priv = vm->privateData;
+
+ if (virDomainMemoryStatsEnsureACL(dom->conn, vm->def) < 0)
+ goto cleanup;
+
+ ret = 0;
+ if (!virDomainObjIsActive(vm))
+ goto cleanup;
+
+ if (ret < nr_stats) {
+ stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON;
+ stats[ret].val = vm->def->mem.cur_balloon;
+ ret++;
+ }
+ if (ret < nr_stats) {
+ stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_SWAP_IN;
+ virCgroupGetMemSwapUsage(priv->cgroup, &stats[ret].val);
+ ret++;
+ }
+ if (ret < nr_stats) {
+ unsigned long kb;
+ stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_RSS;
+ virCgroupGetMemoryUsage(priv->cgroup, &kb);
+ stats[ret].val = kb;
+ ret++;
+ }
Both these virCgroupGetMem* may fail, in which case the return pointer
is untouched hence we will return bogus value.
Just a side note - all of our virCgroupGet* handling functions take
unsigned long long (if they take integer at all). Except the
virCgroupGetMemoryUsage(). I wonder why. In fact quick git grep over the
function reveals another places where a UL local variable is introduced
just so it can be later casted to ULL .. If you decide to do something
with this, it can be a follow up/separate patch to this series.
Michal
11:44 a.m.
New subject: [libvirt] [PATCHv3 03/10] Make qemuGetDomainTotalCPUStats a virCgroup function.
To reuse this from other drivers, like lxc.
---
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 54 ++----------------------------------------------
src/util/vircgroup.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++
src/util/vircgroup.h | 5 +++++
4 files changed, 61 insertions(+), 52 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 5b40d73..890d6c7 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1016,6 +1016,7 @@ virCgroupGetCpuCfsQuota;
virCgroupGetCpusetCpus;
virCgroupGetCpusetMems;
virCgroupGetCpuShares;
+virCgroupGetDomainTotalCpuStats;
virCgroupGetFreezerState;
virCgroupGetMemoryHardLimit;
virCgroupGetMemorySoftLimit;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0128356..f3570bd 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -105,7 +105,6 @@
#define QEMU_NB_NUMA_PARAM 2
-#define QEMU_NB_TOTAL_CPU_STAT_PARAM 3
#define QEMU_NB_PER_CPU_STAT_PARAM 2
#define QEMU_SCHED_MIN_PERIOD 1000LL
@@ -15772,56 +15771,6 @@ cleanup:
return ret;
}
-/* qemuDomainGetCPUStats() with start_cpu == -1 */
-static int
-qemuDomainGetTotalcpuStats(virDomainObjPtr vm,
- virTypedParameterPtr params,
- int nparams)
-{
- unsigned long long cpu_time;
- int ret;
- qemuDomainObjPrivatePtr priv = vm->privateData;
-
- if (nparams == 0) /* return supported number of params */
- return QEMU_NB_TOTAL_CPU_STAT_PARAM;
- /* entry 0 is cputime */
- ret = virCgroupGetCpuacctUsage(priv->cgroup, &cpu_time);
- if (ret < 0) {
- virReportSystemError(-ret, "%s", _("unable to get cpu
account"));
- return -1;
- }
-
- if (virTypedParameterAssign(¶ms[0], VIR_DOMAIN_CPU_STATS_CPUTIME,
- VIR_TYPED_PARAM_ULLONG, cpu_time) < 0)
- return -1;
-
- if (nparams > 1) {
- unsigned long long user;
- unsigned long long sys;
-
- ret = virCgroupGetCpuacctStat(priv->cgroup, &user, &sys);
- if (ret < 0) {
- virReportSystemError(-ret, "%s", _("unable to get cpu
account"));
- return -1;
- }
-
- if (virTypedParameterAssign(¶ms[1],
- VIR_DOMAIN_CPU_STATS_USERTIME,
- VIR_TYPED_PARAM_ULLONG, user) < 0)
- return -1;
- if (nparams > 2 &&
- virTypedParameterAssign(¶ms[2],
- VIR_DOMAIN_CPU_STATS_SYSTEMTIME,
- VIR_TYPED_PARAM_ULLONG, sys) < 0)
- return -1;
-
- if (nparams > QEMU_NB_TOTAL_CPU_STAT_PARAM)
- nparams = QEMU_NB_TOTAL_CPU_STAT_PARAM;
- }
-
- return nparams;
-}
-
/* This function gets the sums of cpu time consumed by all vcpus.
* For example, if there are 4 physical cpus, and 2 vcpus in a domain,
* then for each vcpu, the cpuacct.usage_percpu looks like this:
@@ -16019,7 +15968,8 @@ qemuDomainGetCPUStats(virDomainPtr domain,
}
if (start_cpu == -1)
- ret = qemuDomainGetTotalcpuStats(vm, params, nparams);
+ ret = virCgroupGetDomainTotalCpuStats(priv->cgroup,
+ params, nparams);
else
ret = qemuDomainGetPercpuStats(vm, params, nparams,
start_cpu, ncpus);
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 867bd26..7427a21 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -51,11 +51,14 @@
#include "virhashcode.h"
#include "virstring.h"
#include "virsystemd.h"
+#include "virtypedparam.h"
#define CGROUP_MAX_VAL 512
#define VIR_FROM_THIS VIR_FROM_CGROUP
+#define CGROUP_NB_TOTAL_CPU_STAT_PARAM 3
+
#if defined(__linux__) && defined(HAVE_GETMNTENT_R) && \
defined(_DIRENT_HAVE_D_TYPE) && defined(_SC_CLK_TCK)
# define VIR_CGROUP_SUPPORTED
@@ -2821,6 +2824,56 @@ virCgroupDenyDevicePath(virCgroupPtr group, const char *path, int
perms)
}
+
+int
+virCgroupGetDomainTotalCpuStats(virCgroupPtr group,
+ virTypedParameterPtr params,
+ int nparams)
+{
+ unsigned long long cpu_time;
+ int ret;
+
+ if (nparams == 0) /* return supported number of params */
+ return CGROUP_NB_TOTAL_CPU_STAT_PARAM;
+ /* entry 0 is cputime */
+ ret = virCgroupGetCpuacctUsage(group, &cpu_time);
+ if (ret < 0) {
+ virReportSystemError(-ret, "%s", _("unable to get cpu
account"));
+ return -1;
+ }
+
+ if (virTypedParameterAssign(¶ms[0], VIR_DOMAIN_CPU_STATS_CPUTIME,
+ VIR_TYPED_PARAM_ULLONG, cpu_time) < 0)
+ return -1;
+
+ if (nparams > 1) {
+ unsigned long long user;
+ unsigned long long sys;
+
+ ret = virCgroupGetCpuacctStat(group, &user, &sys);
+ if (ret < 0) {
+ virReportSystemError(-ret, "%s", _("unable to get cpu
account"));
+ return -1;
+ }
+
+ if (virTypedParameterAssign(¶ms[1],
+ VIR_DOMAIN_CPU_STATS_USERTIME,
+ VIR_TYPED_PARAM_ULLONG, user) < 0)
+ return -1;
+ if (nparams > 2 &&
+ virTypedParameterAssign(¶ms[2],
+ VIR_DOMAIN_CPU_STATS_SYSTEMTIME,
+ VIR_TYPED_PARAM_ULLONG, sys) < 0)
+ return -1;
+
+ if (nparams > CGROUP_NB_TOTAL_CPU_STAT_PARAM)
+ nparams = CGROUP_NB_TOTAL_CPU_STAT_PARAM;
+ }
+
+ return nparams;
+}
+
+
int
virCgroupSetCpuShares(virCgroupPtr group, unsigned long long shares)
{
diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h
index 3159a08..fae4d92 100644
--- a/src/util/vircgroup.h
+++ b/src/util/vircgroup.h
@@ -201,6 +201,11 @@ int virCgroupDenyDevicePath(virCgroupPtr group,
const char *path,
int perms);
+int
+virCgroupGetDomainTotalCpuStats(virCgroupPtr group,
+ virTypedParameterPtr params,
+ int nparams);
+
int virCgroupSetCpuShares(virCgroupPtr group, unsigned long long shares);
int virCgroupGetCpuShares(virCgroupPtr group, unsigned long long *shares);
--
1.8.4
5:46 a.m.
New subject: [libvirt] [PATCHv3 03/10] Make qemuGetDomainTotalCPUStats a virCgroup function.
On 03.02.2014 18:44, Thorsten Behrens wrote:
To reuse this from other drivers, like lxc.
---
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 54 ++----------------------------------------------
src/util/vircgroup.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++
src/util/vircgroup.h | 5 +++++
4 files changed, 61 insertions(+), 52 deletions(-)
ACK, pure code movement.
Michal
11:44 a.m.
New subject: [libvirt] [PATCHv3 04/10] Implement domainGetCPUStats for lxc driver.
---
Notes v3:
- moved most of the code out to virCgroupGetPercpuStats, for better
testability.
- addressed comments from v2 review
src/libvirt_private.syms | 1 +
src/lxc/lxc_driver.c | 51 ++++++++++++++++++++++++++++++++
src/util/vircgroup.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++
src/util/vircgroup.h | 7 +++++
4 files changed, 134 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 890d6c7..4a7201e 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1023,6 +1023,7 @@ virCgroupGetMemorySoftLimit;
virCgroupGetMemoryUsage;
virCgroupGetMemSwapHardLimit;
virCgroupGetMemSwapUsage;
+virCgroupGetPercpuStats;
virCgroupHasController;
virCgroupIsolateMount;
virCgroupKill;
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 02b5cc3..39cd981 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -76,6 +76,7 @@
#define LXC_NB_MEM_PARAM 3
+
static int lxcStateInitialize(bool privileged,
virStateInhibitCallback callback,
void *opaque);
@@ -5388,6 +5389,55 @@ cleanup:
}
+static int
+lxcDomainGetCPUStats(virDomainPtr dom,
+ virTypedParameterPtr params,
+ unsigned int nparams,
+ int start_cpu,
+ unsigned int ncpus,
+ unsigned int flags)
+{
+ virDomainObjPtr vm = NULL;
+ int ret = -1;
+ bool isActive;
+ virLXCDomainObjPrivatePtr priv;
+
+ virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
+
+ if (!(vm = lxcDomObjFromDomain(dom)))
+ return ret;
+
+ priv = vm->privateData;
+
+ if (virDomainGetCPUStatsEnsureACL(dom->conn, vm->def) < 0)
+ goto cleanup;
+
+ isActive = virDomainObjIsActive(vm);
+ if (!isActive) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("domain is not running"));
+ goto cleanup;
+ }
+
+ if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUACCT)) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ "%s", _("cgroup CPUACCT controller is not
mounted"));
+ goto cleanup;
+ }
+
+ if (start_cpu == -1)
+ ret = virCgroupGetDomainTotalCpuStats(priv->cgroup,
+ params, nparams);
+ else
+ ret = virCgroupGetPercpuStats(priv->cgroup, params,
+ nparams, start_cpu, ncpus);
+cleanup:
+ if (vm)
+ virObjectUnlock(vm);
+ return ret;
+}
+
+
/* Function Tables */
static virDriver lxcDriver = {
.no = VIR_DRV_LXC,
@@ -5466,6 +5516,7 @@ static virDriver lxcDriver = {
.nodeSuspendForDuration = lxcNodeSuspendForDuration, /* 0.9.8 */
.domainSetMetadata = lxcDomainSetMetadata, /* 1.1.3 */
.domainGetMetadata = lxcDomainGetMetadata, /* 1.1.3 */
+ .domainGetCPUStats = lxcDomainGetCPUStats, /* 1.2.2 */
.nodeGetMemoryParameters = lxcNodeGetMemoryParameters, /* 0.10.2 */
.nodeSetMemoryParameters = lxcNodeSetMemoryParameters, /* 0.10.2 */
.domainSendProcessSignal = lxcDomainSendProcessSignal, /* 1.0.1 */
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 7427a21..268a4ae 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -53,11 +53,14 @@
#include "virsystemd.h"
#include "virtypedparam.h"
+#include "nodeinfo.h"
+
#define CGROUP_MAX_VAL 512
#define VIR_FROM_THIS VIR_FROM_CGROUP
#define CGROUP_NB_TOTAL_CPU_STAT_PARAM 3
+#define CGROUP_NB_PER_CPU_STAT_PARAM 1
#if defined(__linux__) && defined(HAVE_GETMNTENT_R) && \
defined(_DIRENT_HAVE_D_TYPE) && defined(_SC_CLK_TCK)
@@ -2824,6 +2827,78 @@ virCgroupDenyDevicePath(virCgroupPtr group, const char *path, int
perms)
}
+int
+virCgroupGetPercpuStats(virCgroupPtr group,
+ virTypedParameterPtr params,
+ unsigned int nparams,
+ int start_cpu,
+ unsigned int ncpus)
+{
+ int rv = -1;
+ size_t i;
+ int id, max_id;
+ char *pos;
+ char *buf = NULL;
+ virTypedParameterPtr ent;
+ int param_idx;
+ unsigned long long cpu_time;
+
+ /* return the number of supported params */
+ if (nparams == 0 && ncpus != 0)
+ return CGROUP_NB_PER_CPU_STAT_PARAM;
+
+ /* To parse account file, we need to know how many cpus are present. */
+ max_id = nodeGetCPUCount();
+ if (max_id < 0)
+ return rv;
+
+ if (ncpus == 0) { /* returns max cpu ID */
+ rv = max_id;
+ goto cleanup;
+ }
+
+ if (start_cpu > max_id) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("start_cpu %d larger than maximum of %d"),
+ start_cpu, max_id);
+ goto cleanup;
+ }
+
+ /* we get percpu cputime accounting info. */
+ if (virCgroupGetCpuacctPercpuUsage(group, &buf))
+ goto cleanup;
+ pos = buf;
+
+ /* return percpu cputime in index 0 */
+ param_idx = 0;
+
+ /* number of cpus to compute */
+ if (start_cpu >= max_id - ncpus)
+ id = max_id - 1;
+ else
+ id = start_cpu + ncpus - 1;
+
+ for (i = 0; i <= id; i++) {
+ if (virStrToLong_ull(pos, &pos, 10, &cpu_time) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("cpuacct parse error"));
+ goto cleanup;
+ }
+ if (i < start_cpu)
+ continue;
+ ent = ¶ms[(i - start_cpu) * nparams + param_idx];
+ if (virTypedParameterAssign(ent, VIR_DOMAIN_CPU_STATS_CPUTIME,
+ VIR_TYPED_PARAM_ULLONG, cpu_time) < 0)
+ goto cleanup;
+ }
+
+ rv = nparams;
+
+cleanup:
+ VIR_FREE(buf);
+ return rv;
+}
+
int
virCgroupGetDomainTotalCpuStats(virCgroupPtr group,
diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h
index fae4d92..67478f5 100644
--- a/src/util/vircgroup.h
+++ b/src/util/vircgroup.h
@@ -202,6 +202,13 @@ int virCgroupDenyDevicePath(virCgroupPtr group,
int perms);
int
+virCgroupGetPercpuStats(virCgroupPtr group,
+ virTypedParameterPtr params,
+ unsigned int nparams,
+ int start_cpu,
+ unsigned int ncpus);
+
+int
virCgroupGetDomainTotalCpuStats(virCgroupPtr group,
virTypedParameterPtr params,
int nparams);
--
1.8.4
5:46 a.m.
New subject: [libvirt] [PATCHv3 04/10] Implement domainGetCPUStats for lxc driver.
On 03.02.2014 18:44, Thorsten Behrens wrote:
---
Notes v3:
- moved most of the code out to virCgroupGetPercpuStats, for better
testability.
- addressed comments from v2 review
src/libvirt_private.syms | 1 +
src/lxc/lxc_driver.c | 51 ++++++++++++++++++++++++++++++++
src/util/vircgroup.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++
src/util/vircgroup.h | 7 +++++
4 files changed, 134 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 890d6c7..4a7201e 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1023,6 +1023,7 @@ virCgroupGetMemorySoftLimit;
virCgroupGetMemoryUsage;
virCgroupGetMemSwapHardLimit;
virCgroupGetMemSwapUsage;
+virCgroupGetPercpuStats;
virCgroupHasController;
virCgroupIsolateMount;
virCgroupKill;
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 02b5cc3..39cd981 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -76,6 +76,7 @@
#define LXC_NB_MEM_PARAM 3
+
static int lxcStateInitialize(bool privileged,
virStateInhibitCallback callback,
void *opaque);
@@ -5388,6 +5389,55 @@ cleanup:
}
+static int
+lxcDomainGetCPUStats(virDomainPtr dom,
+ virTypedParameterPtr params,
+ unsigned int nparams,
+ int start_cpu,
+ unsigned int ncpus,
+ unsigned int flags)
+{
+ virDomainObjPtr vm = NULL;
+ int ret = -1;
+ bool isActive;
This variable is unnecessary. Although I see you are just copying
structure we have in the qemu driver.
+ virLXCDomainObjPrivatePtr priv;
+
+ virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
+
+ if (!(vm = lxcDomObjFromDomain(dom)))
+ return ret;
+
+ priv = vm->privateData;
+
+ if (virDomainGetCPUStatsEnsureACL(dom->conn, vm->def) < 0)
+ goto cleanup;
+
+ isActive = virDomainObjIsActive(vm);
+ if (!isActive) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("domain is not running"));
+ goto cleanup;
+ }
+
+ if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUACCT)) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ "%s", _("cgroup CPUACCT controller is not
mounted"));
+ goto cleanup;
+ }
+
+ if (start_cpu == -1)
+ ret = virCgroupGetDomainTotalCpuStats(priv->cgroup,
+ params, nparams);
+ else
+ ret = virCgroupGetPercpuStats(priv->cgroup, params,
+ nparams, start_cpu, ncpus);
+cleanup:
+ if (vm)
+ virObjectUnlock(vm);
+ return ret;
+}
+
+
/* Function Tables */
static virDriver lxcDriver = {
.no = VIR_DRV_LXC,
@@ -5466,6 +5516,7 @@ static virDriver lxcDriver = {
.nodeSuspendForDuration = lxcNodeSuspendForDuration, /* 0.9.8 */
.domainSetMetadata = lxcDomainSetMetadata, /* 1.1.3 */
.domainGetMetadata = lxcDomainGetMetadata, /* 1.1.3 */
+ .domainGetCPUStats = lxcDomainGetCPUStats, /* 1.2.2 */
.nodeGetMemoryParameters = lxcNodeGetMemoryParameters, /* 0.10.2 */
.nodeSetMemoryParameters = lxcNodeSetMemoryParameters, /* 0.10.2 */
.domainSendProcessSignal = lxcDomainSendProcessSignal, /* 1.0.1 */
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 7427a21..268a4ae 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -53,11 +53,14 @@
#include "virsystemd.h"
#include "virtypedparam.h"
+#include "nodeinfo.h"
+
#define CGROUP_MAX_VAL 512
#define VIR_FROM_THIS VIR_FROM_CGROUP
#define CGROUP_NB_TOTAL_CPU_STAT_PARAM 3
+#define CGROUP_NB_PER_CPU_STAT_PARAM 1
#if defined(__linux__) && defined(HAVE_GETMNTENT_R) && \
defined(_DIRENT_HAVE_D_TYPE) && defined(_SC_CLK_TCK)
@@ -2824,6 +2827,78 @@ virCgroupDenyDevicePath(virCgroupPtr group, const char *path, int
perms)
}
+int
+virCgroupGetPercpuStats(virCgroupPtr group,
+ virTypedParameterPtr params,
+ unsigned int nparams,
+ int start_cpu,
+ unsigned int ncpus)
+{
+ int rv = -1;
+ size_t i;
+ int id, max_id;
+ char *pos;
+ char *buf = NULL;
+ virTypedParameterPtr ent;
+ int param_idx;
+ unsigned long long cpu_time;
+
+ /* return the number of supported params */
+ if (nparams == 0 && ncpus != 0)
+ return CGROUP_NB_PER_CPU_STAT_PARAM;
+
+ /* To parse account file, we need to know how many cpus are present. */
+ max_id = nodeGetCPUCount();
+ if (max_id < 0)
+ return rv;
+
+ if (ncpus == 0) { /* returns max cpu ID */
+ rv = max_id;
+ goto cleanup;
+ }
+
+ if (start_cpu > max_id) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("start_cpu %d larger than maximum of %d"),
+ start_cpu, max_id);
+ goto cleanup;
+ }
+
+ /* we get percpu cputime accounting info. */
+ if (virCgroupGetCpuacctPercpuUsage(group, &buf))
+ goto cleanup;
+ pos = buf;
+
+ /* return percpu cputime in index 0 */
+ param_idx = 0;
+
+ /* number of cpus to compute */
+ if (start_cpu >= max_id - ncpus)
+ id = max_id - 1;
+ else
+ id = start_cpu + ncpus - 1;
+
+ for (i = 0; i <= id; i++) {
+ if (virStrToLong_ull(pos, &pos, 10, &cpu_time) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("cpuacct parse error"));
+ goto cleanup;
+ }
+ if (i < start_cpu)
+ continue;
+ ent = ¶ms[(i - start_cpu) * nparams + param_idx];
+ if (virTypedParameterAssign(ent, VIR_DOMAIN_CPU_STATS_CPUTIME,
+ VIR_TYPED_PARAM_ULLONG, cpu_time) < 0)
+ goto cleanup;
+ }
+
+ rv = nparams;
+
+cleanup:
+ VIR_FREE(buf);
+ return rv;
+}
+
int
virCgroupGetDomainTotalCpuStats(virCgroupPtr group,
diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h
index fae4d92..67478f5 100644
--- a/src/util/vircgroup.h
+++ b/src/util/vircgroup.h
@@ -202,6 +202,13 @@ int virCgroupDenyDevicePath(virCgroupPtr group,
int perms);
int
+virCgroupGetPercpuStats(virCgroupPtr group,
+ virTypedParameterPtr params,
+ unsigned int nparams,
+ int start_cpu,
+ unsigned int ncpus);
+
+int
virCgroupGetDomainTotalCpuStats(virCgroupPtr group,
virTypedParameterPtr params,
int nparams);
ACK
Michal
11:44 a.m.
New subject: [libvirt] [PATCHv3 05/10] Implement lxcDomainBlockStats* for lxc driver
Adds lxcDomainBlockStatsFlags and lxcDomainBlockStats functions.
---
Notes v3:
- merged patch, both api methods now added in one
- addressed comments from v2 review
- check for cgroup controllers actually being mounted
src/lxc/lxc_driver.c | 196 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 196 insertions(+)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 39cd981..103352c 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -75,6 +75,7 @@
#define LXC_NB_MEM_PARAM 3
+#define LXC_NB_DOMAIN_BLOCK_STAT_PARAM 4
static int lxcStateInitialize(bool privileged,
@@ -2202,6 +2203,199 @@ lxcDomainMergeBlkioDevice(virBlkioDevicePtr *dest_array,
static int
+lxcDomainBlockStats(virDomainPtr dom,
+ const char *path,
+ struct _virDomainBlockStats *stats)
+{
+ int ret = -1, idx;
+ virDomainObjPtr vm;
+ virDomainDiskDefPtr disk = NULL;
+ virLXCDomainObjPrivatePtr priv;
+
+ if (!(vm = lxcDomObjFromDomain(dom)))
+ return ret;
+
+ priv = vm->privateData;
+
+ if (virDomainBlockStatsEnsureACL(dom->conn, vm->def) < 0)
+ goto cleanup;
+
+ if (!virDomainObjIsActive(vm)) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ "%s", _("domain is not running"));
+ goto cleanup;
+ }
+
+ if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("blkio cgroup isn't mounted"));
+ goto cleanup;
+ }
+
+ if (!*path) {
+ /* empty path - return entire domain blkstats instead */
+ ret = virCgroupGetBlkioIoServiced(priv->cgroup,
+ &stats->rd_bytes,
+ &stats->wr_bytes,
+ &stats->rd_req,
+ &stats->wr_req);
+ goto cleanup;
+ }
+
+ if ((idx = virDomainDiskIndexByName(vm->def, path, false)) < 0) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("invalid path: %s"), path);
+ goto cleanup;
+ }
+ disk = vm->def->disks[idx];
+
+ if (!disk->info.alias) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("missing disk device alias name for %s"),
disk->dst);
+ goto cleanup;
+ }
+
+ ret = virCgroupGetBlkioIoDeviceServiced(priv->cgroup,
+ disk->info.alias,
+ &stats->rd_bytes,
+ &stats->wr_bytes,
+ &stats->rd_req,
+ &stats->wr_req);
+cleanup:
+ if (vm)
+ virObjectUnlock(vm);
+ return ret;
+}
+
+
+static int
+lxcDomainBlockStatsFlags(virDomainPtr dom,
+ const char * path,
+ virTypedParameterPtr params,
+ int * nparams,
+ unsigned int flags)
+{
+ int tmp, ret = -1, idx;
+ virDomainObjPtr vm;
+ virDomainDiskDefPtr disk = NULL;
+ virLXCDomainObjPrivatePtr priv;
+ long long rd_req, rd_bytes, wr_req, wr_bytes;
+ virTypedParameterPtr param;
+
+ virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
+
+ /* We don't return strings, and thus trivially support this flag. */
+ flags &= ~VIR_TYPED_PARAM_STRING_OKAY;
+
+ if (!params && !*nparams) {
+ *nparams = LXC_NB_DOMAIN_BLOCK_STAT_PARAM;
+ return 0;
+ }
+
+ if (!(vm = lxcDomObjFromDomain(dom)))
+ return ret;
+
+ priv = vm->privateData;
+
+ if (virDomainBlockStatsFlagsEnsureACL(dom->conn, vm->def) < 0)
+ goto cleanup;
+
+ if (!virDomainObjIsActive(vm)) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ "%s", _("domain is not running"));
+ goto cleanup;
+ }
+
+ if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("blkio cgroup isn't mounted"));
+ goto cleanup;
+ }
+
+ if (!*path) {
+ /* empty path - return entire domain blkstats instead */
+ if (virCgroupGetBlkioIoServiced(priv->cgroup,
+ &rd_bytes,
+ &wr_bytes,
+ &rd_req,
+ &wr_req) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("domain stats query failed"));
+ goto cleanup;
+ }
+ }
+ else {
+ if ((idx = virDomainDiskIndexByName(vm->def, path, false)) < 0) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("invalid path: %s"), path);
+ goto cleanup;
+ }
+ disk = vm->def->disks[idx];
+
+ if (!disk->info.alias) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("missing disk device alias name for %s"),
disk->dst);
+ goto cleanup;
+ }
+
+ if (virCgroupGetBlkioIoDeviceServiced(priv->cgroup,
+ disk->info.alias,
+ &rd_bytes,
+ &wr_bytes,
+ &rd_req,
+ &wr_req) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("domain stats query failed"));
+ goto cleanup;
+ }
+ }
+
+ tmp = 0;
+ ret = -1;
+
+ if (tmp < *nparams && wr_bytes != -1) {
+ param = ¶ms[tmp];
+ if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_WRITE_BYTES,
+ VIR_TYPED_PARAM_LLONG, wr_bytes) < 0)
+ goto cleanup;
+ tmp++;
+ }
+
+ if (tmp < *nparams && wr_req != -1) {
+ param = ¶ms[tmp];
+ if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_WRITE_REQ,
+ VIR_TYPED_PARAM_LLONG, wr_req) < 0)
+ goto cleanup;
+ tmp++;
+ }
+
+ if (tmp < *nparams && rd_bytes != -1) {
+ param = ¶ms[tmp];
+ if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_READ_BYTES,
+ VIR_TYPED_PARAM_LLONG, rd_bytes) < 0)
+ goto cleanup;
+ tmp++;
+ }
+
+ if (tmp < *nparams && rd_req != -1) {
+ param = ¶ms[tmp];
+ if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_READ_REQ,
+ VIR_TYPED_PARAM_LLONG, rd_req) < 0)
+ goto cleanup;
+ tmp++;
+ }
+
+ ret = 0;
+ *nparams = tmp;
+
+cleanup:
+ if (vm)
+ virObjectUnlock(vm);
+ return ret;
+}
+
+
+static int
lxcDomainSetBlkioParameters(virDomainPtr dom,
virTypedParameterPtr params,
int nparams,
@@ -5495,6 +5689,8 @@ static virDriver lxcDriver = {
.domainGetSchedulerParametersFlags = lxcDomainGetSchedulerParametersFlags, /* 0.9.2
*/
.domainSetSchedulerParameters = lxcDomainSetSchedulerParameters, /* 0.5.0 */
.domainSetSchedulerParametersFlags = lxcDomainSetSchedulerParametersFlags, /* 0.9.2
*/
+ .domainBlockStats = lxcDomainBlockStats, /* 1.2.2 */
+ .domainBlockStatsFlags = lxcDomainBlockStatsFlags, /* 1.2.2 */
.domainInterfaceStats = lxcDomainInterfaceStats, /* 0.7.3 */
.domainMemoryStats = lxcDomainMemoryStats, /* 1.2.2 */
.nodeGetCPUStats = lxcNodeGetCPUStats, /* 0.9.3 */
--
1.8.4
5:46 a.m.
New subject: [libvirt] [PATCHv3 05/10] Implement lxcDomainBlockStats* for lxc driver
On 03.02.2014 18:44, Thorsten Behrens wrote:
Adds lxcDomainBlockStatsFlags and lxcDomainBlockStats functions.
---
Notes v3:
- merged patch, both api methods now added in one
- addressed comments from v2 review
- check for cgroup controllers actually being mounted
src/lxc/lxc_driver.c | 196 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 196 insertions(+)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 39cd981..103352c 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -75,6 +75,7 @@
#define LXC_NB_MEM_PARAM 3
+#define LXC_NB_DOMAIN_BLOCK_STAT_PARAM 4
static int lxcStateInitialize(bool privileged,
@@ -2202,6 +2203,199 @@ lxcDomainMergeBlkioDevice(virBlkioDevicePtr *dest_array,
static int
+lxcDomainBlockStats(virDomainPtr dom,
+ const char *path,
+ struct _virDomainBlockStats *stats)
+{
+ int ret = -1, idx;
+ virDomainObjPtr vm;
+ virDomainDiskDefPtr disk = NULL;
+ virLXCDomainObjPrivatePtr priv;
+
+ if (!(vm = lxcDomObjFromDomain(dom)))
+ return ret;
+
+ priv = vm->privateData;
+
+ if (virDomainBlockStatsEnsureACL(dom->conn, vm->def) < 0)
+ goto cleanup;
+
+ if (!virDomainObjIsActive(vm)) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ "%s", _("domain is not running"));
+ goto cleanup;
+ }
+
+ if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("blkio cgroup isn't mounted"));
+ goto cleanup;
+ }
+
+ if (!*path) {
+ /* empty path - return entire domain blkstats instead */
+ ret = virCgroupGetBlkioIoServiced(priv->cgroup,
+ &stats->rd_bytes,
+ &stats->wr_bytes,
+ &stats->rd_req,
+ &stats->wr_req);
+ goto cleanup;
+ }
This is interesting. In the qemu driver we don't allow this, and neither
do we in the public API (well, at least in API documentation), nor
virsh. I'm okay with this as long as we relax these restrictions in the
places mentioned earlier.
+
+ if ((idx = virDomainDiskIndexByName(vm->def, path, false)) < 0) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("invalid path: %s"), path);
+ goto cleanup;
+ }
+ disk = vm->def->disks[idx];
+
+ if (!disk->info.alias) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("missing disk device alias name for %s"),
disk->dst);
+ goto cleanup;
+ }
+
+ ret = virCgroupGetBlkioIoDeviceServiced(priv->cgroup,
+ disk->info.alias,
+ &stats->rd_bytes,
+ &stats->wr_bytes,
+ &stats->rd_req,
+ &stats->wr_req);
+cleanup:
+ if (vm)
+ virObjectUnlock(vm);
+ return ret;
+}
+
+
+static int
+lxcDomainBlockStatsFlags(virDomainPtr dom,
+ const char * path,
+ virTypedParameterPtr params,
+ int * nparams,
+ unsigned int flags)
+{
+ int tmp, ret = -1, idx;
+ virDomainObjPtr vm;
+ virDomainDiskDefPtr disk = NULL;
+ virLXCDomainObjPrivatePtr priv;
+ long long rd_req, rd_bytes, wr_req, wr_bytes;
+ virTypedParameterPtr param;
+
+ virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
+
+ /* We don't return strings, and thus trivially support this flag. */
+ flags &= ~VIR_TYPED_PARAM_STRING_OKAY;
+
+ if (!params && !*nparams) {
+ *nparams = LXC_NB_DOMAIN_BLOCK_STAT_PARAM;
+ return 0;
+ }
+
+ if (!(vm = lxcDomObjFromDomain(dom)))
+ return ret;
+
+ priv = vm->privateData;
+
+ if (virDomainBlockStatsFlagsEnsureACL(dom->conn, vm->def) < 0)
+ goto cleanup;
+
+ if (!virDomainObjIsActive(vm)) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ "%s", _("domain is not running"));
+ goto cleanup;
+ }
+
+ if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("blkio cgroup isn't mounted"));
+ goto cleanup;
+ }
+
+ if (!*path) {
+ /* empty path - return entire domain blkstats instead */
+ if (virCgroupGetBlkioIoServiced(priv->cgroup,
+ &rd_bytes,
+ &wr_bytes,
+ &rd_req,
+ &wr_req) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("domain stats query failed"));
+ goto cleanup;
+ }
+ }
+ else {
We tend to write this as '} else {'
+ if ((idx = virDomainDiskIndexByName(vm->def, path,
false)) < 0) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("invalid path: %s"), path);
+ goto cleanup;
+ }
+ disk = vm->def->disks[idx];
+
+ if (!disk->info.alias) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("missing disk device alias name for %s"),
disk->dst);
+ goto cleanup;
+ }
+
+ if (virCgroupGetBlkioIoDeviceServiced(priv->cgroup,
+ disk->info.alias,
+ &rd_bytes,
+ &wr_bytes,
+ &rd_req,
+ &wr_req) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("domain stats query failed"));
+ goto cleanup;
+ }
+ }
+
+ tmp = 0;
+ ret = -1;
+
+ if (tmp < *nparams && wr_bytes != -1) {
+ param = ¶ms[tmp];
+ if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_WRITE_BYTES,
+ VIR_TYPED_PARAM_LLONG, wr_bytes) < 0)
+ goto cleanup;
+ tmp++;
+ }
+
+ if (tmp < *nparams && wr_req != -1) {
+ param = ¶ms[tmp];
+ if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_WRITE_REQ,
+ VIR_TYPED_PARAM_LLONG, wr_req) < 0)
+ goto cleanup;
+ tmp++;
+ }
+
+ if (tmp < *nparams && rd_bytes != -1) {
+ param = ¶ms[tmp];
+ if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_READ_BYTES,
+ VIR_TYPED_PARAM_LLONG, rd_bytes) < 0)
+ goto cleanup;
+ tmp++;
+ }
+
+ if (tmp < *nparams && rd_req != -1) {
+ param = ¶ms[tmp];
+ if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_READ_REQ,
+ VIR_TYPED_PARAM_LLONG, rd_req) < 0)
+ goto cleanup;
+ tmp++;
+ }
+
+ ret = 0;
+ *nparams = tmp;
+
+cleanup:
+ if (vm)
+ virObjectUnlock(vm);
+ return ret;
+}
+
+
+static int
lxcDomainSetBlkioParameters(virDomainPtr dom,
virTypedParameterPtr params,
int nparams,
@@ -5495,6 +5689,8 @@ static virDriver lxcDriver = {
.domainGetSchedulerParametersFlags = lxcDomainGetSchedulerParametersFlags, /* 0.9.2
*/
.domainSetSchedulerParameters = lxcDomainSetSchedulerParameters, /* 0.5.0 */
.domainSetSchedulerParametersFlags = lxcDomainSetSchedulerParametersFlags, /* 0.9.2
*/
+ .domainBlockStats = lxcDomainBlockStats, /* 1.2.2 */
+ .domainBlockStatsFlags = lxcDomainBlockStatsFlags, /* 1.2.2 */
.domainInterfaceStats = lxcDomainInterfaceStats, /* 0.7.3 */
.domainMemoryStats = lxcDomainMemoryStats, /* 1.2.2 */
.nodeGetCPUStats = lxcNodeGetCPUStats, /* 0.9.3 */
Other than that, the patch looks good to me.
Michal
11:44 a.m.
New subject: [libvirt] [PATCHv3 06/10] Widening API change - accept empty path for virDomainBlockStats
And provide domain summary stat in that case, for lxc backend.
Use case is a container inheriting all devices from the host,
e.g. when doing application containerization.
---
src/libvirt.c | 8 ++++++--
tools/virsh-domain-monitor.c | 11 ++++++++---
tools/virsh.pod | 5 +++--
3 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index 9cc5b1c..64da438 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -7747,7 +7747,9 @@ error:
* an unambiguous source name of the block device (the <source
* file='...'/> sub-element, such as "/path/to/image"). Valid
names
* can be found by calling virDomainGetXMLDesc() and inspecting
- * elements within //domain/devices/disk.
+ * elements within //domain/devices/disk. Some drivers might also
+ * accept the empty string for the @disk parameter, and then yield
+ * summary stats for the entire domain.
*
* Domains may have more than one block device. To get stats for
* each you should make multiple calls to this function.
@@ -7813,7 +7815,9 @@ error:
* an unambiguous source name of the block device (the <source
* file='...'/> sub-element, such as "/path/to/image"). Valid
names
* can be found by calling virDomainGetXMLDesc() and inspecting
- * elements within //domain/devices/disk.
+ * elements within //domain/devices/disk. Some drivers might also
+ * accept the empty string for the @disk parameter, and then yield
+ * summary stats for the entire domain.
*
* Domains may have more than one block device. To get stats for
* each you should make multiple calls to this function.
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index de4afbb..105f841 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -888,7 +888,7 @@ static const vshCmdOptDef opts_domblkstat[] = {
},
{.name = "device",
.type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
+ .flags = VSH_OFLAG_EMPTY_OK,
.help = N_("block device")
},
{.name = "human",
@@ -954,8 +954,13 @@ cmdDomblkstat(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
return false;
- if (vshCommandOptStringReq(ctl, cmd, "device", &device) < 0)
- goto cleanup;
+ /* device argument is optional now. if it's missing, supply empty
+ string to denote 'all devices'. A NULL device arg would violate
+ API contract.
+ */
+ rc = vshCommandOptStringReq(ctl, cmd, "device", &device); /* and ignore
rc */
+ if (!device)
+ device = "";
rc = virDomainBlockStatsFlags(dom, device, NULL, &nparams, 0);
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 9e670da..3e45177 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -623,12 +623,13 @@ If I<--graceful> is specified, don't resort to extreme
measures
(e.g. SIGKILL) when the guest doesn't stop after a reasonable timeout;
return an error instead.
-=item B<domblkstat> I<domain> I<block-device> [I<--human>]
+=item B<domblkstat> I<domain> [I<block-device>] [I<--human>]
Get device block stats for a running domain. A I<block-device> corresponds
to a unique target name (<target dev='name'/>) or source file (<source
file='name'/>) for one of the disk devices attached to I<domain> (see
-also B<domblklist> for listing these names).
+also B<domblklist> for listing these names). On a lxc domain, omitting the
+I<block-device> yields device block stats summarily for the entire domain.
Use I<--human> for a more human readable output.
--
1.8.4
5:46 a.m.
New subject: [libvirt] [PATCHv3 06/10] Widening API change - accept empty path for virDomainBlockStats
On 03.02.2014 18:44, Thorsten Behrens wrote:
And provide domain summary stat in that case, for lxc backend.
Use case is a container inheriting all devices from the host,
e.g. when doing application containerization.
---
src/libvirt.c | 8 ++++++--
tools/virsh-domain-monitor.c | 11 ++++++++---
tools/virsh.pod | 5 +++--
3 files changed, 17 insertions(+), 7 deletions(-)
Aah, I didn't see this coming :)
diff --git a/src/libvirt.c b/src/libvirt.c
index 9cc5b1c..64da438 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -7747,7 +7747,9 @@ error:
* an unambiguous source name of the block device (the <source
* file='...'/> sub-element, such as "/path/to/image"). Valid
names
* can be found by calling virDomainGetXMLDesc() and inspecting
- * elements within //domain/devices/disk.
+ * elements within //domain/devices/disk. Some drivers might also
+ * accept the empty string for the @disk parameter, and then yield
+ * summary stats for the entire domain.
*
* Domains may have more than one block device. To get stats for
* each you should make multiple calls to this function.
@@ -7813,7 +7815,9 @@ error:
* an unambiguous source name of the block device (the <source
* file='...'/> sub-element, such as "/path/to/image"). Valid
names
* can be found by calling virDomainGetXMLDesc() and inspecting
- * elements within //domain/devices/disk.
+ * elements within //domain/devices/disk. Some drivers might also
+ * accept the empty string for the @disk parameter, and then yield
+ * summary stats for the entire domain.
*
* Domains may have more than one block device. To get stats for
* each you should make multiple calls to this function.
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index de4afbb..105f841 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -888,7 +888,7 @@ static const vshCmdOptDef opts_domblkstat[] = {
},
{.name = "device",
.type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
+ .flags = VSH_OFLAG_EMPTY_OK,
.help = N_("block device")
},
{.name = "human",
@@ -954,8 +954,13 @@ cmdDomblkstat(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
return false;
- if (vshCommandOptStringReq(ctl, cmd, "device", &device) < 0)
- goto cleanup;
+ /* device argument is optional now. if it's missing, supply empty
+ string to denote 'all devices'. A NULL device arg would violate
+ API contract.
+ */
+ rc = vshCommandOptStringReq(ctl, cmd, "device", &device); /* and
ignore rc */
+ if (!device)
+ device = "";
rc = virDomainBlockStatsFlags(dom, device, NULL, &nparams, 0);
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 9e670da..3e45177 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -623,12 +623,13 @@ If I<--graceful> is specified, don't resort to extreme
measures
(e.g. SIGKILL) when the guest doesn't stop after a reasonable timeout;
return an error instead.
-=item B<domblkstat> I<domain> I<block-device> [I<--human>]
+=item B<domblkstat> I<domain> [I<block-device>] [I<--human>]
Get device block stats for a running domain. A I<block-device> corresponds
to a unique target name (<target dev='name'/>) or source file
(<source
file='name'/>) for one of the disk devices attached to I<domain> (see
-also B<domblklist> for listing these names).
+also B<domblklist> for listing these names). On a lxc domain, omitting the
+I<block-device> yields device block stats summarily for the entire domain.
Use I<--human> for a more human readable output.
ACK to this one and the previous one too then.
Michal
11:44 a.m.
New subject: [libvirt] [PATCHv3 07/10] Add unit test for virCgroupGetBlkioIo*Serviced
---
Adds another dummy device to push the cgroup parsing code a bit
harder
tests/vircgroupmock.c | 107 +++++++++++++++++++++++++++++++++++++++-
tests/vircgrouptest.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 238 insertions(+), 2 deletions(-)
diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c
index 6542973..d772652 100644
--- a/tests/vircgroupmock.c
+++ b/tests/vircgroupmock.c
@@ -34,6 +34,8 @@
static int (*realopen)(const char *path, int flags, ...);
static FILE *(*realfopen)(const char *path, const char *mode);
static int (*realaccess)(const char *path, int mode);
+static int (*realstat)(const char *path, struct stat *sb);
+static int (*real__xstat)(int ver, const char *path, struct stat *sb);
static int (*reallstat)(const char *path, struct stat *sb);
static int (*real__lxstat)(int ver, const char *path, struct stat *sb);
static int (*realmkdir)(const char *path, mode_t mode);
@@ -43,6 +45,8 @@ static int (*realmkdir)(const char *path, mode_t mode);
* vircgroupmock.c:462:22: error: static variable 'fakesysfsdir' is used in an
inline function with external linkage [-Werror,-Wstatic-in-inline]
*/
char *fakesysfsdir;
+char *fakedevicedir0;
+char *fakedevicedir1;
# define SYSFS_PREFIX "/not/really/sys/fs/cgroup/"
@@ -332,13 +336,23 @@ static int make_controller(const char *path, mode_t mode)
"8:0 Write 411440480256\n"
"8:0 Sync 248486822912\n"
"8:0 Async 222495764480\n"
- "8:0 Total 470982587392\n");
+ "8:0 Total 470982587392\n"
+ "9:0 Read 59542107137\n"
+ "9:0 Write 411440480257\n"
+ "9:0 Sync 248486822912\n"
+ "9:0 Async 222495764480\n"
+ "9:0 Total 470982587392\n");
MAKE_FILE("blkio.throttle.io_serviced",
"8:0 Read 4832583\n"
"8:0 Write 36641903\n"
"8:0 Sync 30723171\n"
"8:0 Async 10751315\n"
- "8:0 Total 41474486\n");
+ "8:0 Total 41474486\n"
+ "9:0 Read 4832584\n"
+ "9:0 Write 36641904\n"
+ "9:0 Sync 30723171\n"
+ "9:0 Async 10751315\n"
+ "9:0 Total 41474486\n");
MAKE_FILE("blkio.throttle.read_bps_device", "");
MAKE_FILE("blkio.throttle.read_iops_device", "");
MAKE_FILE("blkio.throttle.write_bps_device", "");
@@ -382,6 +396,7 @@ static void init_syms(void)
LOAD_SYM(fopen);
LOAD_SYM(access);
LOAD_SYM_ALT(lstat, __lxstat);
+ LOAD_SYM_ALT(stat, __xstat);
LOAD_SYM(mkdir);
LOAD_SYM(open);
}
@@ -396,6 +411,16 @@ static void init_sysfs(void)
abort();
}
+ if (!(fakedevicedir0 = getenv("LIBVIRT_FAKE_DEVICE_DIR0"))) {
+ fprintf(stderr, "Missing LIBVIRT_FAKE_DEVICE_DIR0 env variable\n");
+ abort();
+ }
+
+ if (!(fakedevicedir1 = getenv("LIBVIRT_FAKE_DEVICE_DIR1"))) {
+ fprintf(stderr, "Missing LIBVIRT_FAKE_DEVICE_DIR1 env variable\n");
+ abort();
+ }
+
# define MAKE_CONTROLLER(subpath) \
do { \
char *path; \
@@ -529,6 +554,14 @@ int __lxstat(int ver, const char *path, struct stat *sb)
}
ret = real__lxstat(ver, newpath, sb);
free(newpath);
+ } else if (STRPREFIX(path, fakedevicedir0)) {
+ sb->st_mode = S_IFBLK;
+ sb->st_rdev = makedev(8, 0);
+ return 0;
+ } else if (STRPREFIX(path, fakedevicedir1)) {
+ sb->st_mode = S_IFBLK;
+ sb->st_rdev = makedev(9, 0);
+ return 0;
} else {
ret = real__lxstat(ver, path, sb);
}
@@ -552,12 +585,82 @@ int lstat(const char *path, struct stat *sb)
}
ret = reallstat(newpath, sb);
free(newpath);
+ } else if (STRPREFIX(path, fakedevicedir0)) {
+ sb->st_mode = S_IFBLK;
+ sb->st_rdev = makedev(8, 0);
+ return 0;
+ } else if (STRPREFIX(path, fakedevicedir1)) {
+ sb->st_mode = S_IFBLK;
+ sb->st_rdev = makedev(9, 0);
+ return 0;
} else {
ret = reallstat(path, sb);
}
return ret;
}
+int __xstat(int ver, const char *path, struct stat *sb)
+{
+ int ret;
+
+ init_syms();
+
+ if (STRPREFIX(path, SYSFS_PREFIX)) {
+ init_sysfs();
+ char *newpath;
+ if (asprintf(&newpath, "%s/%s",
+ fakesysfsdir,
+ path + strlen(SYSFS_PREFIX)) < 0) {
+ errno = ENOMEM;
+ return -1;
+ }
+ ret = real__xstat(ver, newpath, sb);
+ free(newpath);
+ } else if (STRPREFIX(path, fakedevicedir0)) {
+ sb->st_mode = S_IFBLK;
+ sb->st_rdev = makedev(8, 0);
+ return 0;
+ } else if (STRPREFIX(path, fakedevicedir1)) {
+ sb->st_mode = S_IFBLK;
+ sb->st_rdev = makedev(9, 0);
+ return 0;
+ } else {
+ ret = real__xstat(ver, path, sb);
+ }
+ return ret;
+}
+
+int stat(const char *path, struct stat *sb)
+{
+ int ret;
+
+ init_syms();
+
+ if (STRPREFIX(path, SYSFS_PREFIX)) {
+ init_sysfs();
+ char *newpath;
+ if (asprintf(&newpath, "%s/%s",
+ fakesysfsdir,
+ path + strlen(SYSFS_PREFIX)) < 0) {
+ errno = ENOMEM;
+ return -1;
+ }
+ ret = realstat(newpath, sb);
+ free(newpath);
+ } else if (STRPREFIX(path, fakedevicedir0)) {
+ sb->st_mode = S_IFBLK;
+ sb->st_rdev = makedev(8, 0);
+ return 0;
+ } else if (STRPREFIX(path, fakedevicedir1)) {
+ sb->st_mode = S_IFBLK;
+ sb->st_rdev = makedev(9, 0);
+ return 0;
+ } else {
+ ret = realstat(path, sb);
+ }
+ return ret;
+}
+
int mkdir(const char *path, mode_t mode)
{
int ret;
diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c
index d5ed016..a29cdd2 100644
--- a/tests/vircgrouptest.c
+++ b/tests/vircgrouptest.c
@@ -35,6 +35,9 @@
# define VIR_FROM_THIS VIR_FROM_NONE
+# define FAKEDEVDIR0 "/fakedevdir0/bla/fasl"
+# define FAKEDEVDIR1 "/fakedevdir1/bla/fasl"
+
static int validateCgroup(virCgroupPtr cgroup,
const char *expectPath,
const char **expectMountPoint,
@@ -529,6 +532,128 @@ static int testCgroupAvailable(const void *args)
return 0;
}
+static int testCgroupGetBlkioIoServiced(const void *args ATTRIBUTE_UNUSED)
+{
+ virCgroupPtr cgroup = NULL;
+ size_t i;
+ int rv, ret = -1;
+
+ const long long expected_values[] = {
+ 119084214273,
+ 822880960513,
+ 9665167,
+ 73283807
+ };
+ const char* names[] = {
+ "bytes read",
+ "bytes written",
+ "requests read",
+ "requests written"
+ };
+ long long values[ARRAY_CARDINALITY(expected_values)];
+
+ if ((rv = virCgroupNewPartition("/virtualmachines", true,
+ (1 << VIR_CGROUP_CONTROLLER_BLKIO),
+ &cgroup)) < 0) {
+ fprintf(stderr, "Could not create /virtualmachines cgroup: %d\n",
-rv);
+ goto cleanup;
+ }
+
+ if ((rv = virCgroupGetBlkioIoServiced(cgroup,
+ values, &values[1],
+ &values[2], &values[3])) < 0) {
+ fprintf(stderr, "Could not retrieve BlkioIoServiced for /virtualmachines
cgroup: %d\n", -rv);
+ goto cleanup;
+ }
+
+ for (i = 0; i < ARRAY_CARDINALITY(expected_values); i++) {
+ if (expected_values[i] != values[i]) {
+ fprintf(stderr,
+ "Wrong value for %s from virCgroupBlkioIoServiced (expected
%lld)\n",
+ names[i], expected_values[i]);
+ goto cleanup;
+ }
+ }
+
+ ret = 0;
+
+cleanup:
+ virCgroupFree(&cgroup);
+ return ret;
+}
+
+static int testCgroupGetBlkioIoDeviceServiced(const void *args ATTRIBUTE_UNUSED)
+{
+ virCgroupPtr cgroup = NULL;
+ size_t i;
+ int rv, ret = -1;
+ const long long expected_values0[] = {
+ 59542107136,
+ 411440480256,
+ 4832583,
+ 36641903
+ };
+ const long long expected_values1[] = {
+ 59542107137,
+ 411440480257,
+ 4832584,
+ 36641904
+ };
+ const char* names[] = {
+ "bytes read",
+ "bytes written",
+ "requests read",
+ "requests written"
+ };
+ long long values[ARRAY_CARDINALITY(expected_values0)];
+
+ if ((rv = virCgroupNewPartition("/virtualmachines", true,
+ (1 << VIR_CGROUP_CONTROLLER_BLKIO),
+ &cgroup)) < 0) {
+ fprintf(stderr, "Could not create /virtualmachines cgroup: %d\n",
-rv);
+ goto cleanup;
+ }
+
+ if ((rv = virCgroupGetBlkioIoDeviceServiced(cgroup,
+ FAKEDEVDIR0,
+ values, &values[1],
+ &values[2], &values[3])) < 0)
{
+ fprintf(stderr, "Could not retrieve BlkioIoDeviceServiced for
/virtualmachines cgroup: %d\n", -rv);
+ goto cleanup;
+ }
+
+ for (i = 0; i < ARRAY_CARDINALITY(expected_values0); i++) {
+ if (expected_values0[i] != values[i]) {
+ fprintf(stderr,
+ "Wrong value for %s from virCgroupGetBlkioIoDeviceServiced
(expected %lld)\n",
+ names[i], expected_values0[i]);
+ goto cleanup;
+ }
+ }
+
+ if ((rv = virCgroupGetBlkioIoDeviceServiced(cgroup,
+ FAKEDEVDIR1,
+ values, &values[1],
+ &values[2], &values[3])) < 0)
{
+ fprintf(stderr, "Could not retrieve BlkioIoDeviceServiced for
/virtualmachines cgroup: %d\n", -rv);
+ goto cleanup;
+ }
+
+ for (i = 0; i < ARRAY_CARDINALITY(expected_values1); i++) {
+ if (expected_values1[i] != values[i]) {
+ fprintf(stderr,
+ "Wrong value for %s from virCgroupGetBlkioIoDeviceServiced
(expected %lld)\n",
+ names[i], expected_values1[i]);
+ goto cleanup;
+ }
+ }
+
+ ret = 0;
+
+cleanup:
+ virCgroupFree(&cgroup);
+ return ret;
+}
# define FAKESYSFSDIRTEMPLATE abs_builddir "/fakesysfsdir-XXXXXX"
@@ -549,6 +674,8 @@ mymain(void)
}
setenv("LIBVIRT_FAKE_SYSFS_DIR", fakesysfsdir, 1);
+ setenv("LIBVIRT_FAKE_DEVICE_DIR0", FAKEDEVDIR0, 1);
+ setenv("LIBVIRT_FAKE_DEVICE_DIR1", FAKEDEVDIR1, 1);
if (virtTestRun("New cgroup for self", testCgroupNewForSelf, NULL) < 0)
ret = -1;
@@ -571,6 +698,12 @@ mymain(void)
if (virtTestRun("Cgroup available", testCgroupAvailable, (void*)0x1) <
0)
ret = -1;
+ if (virtTestRun("virCgroupGetBlkioIoServiced works",
testCgroupGetBlkioIoServiced, NULL) < 0)
+ ret = -1;
+
+ if (virtTestRun("virCgroupGetBlkioIoDeviceServiced works",
testCgroupGetBlkioIoDeviceServiced, NULL) < 0)
+ ret = -1;
+
setenv("VIR_CGROUP_MOCK_MODE", "allinone", 1);
if (virtTestRun("New cgroup for self (allinone)",
testCgroupNewForSelfAllInOne, NULL) < 0)
ret = -1;
--
1.8.4
5:46 a.m.
New subject: [libvirt] [PATCHv3 07/10] Add unit test for virCgroupGetBlkioIo*Serviced
On 03.02.2014 18:44, Thorsten Behrens wrote:
---
Adds another dummy device to push the cgroup parsing code a bit
harder
tests/vircgroupmock.c | 107 +++++++++++++++++++++++++++++++++++++++-
tests/vircgrouptest.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 238 insertions(+), 2 deletions(-)
diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c
index 6542973..d772652 100644
--- a/tests/vircgroupmock.c
+++ b/tests/vircgroupmock.c
@@ -34,6 +34,8 @@
static int (*realopen)(const char *path, int flags, ...);
static FILE *(*realfopen)(const char *path, const char *mode);
static int (*realaccess)(const char *path, int mode);
+static int (*realstat)(const char *path, struct stat *sb);
+static int (*real__xstat)(int ver, const char *path, struct stat *sb);
static int (*reallstat)(const char *path, struct stat *sb);
static int (*real__lxstat)(int ver, const char *path, struct stat *sb);
static int (*realmkdir)(const char *path, mode_t mode);
@@ -43,6 +45,8 @@ static int (*realmkdir)(const char *path, mode_t mode);
* vircgroupmock.c:462:22: error: static variable 'fakesysfsdir' is used in an
inline function with external linkage [-Werror,-Wstatic-in-inline]
*/
char *fakesysfsdir;
+char *fakedevicedir0;
+char *fakedevicedir1;
# define SYSFS_PREFIX "/not/really/sys/fs/cgroup/"
@@ -332,13 +336,23 @@ static int make_controller(const char *path, mode_t mode)
"8:0 Write 411440480256\n"
"8:0 Sync 248486822912\n"
"8:0 Async 222495764480\n"
- "8:0 Total 470982587392\n");
+ "8:0 Total 470982587392\n"
+ "9:0 Read 59542107137\n"
+ "9:0 Write 411440480257\n"
+ "9:0 Sync 248486822912\n"
+ "9:0 Async 222495764480\n"
+ "9:0 Total 470982587392\n");
MAKE_FILE("blkio.throttle.io_serviced",
"8:0 Read 4832583\n"
"8:0 Write 36641903\n"
"8:0 Sync 30723171\n"
"8:0 Async 10751315\n"
- "8:0 Total 41474486\n");
+ "8:0 Total 41474486\n"
+ "9:0 Read 4832584\n"
+ "9:0 Write 36641904\n"
+ "9:0 Sync 30723171\n"
+ "9:0 Async 10751315\n"
+ "9:0 Total 41474486\n");
MAKE_FILE("blkio.throttle.read_bps_device", "");
MAKE_FILE("blkio.throttle.read_iops_device", "");
MAKE_FILE("blkio.throttle.write_bps_device", "");
@@ -382,6 +396,7 @@ static void init_syms(void)
LOAD_SYM(fopen);
LOAD_SYM(access);
LOAD_SYM_ALT(lstat, __lxstat);
+ LOAD_SYM_ALT(stat, __xstat);
LOAD_SYM(mkdir);
LOAD_SYM(open);
}
@@ -396,6 +411,16 @@ static void init_sysfs(void)
abort();
}
+ if (!(fakedevicedir0 = getenv("LIBVIRT_FAKE_DEVICE_DIR0"))) {
+ fprintf(stderr, "Missing LIBVIRT_FAKE_DEVICE_DIR0 env variable\n");
+ abort();
+ }
+
+ if (!(fakedevicedir1 = getenv("LIBVIRT_FAKE_DEVICE_DIR1"))) {
+ fprintf(stderr, "Missing LIBVIRT_FAKE_DEVICE_DIR1 env variable\n");
+ abort();
+ }
+
# define MAKE_CONTROLLER(subpath) \
do { \
char *path; \
@@ -529,6 +554,14 @@ int __lxstat(int ver, const char *path, struct stat *sb)
}
ret = real__lxstat(ver, newpath, sb);
free(newpath);
+ } else if (STRPREFIX(path, fakedevicedir0)) {
+ sb->st_mode = S_IFBLK;
+ sb->st_rdev = makedev(8, 0);
+ return 0;
+ } else if (STRPREFIX(path, fakedevicedir1)) {
+ sb->st_mode = S_IFBLK;
+ sb->st_rdev = makedev(9, 0);
+ return 0;
} else {
ret = real__lxstat(ver, path, sb);
}
This won't work. If one __lxstat, lstat, __xstat, stat is called over path that does
not start with SYSFS_PREFIX, in fact it has not been called with such prefix yet, then the
fakedevicedir0 and fakedevicedir1 are NULL and hence strncmp will SIGSEGV:
Program received signal SIGSEGV, Segmentation fault.
__strlen_sse2_pminub () at ../sysdeps/x86_64/multiarch/strlen-sse2-pminub.S:38
38 ../sysdeps/x86_64/multiarch/strlen-sse2-pminub.S: No such file or directory.
(gdb) bt
#0 __strlen_sse2_pminub () at ../sysdeps/x86_64/multiarch/strlen-sse2-pminub.S:38
#1 0x00007ffff7df8f79 in __xstat (ver=1, path=0x6215a0 "/etc/libnl/classid",
sb=0x7fffffffd7e0) at vircgroupmock.c:619
#2 0x0000003468625627 in rtnl_tc_read_classid_file () from
/usr/lib64/libnl-route-3.so.200
#3 0x0000003468617e99 in ?? () from /usr/lib64/libnl-route-3.so.200
#4 0x000000345e80ec2a in call_init (l=<optimized out>, argc=1, argv=0x7fffffffda38,
env=0x7fffffffda48) at dl-init.c:78
#5 0x000000345e80ecfc in _dl_init (main_map=0x345ea231a8, argc=1, argv=0x7fffffffda38,
env=0x7fffffffda48) at dl-init.c:127
#6 0x000000345e80152a in _dl_start_user () from /lib64/ld-linux-x86-64.so.2
#7 0x0000000000000001 in ?? ()
#8 0x00007fffffffde35 in ?? ()
#9 0x0000000000000000 in ?? ()
(gdb) p fakedevicedir0
$5 = 0x0
(gdb) p fakedevicedir1
$6 = 0x0
Michal
11:44 a.m.
New subject: [libvirt] [PATCHv3 08/10] Add unit test for virCgroupGetMemoryUsage.
---
tests/vircgrouptest.c | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c
index a29cdd2..6826442 100644
--- a/tests/vircgrouptest.c
+++ b/tests/vircgrouptest.c
@@ -532,6 +532,38 @@ static int testCgroupAvailable(const void *args)
return 0;
}
+static int testCgroupGetMemoryUsage(const void *args ATTRIBUTE_UNUSED)
+{
+ virCgroupPtr cgroup = NULL;
+ int rv, ret = -1;
+ unsigned long kb;
+
+ if ((rv = virCgroupNewPartition("/virtualmachines", true,
+ (1 << VIR_CGROUP_CONTROLLER_MEMORY),
+ &cgroup)) < 0) {
+ fprintf(stderr, "Could not create /virtualmachines cgroup: %d\n",
-rv);
+ goto cleanup;
+ }
+
+ if ((rv = virCgroupGetMemoryUsage(cgroup, &kb)) < 0) {
+ fprintf(stderr, "Could not retrieve GetMemoryUsage for /virtualmachines
cgroup: %d\n", -rv);
+ goto cleanup;
+ }
+
+ if (kb != 1421212UL) {
+ fprintf(stderr,
+ "Wrong value from virCgroupGetMemoryUsage (expected %ld)\n",
+ 1421212UL);
+ goto cleanup;
+ }
+
+ ret = 0;
+
+cleanup:
+ virCgroupFree(&cgroup);
+ return ret;
+}
+
static int testCgroupGetBlkioIoServiced(const void *args ATTRIBUTE_UNUSED)
{
virCgroupPtr cgroup = NULL;
@@ -704,6 +736,9 @@ mymain(void)
if (virtTestRun("virCgroupGetBlkioIoDeviceServiced works",
testCgroupGetBlkioIoDeviceServiced, NULL) < 0)
ret = -1;
+ if (virtTestRun("virCgroupGetMemoryUsage works", testCgroupGetMemoryUsage,
NULL) < 0)
+ ret = -1;
+
setenv("VIR_CGROUP_MOCK_MODE", "allinone", 1);
if (virtTestRun("New cgroup for self (allinone)",
testCgroupNewForSelfAllInOne, NULL) < 0)
ret = -1;
--
1.8.4
5:46 a.m.
New subject: [libvirt] [PATCHv3 08/10] Add unit test for virCgroupGetMemoryUsage.
On 03.02.2014 18:44, Thorsten Behrens wrote:
---
tests/vircgrouptest.c | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c
index a29cdd2..6826442 100644
--- a/tests/vircgrouptest.c
+++ b/tests/vircgrouptest.c
@@ -532,6 +532,38 @@ static int testCgroupAvailable(const void *args)
return 0;
}
+static int testCgroupGetMemoryUsage(const void *args ATTRIBUTE_UNUSED)
+{
+ virCgroupPtr cgroup = NULL;
+ int rv, ret = -1;
+ unsigned long kb;
+
+ if ((rv = virCgroupNewPartition("/virtualmachines", true,
+ (1 << VIR_CGROUP_CONTROLLER_MEMORY),
+ &cgroup)) < 0) {
+ fprintf(stderr, "Could not create /virtualmachines cgroup: %d\n",
-rv);
+ goto cleanup;
+ }
+
+ if ((rv = virCgroupGetMemoryUsage(cgroup, &kb)) < 0) {
+ fprintf(stderr, "Could not retrieve GetMemoryUsage for /virtualmachines
cgroup: %d\n", -rv);
+ goto cleanup;
+ }
+
+ if (kb != 1421212UL) {
+ fprintf(stderr,
+ "Wrong value from virCgroupGetMemoryUsage (expected %ld)\n",
+ 1421212UL);
+ goto cleanup;
+ }
+
+ ret = 0;
+
+cleanup:
+ virCgroupFree(&cgroup);
+ return ret;
+}
+
static int testCgroupGetBlkioIoServiced(const void *args ATTRIBUTE_UNUSED)
{
virCgroupPtr cgroup = NULL;
@@ -704,6 +736,9 @@ mymain(void)
if (virtTestRun("virCgroupGetBlkioIoDeviceServiced works",
testCgroupGetBlkioIoDeviceServiced, NULL) < 0)
ret = -1;
+ if (virtTestRun("virCgroupGetMemoryUsage works", testCgroupGetMemoryUsage,
NULL) < 0)
+ ret = -1;
+
setenv("VIR_CGROUP_MOCK_MODE", "allinone", 1);
if (virtTestRun("New cgroup for self (allinone)",
testCgroupNewForSelfAllInOne, NULL) < 0)
ret = -1;
ACK
Michal
11:44 a.m.
New subject: [libvirt] [PATCHv3 09/10] Fix misspelled cpuacct.usage_percpu in cgroup mock.
---
Turned out during test writing, when this was actually used the
first time.
tests/vircgroupmock.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c
index d772652..84bb1b0 100644
--- a/tests/vircgroupmock.c
+++ b/tests/vircgroupmock.c
@@ -214,7 +214,7 @@ static int make_controller(const char *path, mode_t mode)
"user 216687025\n"
"system 43421396\n");
MAKE_FILE("cpuacct.usage", "2787788855799582\n");
- MAKE_FILE("cpuacct.usage_per_cpu", "1413142688153030
1374646168910542\n");
+ MAKE_FILE("cpuacct.usage_percpu", "1413142688153030
1374646168910542\n");
} else if (STRPREFIX(controller, "cpuset")) {
MAKE_FILE("cpuset.cpu_exclusive", "1\n");
if (STREQ(controller, "cpuset"))
--
1.8.4
5:46 a.m.
New subject: [libvirt] [PATCHv3 09/10] Fix misspelled cpuacct.usage_percpu in cgroup mock.
On 03.02.2014 18:44, Thorsten Behrens wrote:
---
Turned out during test writing, when this was actually used the
first time.
tests/vircgroupmock.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c
index d772652..84bb1b0 100644
--- a/tests/vircgroupmock.c
+++ b/tests/vircgroupmock.c
@@ -214,7 +214,7 @@ static int make_controller(const char *path, mode_t mode)
"user 216687025\n"
"system 43421396\n");
MAKE_FILE("cpuacct.usage", "2787788855799582\n");
- MAKE_FILE("cpuacct.usage_per_cpu", "1413142688153030
1374646168910542\n");
+ MAKE_FILE("cpuacct.usage_percpu", "1413142688153030
1374646168910542\n");
} else if (STRPREFIX(controller, "cpuset")) {
MAKE_FILE("cpuset.cpu_exclusive", "1\n");
if (STREQ(controller, "cpuset"))
ACK
Michal
11:44 a.m.
New subject: [libvirt] [PATCHv3 10/10] Add unit test for virCgroupGetPercpuStats.
---
tests/vircgrouptest.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 66 insertions(+)
diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c
index 6826442..dfcb0aa 100644
--- a/tests/vircgrouptest.c
+++ b/tests/vircgrouptest.c
@@ -32,6 +32,7 @@
# include "virerror.h"
# include "virlog.h"
# include "virfile.h"
+# include "nodeinfo.h"
# define VIR_FROM_THIS VIR_FROM_NONE
@@ -532,6 +533,68 @@ static int testCgroupAvailable(const void *args)
return 0;
}
+static int testCgroupGetPercpuStats(const void *args ATTRIBUTE_UNUSED)
+{
+ virCgroupPtr cgroup = NULL;
+ size_t i;
+ int rv, ret = -1;
+ virTypedParameter params[2];
+
+ // TODO: mock nodeGetCPUCount() as well & check 2nd cpu, too
+ unsigned long long expected[] = {
+ 1413142688153030
+ };
+
+ if ((rv = virCgroupNewPartition("/virtualmachines", true,
+ (1 << VIR_CGROUP_CONTROLLER_CPU) |
+ (1 << VIR_CGROUP_CONTROLLER_CPUACCT),
+ &cgroup)) < 0) {
+ fprintf(stderr, "Could not create /virtualmachines cgroup: %d\n",
-rv);
+ goto cleanup;
+ }
+
+ if (nodeGetCPUCount() < 1) {
+ fprintf(stderr, "Unexpected: nodeGetCPUCount() yields: %d\n",
nodeGetCPUCount());
+ goto cleanup;
+ }
+
+ if ((rv = virCgroupGetPercpuStats(cgroup,
+ params,
+ 2, 0, 1)) < 0) {
+ fprintf(stderr, "Failed call to virCgroupGetPercpuStats for /virtualmachines
cgroup: %d\n", -rv);
+ goto cleanup;
+ }
+
+ for (i = 0; i < ARRAY_CARDINALITY(expected); i++) {
+ if (!STREQ(params[i].field, VIR_DOMAIN_CPU_STATS_CPUTIME)) {
+ fprintf(stderr,
+ "Wrong parameter name value from virCgroupGetPercpuStats (is:
%s)\n",
+ params[i].field);
+ goto cleanup;
+ }
+
+ if (params[i].type != VIR_TYPED_PARAM_ULLONG) {
+ fprintf(stderr,
+ "Wrong parameter value type from virCgroupGetPercpuStats (is:
%d)\n",
+ params[i].type);
+ goto cleanup;
+ }
+
+ if (params[i].value.ul != expected[i]) {
+ fprintf(stderr,
+ "Wrong value from virCgroupGetMemoryUsage (expected
%llu)\n",
+ params[i].value.ul);
+ goto cleanup;
+ }
+ }
+
+ ret = 0;
+
+cleanup:
+ virCgroupFree(&cgroup);
+ return ret;
+}
+
static int testCgroupGetMemoryUsage(const void *args ATTRIBUTE_UNUSED)
{
virCgroupPtr cgroup = NULL;
@@ -739,6 +802,9 @@ mymain(void)
if (virtTestRun("virCgroupGetMemoryUsage works", testCgroupGetMemoryUsage,
NULL) < 0)
ret = -1;
+ if (virtTestRun("virCgroupGetPercpuStats works", testCgroupGetPercpuStats,
NULL) < 0)
+ ret = -1;
+
setenv("VIR_CGROUP_MOCK_MODE", "allinone", 1);
if (virtTestRun("New cgroup for self (allinone)",
testCgroupNewForSelfAllInOne, NULL) < 0)
ret = -1;
--
1.8.4
5:46 a.m.
New subject: [libvirt] [PATCHv3 10/10] Add unit test for virCgroupGetPercpuStats.
On 03.02.2014 18:44, Thorsten Behrens wrote:
---
tests/vircgrouptest.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 66 insertions(+)
diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c
index 6826442..dfcb0aa 100644
--- a/tests/vircgrouptest.c
+++ b/tests/vircgrouptest.c
@@ -32,6 +32,7 @@
# include "virerror.h"
# include "virlog.h"
# include "virfile.h"
+# include "nodeinfo.h"
# define VIR_FROM_THIS VIR_FROM_NONE
@@ -532,6 +533,68 @@ static int testCgroupAvailable(const void *args)
return 0;
}
+static int testCgroupGetPercpuStats(const void *args ATTRIBUTE_UNUSED)
+{
+ virCgroupPtr cgroup = NULL;
+ size_t i;
+ int rv, ret = -1;
+ virTypedParameter params[2];
+
+ // TODO: mock nodeGetCPUCount() as well & check 2nd cpu, too
+ unsigned long long expected[] = {
+ 1413142688153030
+ };
+
+ if ((rv = virCgroupNewPartition("/virtualmachines", true,
+ (1 << VIR_CGROUP_CONTROLLER_CPU) |
+ (1 << VIR_CGROUP_CONTROLLER_CPUACCT),
+ &cgroup)) < 0) {
+ fprintf(stderr, "Could not create /virtualmachines cgroup: %d\n",
-rv);
+ goto cleanup;
+ }
+
+ if (nodeGetCPUCount() < 1) {
+ fprintf(stderr, "Unexpected: nodeGetCPUCount() yields: %d\n",
nodeGetCPUCount());
+ goto cleanup;
+ }
+
+ if ((rv = virCgroupGetPercpuStats(cgroup,
+ params,
+ 2, 0, 1)) < 0) {
+ fprintf(stderr, "Failed call to virCgroupGetPercpuStats for
/virtualmachines cgroup: %d\n", -rv);
+ goto cleanup;
+ }
+
+ for (i = 0; i < ARRAY_CARDINALITY(expected); i++) {
+ if (!STREQ(params[i].field, VIR_DOMAIN_CPU_STATS_CPUTIME)) {
+ fprintf(stderr,
+ "Wrong parameter name value from virCgroupGetPercpuStats (is:
%s)\n",
+ params[i].field);
+ goto cleanup;
+ }
+
+ if (params[i].type != VIR_TYPED_PARAM_ULLONG) {
+ fprintf(stderr,
+ "Wrong parameter value type from virCgroupGetPercpuStats (is:
%d)\n",
+ params[i].type);
+ goto cleanup;
+ }
+
+ if (params[i].value.ul != expected[i]) {
+ fprintf(stderr,
+ "Wrong value from virCgroupGetMemoryUsage (expected
%llu)\n",
+ params[i].value.ul);
+ goto cleanup;
+ }
+ }
+
+ ret = 0;
+
+cleanup:
+ virCgroupFree(&cgroup);
+ return ret;
+}
+
static int testCgroupGetMemoryUsage(const void *args ATTRIBUTE_UNUSED)
{
virCgroupPtr cgroup = NULL;
@@ -739,6 +802,9 @@ mymain(void)
if (virtTestRun("virCgroupGetMemoryUsage works",
testCgroupGetMemoryUsage, NULL) < 0)
ret = -1;
+ if (virtTestRun("virCgroupGetPercpuStats works", testCgroupGetPercpuStats,
NULL) < 0)
+ ret = -1;
+
setenv("VIR_CGROUP_MOCK_MODE", "allinone", 1);
if (virtTestRun("New cgroup for self (allinone)",
testCgroupNewForSelfAllInOne, NULL) < 0)
ret = -1;
ACK
Michal
5:45 a.m.
On 03.02.2014 18:44, Thorsten Behrens wrote:
This patch set adds block io, memory and domain cpu statistics API
slot implementations to the LXC driver, in order to get linux
container monitoring and accounting a bit closer to qemu standards.
The last patch is a tad quirky (happy to hear suggestions on
alternative ways), in that it widens the permissible value set
at the .domainBlockStats slot: for lxc guests, it is relatively
likely to have zero disk devices, since host filesystems can be
used via passthrough bind mounts. Therefore, passing the zero-length
string as device path, is interpreted as 'return summary stats for
the entire domains's block io'.
Right. We can add test for both, can't we?
Anyway, I've ACKed most of the patches except a few. Can you please fix
& resend them? I'll do the review and (hopefully) push them too. Nice work!
Michal
1:38 p.m.
I wrote:
This patch set adds block io, memory and domain cpu statistics API
slot implementations to the LXC driver, in order to get linux
container monitoring and accounting a bit closer to qemu standards.
Hi guys,
hmm, this would be cool to get into 1.2.2 - any chance for that? ;)
Some patches were ACKed already, and I've addressed the remaining
review comments on 02, 04 and 07. For 05, I did not modify the qemu
driver, but relaxed the docs in 06 already (there was no explicit
failure mode for that case anyway, in libvirt.c).
Thanks a lot,
-- Thorsten
2:39 p.m.
On Wed, Feb 19, 2014 at 08:38:33PM +0100, Thorsten Behrens wrote:
I wrote:
> This patch set adds block io, memory and domain cpu statistics API
> slot implementations to the LXC driver, in order to get linux
> container monitoring and accounting a bit closer to qemu standards.
>
Hi guys,
hmm, this would be cool to get into 1.2.2 - any chance for that? ;)
Some patches were ACKed already, and I've addressed the remaining
review comments on 02, 04 and 07. For 05, I did not modify the qemu
driver, but relaxed the docs in 06 already (there was no explicit
failure mode for that case anyway, in libvirt.c).
I'll review the rest of this tomorrow if no one beats me to it.
Getting into 1.2.2 is a reasonable target.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
3929
days inactive
3945
days old
23 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Michal Privoznik -
Thorsten Behrens