On 01/20/2014 08:15 PM, Thorsten Behrens wrote:

This reads blkio stats from blkio.throttle.io_service_bytes and blkio.throttle.io_serviced. ---

Note on v3: - rebased to current master, sadly the virCgroupSetBlkioDeviceReadBps etc conflicted

src/libvirt_private.syms | 2 + src/util/vircgroup.c | 242 +++++++++++++++++++++++++++++++++++++++++++++++ src/util/vircgroup.h | 12 +++ 3 files changed, 256 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3ede3d5..1e44ed8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1002,6 +1002,8 @@ virCgroupDenyDevice; virCgroupDenyDeviceMajor; virCgroupDenyDevicePath; virCgroupFree; +virCgroupGetBlkioIoDeviceServiced; +virCgroupGetBlkioIoServiced; virCgroupGetBlkioWeight; virCgroupGetCpuacctPercpuUsage; virCgroupGetCpuacctStat; diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index a6d60c5..2b70bcb 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1786,6 +1786,221 @@ 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, *p; + 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 (!(p = strstr(str1, str3))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot find byte stats for block device '%s'"), + p);

path?

+ goto cleanup; + } + + if (!(p = strstr(str2, str3))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot find request stats for block device '%s'"), + p);

here too;

+ goto cleanup; + } + + for (i = 0; i < ARRAY_CARDINALITY(value_names); i++) { + if (!(p = strstr(p, value_names[i])) || + virStrToLong_ll(p + strlen(value_names[i]), &p, 10, bytes_ptrs[i]) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot parse stat '%s'"), + p + strlen(value_names[i]));

p may null here.

+ goto cleanup; + } + + if (!(p = strstr(p, value_names[i])) || + virStrToLong_ll(p + strlen(value_names[i]), &p, 10, requests_ptrs[i]) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot parse stat '%s'"), + p + strlen(value_names[i]));

here too; ACK with these changed.

On 01/20/2014 07:12 PM, Thorsten Behrens wrote:

---

Notes on v2: - check if domain is running, fixed ret val calculation - api slot comment is now referencing 1.2.2

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 5ae4b65..8cf8e48 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -4556,6 +4556,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, @@ -4783,6 +4832,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 */

looks good to me ACK thanks!

On 01/20/2014 07:12 PM, Thorsten Behrens wrote:

To reuse this from other drivers, like lxc. ---

Notes on v2: - renamed to proper camel case: virCgroupGetDomainTotalCpuStats

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 e4815b3..97198d1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1013,6 +1013,7 @@ virCgroupGetCpuCfsQuota; virCgroupGetCpusetCpus; virCgroupGetCpusetMems; virCgroupGetCpuShares; +virCgroupGetDomainTotalCpuStats; virCgroupGetFreezerState; virCgroupGetMemoryHardLimit; virCgroupGetMemorySoftLimit; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ebb77dc..a1e45c3 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 @@ -15304,56 +15303,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: @@ -15552,7 +15501,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 83fcefc..66045fb 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 @@ -2629,6 +2632,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 cd6b27b..b42bddc 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -185,6 +185,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);

Just rename and move, ACK thanks!

On 01/20/2014 07:12 PM, Thorsten Behrens wrote:

---

Notes on v2: - elided extra memset and leftover loop var n - api slot comment references 1.2.2 now

src/lxc/lxc_driver.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 128 insertions(+)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 8cf8e48..19426f5 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -75,6 +75,8 @@

#define LXC_NB_MEM_PARAM 3 +#define LXC_NB_PER_CPU_STAT_PARAM 1 +

static int lxcStateInitialize(bool privileged, virStateInhibitCallback callback, @@ -4775,6 +4777,131 @@ cleanup: }

+static int +lxcDomainGetPercpuStats(virDomainObjPtr vm, + 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; + virLXCDomainObjPrivatePtr priv = vm->privateData; + virTypedParameterPtr ent; + int param_idx; + unsigned long long cpu_time; + + /* TODO: share api contract code with other drivers here */ + + /* return the number of supported params */ + if (nparams == 0 && ncpus != 0) + return LXC_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(priv->cgroup, &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; +} + + +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 = lxcDomainGetPercpuStats(vm, params, nparams, + start_cpu, ncpus); +cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + + /* Function Tables */ static virDriver lxcDriver = { .no = VIR_DRV_LXC, @@ -4852,6 +4979,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 */

Looks good to me ACK thanks!

On 01/20/2014 07:12 PM, Thorsten Behrens wrote:

---

Notes on v2: - works as-is, will send lxcDomainBlockStatsFlags patch separately

src/lxc/lxc_driver.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 19426f5..bf6fd5c 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2023,6 +2023,56 @@ lxcDomainGetSchedulerParameters(virDomainPtr domain,

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 ((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 lxcDomainSetBlkioParameters(virDomainPtr dom, virTypedParameterPtr params, int nparams, @@ -4958,6 +5008,7 @@ static virDriver lxcDriver = { .domainGetSchedulerParametersFlags = lxcDomainGetSchedulerParametersFlags, /* 0.9.2 */ .domainSetSchedulerParameters = lxcDomainSetSchedulerParameters, /* 0.5.0 */ .domainSetSchedulerParametersFlags = lxcDomainSetSchedulerParametersFlags, /* 0.9.2 */ + .domainBlockStats = lxcDomainBlockStats, /* 0.4.1 */

this should be 1.2.2

.domainInterfaceStats = lxcDomainInterfaceStats, /* 0.7.3 */ .domainMemoryStats = lxcDomainMemoryStats, /* 1.2.2 */ .nodeGetCPUStats = lxcNodeGetCPUStats, /* 0.9.3 */

ACK thanks!

On 01/20/2014 07:12 PM, 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. ---

Notes on v2: - adapted virDomainBlockStats docs - adapted virsh domblkstat docs - made virsh actually accept empty disk argument - pedaling back a bit on the API change - accepting a NULL ptr for the disk arg would need changing the remote protocol, so better just take the empty string. Makes this less invasive even.

src/libvirt.c | 8 ++++++-- src/lxc/lxc_driver.c | 10 ++++++++++ tools/virsh-domain-monitor.c | 11 ++++++++--- tools/virsh.pod | 5 +++-- 4 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/src/libvirt.c b/src/libvirt.c index 87a4d46..ead0813 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -7781,7 +7781,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. @@ -7847,7 +7849,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/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index bf6fd5c..31f1625 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2046,6 +2046,16 @@ lxcDomainBlockStats(virDomainPtr dom, 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); diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index b29b82a..6be253f 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -880,7 +880,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", @@ -946,8 +946,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 3534b54..c3ca016 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -616,12 +616,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.

Looks good to me ACK thanks!