[libvirt] [PATCH] BSD: implement nodeGetCPUStats
I had doubts how to implement that. Looks like the current implementation is tied to Linux CPU metrics: user nice system idle iowait That list is hardcoded into virsh-host.c. FreeBSD has a slightly different set of metrics: user nice system intr idle I.e. it's interrupt time instead of i/o time. I decided to go without virsh-host.c modification and used VIR_NODE_CPU_STATS_UTILIZATION. If you know better way of doing that, I'm open for suggestions. Roman Bogorodskiy (1): BSD: implement nodeGetCPUStats src/nodeinfo.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) -- 1.8.4.3
Implementation obtains CPU usage information using kern.cp_time and kern.cp_times sysctl(8)s and reports CPU utilization. --- src/nodeinfo.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 05bc038..b837500 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -36,6 +36,7 @@ #if defined(__FreeBSD__) || defined(__APPLE__) # include <sys/types.h> # include <sys/sysctl.h> +# include <sys/resource.h> #endif #include "c-ctype.h" @@ -99,9 +100,91 @@ appleFreebsdNodeGetMemorySize(unsigned long *memory) #endif /* defined(__FreeBSD__) || defined(__APPLE__) */ #ifdef __FreeBSD__ +# define BSD_CPU_STATS_ALL 1 # define BSD_MEMORY_STATS_ALL 4 static int +freebsdNodeGetCPUStats(int cpuNum, + virNodeCPUStatsPtr params, + int *nparams) +{ + char *sysctl_name = NULL; + long *cpu_times, total = 0; + size_t i, cpu_times_size; + int cpu_times_num, offset; + int ret = -1; + double utilization = 0; + + if ((*nparams) == 0) { + *nparams = BSD_CPU_STATS_ALL; + return 0; + } + + if ((*nparams) != BSD_CPU_STATS_ALL) { + virReportInvalidArg(*nparams, + _("nparams in %s must be equal to %d"), + __FUNCTION__, BSD_CPU_STATS_ALL); + return -1; + } + + if (cpuNum == VIR_NODE_CPU_STATS_ALL_CPUS) { + if (VIR_STRDUP(sysctl_name, "kern.cp_time") < 0) + return -1; + cpu_times_num = 1; + offset = 0; + } else { + cpu_times_num = appleFreebsdNodeGetCPUCount(); + + if (cpuNum >= cpu_times_num) { + virReportInvalidArg(cpuNum, + _("Invalid cpuNum in %s"), + __FUNCTION__); + return -1; + } + + offset = cpu_times_num * CPUSTATES; + + if (VIR_STRDUP(sysctl_name, "kern.cp_times") < 0) + return -1; + } + + cpu_times_size = sizeof(long) * cpu_times_num * CPUSTATES; + + if (VIR_ALLOC_N(cpu_times, cpu_times_num * CPUSTATES) < 0) + goto cleanup; + + if (sysctlbyname(sysctl_name, cpu_times, &cpu_times_size, NULL, 0) < 0) { + virReportSystemError(errno, + _("sysctl failed for '%s'"), + sysctl_name); + goto cleanup; + } + + for (i = 0; i < CPUSTATES; ++i) + total += cpu_times[offset + i]; + + utilization = 100 * (1.0 - (double)cpu_times[offset + CP_IDLE] / total); + + virNodeCPUStatsPtr param = ¶ms[0]; + + if (virStrcpyStatic(param->field, VIR_NODE_CPU_STATS_UTILIZATION) == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Field '%s' too long for destination"), + VIR_NODE_CPU_STATS_UTILIZATION); + goto cleanup; + } + + param->value = utilization; + ret = 0; + +cleanup: + VIR_FREE(sysctl_name); + VIR_FREE(cpu_times); + + return ret; +} + +static int freebsdNodeGetMemoryStats(virNodeMemoryStatsPtr params, int *nparams) { @@ -1066,6 +1149,8 @@ int nodeGetCPUStats(int cpuNum ATTRIBUTE_UNUSED, return ret; } +#elif defined(__FreeBSD__) + return freebsdNodeGetCPUStats(cpuNum, params, nparams); #else virReportError(VIR_ERR_NO_SUPPORT, "%s", _("node CPU stats not implemented on this platform")); -- 1.8.4.3
On Tue, Jan 14, 2014 at 12:47:20PM +0400, Roman Bogorodskiy wrote:
I had doubts how to implement that. Looks like the current implementation is tied to Linux CPU metrics:
user nice system idle iowait
That list is hardcoded into virsh-host.c. FreeBSD has a slightly different set of metrics:
user nice system intr idle
I.e. it's interrupt time instead of i/o time.
That's not a problem. Just define a new API constant for INTR and don't use the IOWAIT constant in your impl. No impl is required to support every single possible value.
I decided to go without virsh-host.c modification and used VIR_NODE_CPU_STATS_UTILIZATION.
I'd avoid using that. It only exists for virt which is incapable of providing the raw values. It is much better to let the apps calculate the utilization themselves since they can choose the time period over which to perform the calculation better. 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 :|
Daniel P. Berrange wrote:
On Tue, Jan 14, 2014 at 12:47:20PM +0400, Roman Bogorodskiy wrote:
I had doubts how to implement that. Looks like the current implementation is tied to Linux CPU metrics:
user nice system idle iowait
That list is hardcoded into virsh-host.c. FreeBSD has a slightly different set of metrics:
user nice system intr idle
I.e. it's interrupt time instead of i/o time.
That's not a problem. Just define a new API constant for INTR and don't use the IOWAIT constant in your impl. No impl is required to support every single possible value.
I decided to go without virsh-host.c modification and used VIR_NODE_CPU_STATS_UTILIZATION.
I'd avoid using that. It only exists for virt which is incapable of providing the raw values. It is much better to let the apps calculate the utilization themselves since they can choose the time period over which to perform the calculation better.
Thanks for advise, I'll provide an updated patch. Roman Bogorodskiy
Daniel P. Berrange wrote:
On Tue, Jan 14, 2014 at 12:47:20PM +0400, Roman Bogorodskiy wrote:
I had doubts how to implement that. Looks like the current implementation is tied to Linux CPU metrics:
user nice system idle iowait
That list is hardcoded into virsh-host.c. FreeBSD has a slightly different set of metrics:
user nice system intr idle
I.e. it's interrupt time instead of i/o time.
That's not a problem. Just define a new API constant for INTR and don't use the IOWAIT constant in your impl. No impl is required to support every single possible value.
I've just uploaded new version of the patch. Meanwhile, I was thinking if it would be reasonable just to count INTR as system load without introducing a new variable. Roman Bogorodskiy
participants (2)
-
Daniel P. Berrange -
Roman Bogorodskiy