2:47 a.m.
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
2:47 a.m.
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
5:15 a.m.
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 :|
6:52 a.m.
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
12:57 p.m.
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
3966
days inactive
3967
days old
4 comments
2 participants
participants (2)
-
Daniel P. Berrange -
Roman Bogorodskiy