On 01/15/2014 07:23 AM, Thorsten Behrens wrote:

This reads blkio stats from blkio.throttle.io_service_bytes and blkio.throttle.io_serviced. --- src/libvirt_private.syms | 2 + src/util/vircgroup.c | 208 +++++++++++++++++++++++++++++++++++++++++++++++ src/util/vircgroup.h | 12 +++ 3 files changed, 222 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3b3de15..edbf6ba 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1003,6 +1003,8 @@ virCgroupDenyDevice; virCgroupDenyDeviceMajor; virCgroupDenyDevicePath; virCgroupFree; +virCgroupGetBlkioIoDeviceServiced; +virCgroupGetBlkioIoServiced; virCgroupGetBlkioWeight; virCgroupGetCpuacctPercpuUsage; virCgroupGetCpuacctStat; diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 43eb649..1a579f0 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1826,6 +1826,191 @@ virCgroupGetBlkioWeight(virCgroupPtr group, unsigned int *weight)

/** + * virCgroupGetBlkioIoServiced: + * + * @group: The cgroup to get throughput for + * @kb: Pointer to returned serviced io in kilobytes + * + * 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; + int 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; + + p1 = str1; + p2 = str2; + + /* sum up all entries of the same kind, from all devices */

You only operate the first device below, you need a cycle to operate all the devices.

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

had better report the value_names too.

+ goto cleanup; + } + *bytes_ptrs[i] += stats_val; +

had better add the overflow check and report error.

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

overflow check

+ } + + 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 + * @kb_read: Pointer to serviced read io in kilobytes + * @kb_write: Pointer to serviced write io in kilobytes + * @kb_total: Pointer to serviced io in kilobytes + * + * 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; + int 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; + + for (i = 0; i < ARRAY_CARDINALITY(value_names); i++) { + if (!(p = strstr(str1, str3))) {

Can you move this strstr out of cycle?

+ virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot find state for block device '%s'"), + p); + goto cleanup; + } + 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])); + goto cleanup; + } + + if (!(p = strstr(str2, str3))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot find state for block device '%s'"), + p); + 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])); + goto cleanup; + } + } + + ret = 0; + +cleanup: + VIR_FREE(str3); + VIR_FREE(str2); + VIR_FREE(str1); + return ret; +} + + +/** * virCgroupSetBlkioDeviceWeight: * * @group: The cgroup to change io device weight device for @@ -3303,6 +3488,29 @@ virCgroupGetBlkioWeight(virCgroupPtr group ATTRIBUTE_UNUSED,

int +virCgroupGetBlkioIoServiced(virCgroupPtr group ATTRIBUTE_UNUSED, + unsigned int *kb ATTRIBUTE_UNUSED)

This definition of virCgroupGetBlkioIoServiced is inconsistent.

+{ + virReportSystemError(ENXIO, "%s", + _("Control groups not supported on this platform")); + return -1; +} + + +int +virCgroupGetBlkioIoDeviceServiced(virCgroupPtr group ATTRIBUTE_UNUSED, + const char *path ATTRIBUTE_UNUSED, + unsigned int *kb_read ATTRIBUTE_UNUSED, + unsigned int *kb_write ATTRIBUTE_UNUSED, + unsigned int *kb_total ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENXIO, "%s", + _("Control groups not supported on this platform")); + return -1; +} + + +int virCgroupSetBlkioDeviceWeight(virCgroupPtr group ATTRIBUTE_UNUSED, const char *path ATTRIBUTE_UNUSED, unsigned int weight ATTRIBUTE_UNUSED) diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index 835eb30..cd6b27b 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);

On 01/15/2014 07:23 AM, Thorsten Behrens wrote:

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

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 7e56a59..9f586af 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -4554,6 +4554,57 @@ 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; + int got = 0; + + virCheckFlags(0, -1); + + if (!(vm = lxcDomObjFromDomain(dom))) + goto cleanup; + + priv = vm->privateData; + + if (virDomainMemoryStatsEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; +

You should make sure domain is running and the return value of lxcDomainMemoryStats seem incorrect.

+ if (got < nr_stats) { + stats[got].tag = VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON; + stats[got].val = vm->def->mem.cur_balloon; + got++; + } + if (got < nr_stats) { + stats[got].tag = VIR_DOMAIN_MEMORY_STAT_SWAP_IN; + virCgroupGetMemSwapUsage(priv->cgroup, &stats[got].val); + got++; + } + if (got < nr_stats) { + unsigned long kb; + stats[got].tag = VIR_DOMAIN_MEMORY_STAT_RSS; + virCgroupGetMemoryUsage(priv->cgroup, &kb); + stats[got].val = kb; + ret++; + } + +cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + + +static int lxcNodeGetCPUStats(virConnectPtr conn, int cpuNum, virNodeCPUStatsPtr params, @@ -4781,6 +4832,7 @@ static virDriver lxcDriver = { .domainSetSchedulerParameters = lxcDomainSetSchedulerParameters, /* 0.5.0 */ .domainSetSchedulerParametersFlags = lxcDomainSetSchedulerParametersFlags, /* 0.9.2 */ .domainInterfaceStats = lxcDomainInterfaceStats, /* 0.7.3 */ + .domainMemoryStats = lxcDomainMemoryStats, /* 0.7.5 */

0.7.5 is incorrect, it should be 1.2.1.

.nodeGetCPUStats = lxcNodeGetCPUStats, /* 0.9.3 */ .nodeGetMemoryStats = lxcNodeGetMemoryStats, /* 0.9.3 */ .nodeGetCellsFreeMemory = lxcNodeGetCellsFreeMemory, /* 0.6.5 */

On 01/15/2014 07:23 AM, 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(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index edbf6ba..048d9a0 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1014,6 +1014,7 @@ virCgroupGetCpuCfsQuota; virCgroupGetCpusetCpus; virCgroupGetCpusetMems; virCgroupGetCpuShares; +virCgroupGetDomainTotalCPUStats;

please change virCgroupGetDomainTotalCPUStats to virCgroupGetDomainTotalCpuStats the other part looks good to me.

virCgroupGetFreezerState; virCgroupGetMemoryHardLimit; virCgroupGetMemorySoftLimit; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1949abe..2d92873 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 @@ -15302,56 +15301,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: @@ -15550,7 +15499,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 1a579f0..e02b473 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 @@ -2603,6 +2606,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..f30dbcb 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);

On 01/15/2014 07:23 AM, Thorsten Behrens wrote:

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

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 9f586af..1e9c77a 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,135 @@ 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; + unsigned int n = 0; + 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; + memset(params, 0, nparams * ncpus);

this is unnecessary. we alreay filled params to zero when we allocate it by VIR_ALLOC_N.

+ + /* 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; + } else { + n++;

n is useless.

+ } + 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 +4983,7 @@ static virDriver lxcDriver = { .nodeSuspendForDuration = lxcNodeSuspendForDuration, /* 0.9.8 */ .domainSetMetadata = lxcDomainSetMetadata, /* 1.1.3 */ .domainGetMetadata = lxcDomainGetMetadata, /* 1.1.3 */ + .domainGetCPUStats = lxcDomainGetCPUStats, /* 0.9.11 */

1.2.1 here

On 01/15/2014 07:23 AM, Thorsten Behrens wrote:

--- 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 1e9c77a..1d2a457 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2021,6 +2021,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, @@ -4962,6 +5012,7 @@ static virDriver lxcDriver = { .domainGetSchedulerParametersFlags = lxcDomainGetSchedulerParametersFlags, /* 0.9.2 */ .domainSetSchedulerParameters = lxcDomainSetSchedulerParameters, /* 0.5.0 */ .domainSetSchedulerParametersFlags = lxcDomainSetSchedulerParametersFlags, /* 0.9.2 */ + .domainBlockStats = lxcDomainBlockStats, /* 0.4.1 */

Just one question. Can't we implement the new API domainBlockStatsFlags?

On 01/15/2014 07:23 AM, Thorsten Behrens wrote:

And provide domain summary stats in that case, for lxc backend. Use case is a container domain using passthrough bind mounts of the host filesystem, which is a common case for lxc. --- src/libvirt.c | 1 - src/lxc/lxc_driver.c | 10 ++++++++++ src/qemu/qemu_driver.c | 2 ++ src/remote/remote_driver.c | 2 ++ src/test/test_driver.c | 2 ++ src/xen/xen_driver.c | 2 ++ 6 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/src/libvirt.c b/src/libvirt.c index 87a4d46..14ffca0 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -7804,7 +7804,6 @@ virDomainBlockStats(virDomainPtr dom, const char *disk, virResetLastError();

virCheckDomainReturn(dom, -1); - virCheckNonNullArgGoto(disk, error); virCheckNonNullArgGoto(stats, error); if (size > sizeof(stats2)) { virReportInvalidArg(size, diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 1d2a457..fba9c12 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2044,6 +2044,16 @@ lxcDomainBlockStats(virDomainPtr dom, goto cleanup; }

+ if (!path || !*path) { + /* empty/NULL path - return entire domain blkstats instead */ + ret = virCgroupGetBlkioIoServiced(priv->cgroup, + &stats->rd_bytes, + &stats->wr_bytes, + &stats->rd_req, + &stats->wr_req); + goto cleanup; + } +

I'm ok with this one, Let's see if others will object. but you should check if we can use the NEW API as I mehtioned in prev thread, and change the manpage of virsh domblkstat.

if ((idx = virDomainDiskIndexByName(vm->def, path, false)) < 0) { virReportError(VIR_ERR_INVALID_ARG, _("invalid path: %s"), path); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2d92873..4dcf12b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9021,6 +9021,8 @@ qemuDomainBlockStats(virDomainPtr dom, virDomainDiskDefPtr disk = NULL; qemuDomainObjPrivatePtr priv;

+ virCheckNonNullArgReturn(path, -1); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup;

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index da9c1c9..160bdd4 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1713,6 +1713,8 @@ remoteDomainBlockStatsFlags(virDomainPtr domain, remote_domain_block_stats_flags_ret ret; struct private_data *priv = domain->conn->privateData;

+ virCheckNonNullArgReturn(path, -1); + remoteDriverLock(priv);

make_nonnull_domain(&args.dom, domain); diff --git a/src/test/test_driver.c b/src/test/test_driver.c index b724f82..7c637bb 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -3362,6 +3362,8 @@ static int testDomainBlockStats(virDomainPtr domain, unsigned long long statbase; int ret = -1;

+ virCheckNonNullArgReturn(path, -1); + testDriverLock(privconn); privdom = virDomainObjListFindByName(privconn->domains, domain->name); diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index c45d10f..2b9ac21 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -2217,6 +2217,8 @@ xenUnifiedDomainBlockStats(virDomainPtr dom, const char *path, virDomainDefPtr def = NULL; int ret = -1;

+ virCheckNonNullArgReturn(path, -1); + if (!(def = xenGetDomainDefForDom(dom))) goto cleanup;