[libvirt] [PATCHv4 00/10] Add BlkIO and CPU/mem stat API implementations for lxc
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'. v4 addresses the last remaining review comments. 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 | 300 +++++++++++++++++++++++++++++++++ src/qemu/qemu_driver.c | 54 +----- src/util/vircgroup.c | 382 +++++++++++++++++++++++++++++++++++++++++++ src/util/vircgroup.h | 24 +++ tests/testutilslxc.h | 3 + tests/vircgroupmock.c | 100 ++++++++++- tests/vircgrouptest.c | 230 ++++++++++++++++++++++++++ tools/virsh-domain-monitor.c | 11 +- tools/virsh.pod | 5 +- 11 files changed, 1059 insertions(+), 62 deletions(-) -- 1.8.4.5
This reads blkio stats from blkio.throttle.io_service_bytes and blkio.throttle.io_serviced. --- 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 0b28bac..88a1a89 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1007,6 +1007,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
On Fri, Feb 14, 2014 at 06:48:59PM +0100, 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 | 254 +++++++++++++++++++++++++++++++++++++++++++++++ src/util/vircgroup.h | 12 +++ 3 files changed, 268 insertions(+)
ACK 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 :|
--- Notes to v4: - check errors before filling param array - UL->ULL change punted to separate patch src/lxc/lxc_driver.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index f735631..827d989 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -5197,6 +5197,61 @@ 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; + unsigned long long swap_usage; + unsigned long mem_usage; + + virCheckFlags(0, -1); + + if (!(vm = lxcDomObjFromDomain(dom))) + goto cleanup; + + priv = vm->privateData; + + if (virDomainMemoryStatsEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (!virCgroupGetMemSwapUsage(priv->cgroup, &swap_usage)) + goto cleanup; + + if (!virCgroupGetMemoryUsage(priv->cgroup, &mem_usage)) + 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; + stats[ret].val = swap_usage; + ret++; + } + if (ret < nr_stats) { + stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_RSS; + stats[ret].val = mem_usage; + ret++; + } + +cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + + +static int lxcNodeGetCPUStats(virConnectPtr conn, int cpuNum, virNodeCPUStatsPtr params, @@ -5426,6 +5481,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
On 14.02.2014 18:49, Thorsten Behrens wrote:
--- Notes to v4: - check errors before filling param array - UL->ULL change punted to separate patch
src/lxc/lxc_driver.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index f735631..827d989 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -5197,6 +5197,61 @@ 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; + unsigned long long swap_usage; + unsigned long mem_usage; + + virCheckFlags(0, -1); + + if (!(vm = lxcDomObjFromDomain(dom))) + goto cleanup; + + priv = vm->privateData; + + if (virDomainMemoryStatsEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (!virCgroupGetMemSwapUsage(priv->cgroup, &swap_usage)) + goto cleanup; + + if (!virCgroupGetMemoryUsage(priv->cgroup, &mem_usage)) + goto cleanup; + + ret = 0; + if (!virDomainObjIsActive(vm)) + goto cleanup;
I think these two lines (well three), otherwise we don't fail on inactive domain, in contrast with qemu driver for instance.
+ + 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; + stats[ret].val = swap_usage; + ret++; + } + if (ret < nr_stats) { + stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_RSS; + stats[ret].val = mem_usage; + ret++; + } + +cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + + +static int lxcNodeGetCPUStats(virConnectPtr conn, int cpuNum, virNodeCPUStatsPtr params, @@ -5426,6 +5481,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 */
Michal
Hi Michal, thx for further cleaning up, and pushing - Michal Privoznik wrote:
+ if (!virCgroupGetMemoryUsage(priv->cgroup, &mem_usage)) + goto cleanup; + + ret = 0; + if (!virDomainObjIsActive(vm)) + goto cleanup;
I think these two lines (well three), otherwise we don't fail on inactive domain, in contrast with qemu driver for instance.
Yeah. Though there are cases (e.g. lxcDomainGetInfo()), where a stopped domain is not a hard error. I might have been mislead, but was working under the assumption that get methods (that could return something sensible, like zero cpu time for a stopped domain) would be fine to fail more gracefully? If this is something where some consolidation work would be desirable, please let me know. Cheers, -- Thorsten
On 20.02.2014 22:04, Thorsten Behrens wrote:
Hi Michal,
thx for further cleaning up, and pushing -
Michal Privoznik wrote:
+ if (!virCgroupGetMemoryUsage(priv->cgroup, &mem_usage)) + goto cleanup; + + ret = 0; + if (!virDomainObjIsActive(vm)) + goto cleanup;
I think these two lines (well three), otherwise we don't fail on inactive domain, in contrast with qemu driver for instance.
Yeah. Though there are cases (e.g. lxcDomainGetInfo()), where a stopped domain is not a hard error. I might have been mislead, but was working under the assumption that get methods (that could return something sensible, like zero cpu time for a stopped domain) would be fine to fail more gracefully?
I don't think so. I mean, we allow users to get status of any domain. So they can just check the status of domain prior to getting any runtime info. We shouldn't return an empty info as it might confuse callers: is the domain not running or libvirt isn't possible to get any data? And to distinguish these two states, they'd have to call getDomainState anyway. Michal
On Fri, Feb 14, 2014 at 06:49:00PM +0100, Thorsten Behrens wrote:
--- Notes to v4: - check errors before filling param array - UL->ULL change punted to separate patch
src/lxc/lxc_driver.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index f735631..827d989 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -5197,6 +5197,61 @@ 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; + unsigned long long swap_usage; + unsigned long mem_usage; + + virCheckFlags(0, -1); + + if (!(vm = lxcDomObjFromDomain(dom))) + goto cleanup; + + priv = vm->privateData; + + if (virDomainMemoryStatsEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (!virCgroupGetMemSwapUsage(priv->cgroup, &swap_usage)) + goto cleanup; + + if (!virCgroupGetMemoryUsage(priv->cgroup, &mem_usage)) + goto cleanup; + + ret = 0; + if (!virDomainObjIsActive(vm)) + goto cleanup;
This has to be higher up and raise VIR_ERR_OPERATION_INVALID to prevent us accessing a NULL priv->cgroup. ACK, I'll make that fix before pushing 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 :|
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 88a1a89..5b141d0 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1018,6 +1018,7 @@ virCgroupGetCpuCfsQuota; virCgroupGetCpusetCpus; virCgroupGetCpusetMems; virCgroupGetCpuShares; +virCgroupGetDomainTotalCpuStats; virCgroupGetFreezerState; virCgroupGetMemoryHardLimit; virCgroupGetMemorySoftLimit; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 59e018d..8e12892 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 @@ -15799,56 +15798,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: @@ -16046,7 +15995,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
--- Notes to v4: - removed extraneous variable src/libvirt_private.syms | 1 + src/lxc/lxc_driver.c | 49 +++++++++++++++++++++++++++++++ src/util/vircgroup.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/vircgroup.h | 7 +++++ 4 files changed, 132 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5b141d0..ad3a077 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1025,6 +1025,7 @@ virCgroupGetMemorySoftLimit; virCgroupGetMemoryUsage; virCgroupGetMemSwapHardLimit; virCgroupGetMemSwapUsage; +virCgroupGetPercpuStats; virCgroupHasController; virCgroupIsolateMount; virCgroupKill; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 827d989..e31b3ac 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -77,6 +77,7 @@ #define LXC_NB_MEM_PARAM 3 + static int lxcStateInitialize(bool privileged, virStateInhibitCallback callback, void *opaque); @@ -5422,6 +5423,53 @@ 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; + 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; + + if (!virDomainObjIsActive(vm)) { + 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, @@ -5501,6 +5549,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
Adds lxcDomainBlockStatsFlags and lxcDomainBlockStats functions. --- src/lxc/lxc_driver.c | 195 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 195 insertions(+) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index e31b3ac..e1fcceb 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -76,6 +76,7 @@ #define LXC_NB_MEM_PARAM 3 +#define LXC_NB_DOMAIN_BLOCK_STAT_PARAM 4 static int lxcStateInitialize(bool privileged, @@ -2230,6 +2231,198 @@ 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, @@ -5528,6 +5721,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
On 14.02.2014 18:49, Thorsten Behrens wrote:
Adds lxcDomainBlockStatsFlags and lxcDomainBlockStats functions. --- src/lxc/lxc_driver.c | 195 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 195 insertions(+)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index e31b3ac..e1fcceb 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -76,6 +76,7 @@
#define LXC_NB_MEM_PARAM 3 +#define LXC_NB_DOMAIN_BLOCK_STAT_PARAM 4
static int lxcStateInitialize(bool privileged, @@ -2230,6 +2231,198 @@ 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) {
Okay, we allow passing "" as path. But only via C API, not in virsh. So I guess this is still okay - esp. if a quick look at the very next patch address the virsh part of my concern. Although, the documentation to these APIs don't mention passing empty string - which again you're fixing in the next patch. I think I can overlook fact that it should be in this patch (or the order of these two patches should be reversed).
+ /* 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; + } + }
Michal
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 666ab1e..b0051bb 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 f221475..a13a1c7 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
On 14.02.2014 18:49, 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(-)
Very wise! We unfortunately can't allow NULL here as it would require RPC change (and thus a new API - we have no upstream experience with introducing a new version on RPC :))
diff --git a/src/libvirt.c b/src/libvirt.c index 666ab1e..b0051bb 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 = "";
Well, we rather check the return value in a different manner. Although this should never return an error.
rc = virDomainBlockStatsFlags(dom, device, NULL, &nparams, 0);
diff --git a/tools/virsh.pod b/tools/virsh.pod index f221475..a13a1c7 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.
Michal
On 02/20/2014 08:26 AM, Michal Privoznik wrote:
On 14.02.2014 18:49, 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(-)
Very wise! We unfortunately can't allow NULL here as it would require RPC change (and thus a new API - we have no upstream experience with introducing a new version on RPC :))
Actually, we do: as of 1.2.2, virConnectDomainEventRegisterAny is the first API that knows how to manage 2 versions of the underlying RPC call. But yeah, we don't want to make a habit of doing fork-brained RPC calls. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
--- Notes to v4: - share fake disk device path via header file instead of env var tests/testutilslxc.h | 3 ++ tests/vircgroupmock.c | 98 +++++++++++++++++++++++++++++++++++++- tests/vircgrouptest.c | 129 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 228 insertions(+), 2 deletions(-) diff --git a/tests/testutilslxc.h b/tests/testutilslxc.h index ee8056f..aa0730e 100644 --- a/tests/testutilslxc.h +++ b/tests/testutilslxc.h @@ -1,4 +1,7 @@ #include "capabilities.h" +# define FAKEDEVDIR0 "/fakedevdir0/bla/fasl" +# define FAKEDEVDIR1 "/fakedevdir1/bla/fasl" + virCapsPtr testLXCCapsInit(void); diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c index 6542973..d154a4a 100644 --- a/tests/vircgroupmock.c +++ b/tests/vircgroupmock.c @@ -30,10 +30,13 @@ # include <fcntl.h> # include <sys/stat.h> # include <stdarg.h> +# include "testutilslxc.h" 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 +46,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; +const char *fakedevicedir0 = FAKEDEVDIR0; +const char *fakedevicedir1 = FAKEDEVDIR1; # define SYSFS_PREFIX "/not/really/sys/fs/cgroup/" @@ -332,13 +337,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 +397,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); } @@ -529,6 +545,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 +576,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..df29531 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -32,6 +32,7 @@ # include "virerror.h" # include "virlog.h" # include "virfile.h" +# include "testutilslxc.h" # define VIR_FROM_THIS VIR_FROM_NONE @@ -529,6 +530,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" @@ -571,6 +694,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
On Fri, Feb 14, 2014 at 06:49:05PM +0100, Thorsten Behrens wrote:
--- Notes to v4: - share fake disk device path via header file instead of env var
tests/testutilslxc.h | 3 ++ tests/vircgroupmock.c | 98 +++++++++++++++++++++++++++++++++++++- tests/vircgrouptest.c | 129 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 228 insertions(+), 2 deletions(-)
ACK, but I'll squash this into patch 1 since they are related. 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 :|
On 14.02.2014 18:49, Thorsten Behrens wrote:
--- Notes to v4: - share fake disk device path via header file instead of env var
tests/testutilslxc.h | 3 ++ tests/vircgroupmock.c | 98 +++++++++++++++++++++++++++++++++++++- tests/vircgrouptest.c | 129 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 228 insertions(+), 2 deletions(-)
diff --git a/tests/testutilslxc.h b/tests/testutilslxc.h index ee8056f..aa0730e 100644 --- a/tests/testutilslxc.h +++ b/tests/testutilslxc.h @@ -1,4 +1,7 @@
#include "capabilities.h"
+# define FAKEDEVDIR0 "/fakedevdir0/bla/fasl" +# define FAKEDEVDIR1 "/fakedevdir1/bla/fasl" +
No need for the space in between '#' and 'define' as you're not in ifdef block. In fact, syntax-check raised an error here.
virCapsPtr testLXCCapsInit(void);
Michal
--- tests/vircgrouptest.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index df29531..e05b882 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -530,6 +530,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; @@ -700,6 +732,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
--- tests/vircgroupmock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c index d154a4a..ae88984 100644 --- a/tests/vircgroupmock.c +++ b/tests/vircgroupmock.c @@ -215,7 +215,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
--- tests/vircgrouptest.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index e05b882..5c14efb 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -33,6 +33,7 @@ # include "virlog.h" # include "virfile.h" # include "testutilslxc.h" +# include "nodeinfo.h" # define VIR_FROM_THIS VIR_FROM_NONE @@ -530,6 +531,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; @@ -735,6 +798,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
On 14.02.2014 18:48, 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'.
v4 addresses the last remaining review comments.
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 | 300 +++++++++++++++++++++++++++++++++ src/qemu/qemu_driver.c | 54 +----- src/util/vircgroup.c | 382 +++++++++++++++++++++++++++++++++++++++++++ src/util/vircgroup.h | 24 +++ tests/testutilslxc.h | 3 + tests/vircgroupmock.c | 100 ++++++++++- tests/vircgrouptest.c | 230 ++++++++++++++++++++++++++ tools/virsh-domain-monitor.c | 11 +- tools/virsh.pod | 5 +- 11 files changed, 1059 insertions(+), 62 deletions(-)
ACKed series. I've fixed all the small nits and pushed. Congratulations on first big contribution! Michal
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Michal Privoznik -
Thorsten Behrens