Re: [libvirt] [PATCH] BSD: implement nodeGetMemoryStats
On 01/02/2014 10:35 AM, Roman Bogorodskiy wrote:
Add a BSD implementation of nodeGetMemoryStats based
on sysctl(3).
---
src/nodeinfo.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 73 insertions(+)
Code seems reasonable to me - I don't have the capability to test under
__APPLE__, but figured I could at least give it a once over.
I have a couple of suggestions below, but ACK in general though
John
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 1838547..ecacfa4 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -55,6 +55,8 @@
#define VIR_FROM_THIS VIR_FROM_NONE
#if defined(__FreeBSD__) || defined(__APPLE__)
+# define BSD_MEMORY_STATS_ALL 4
+
static int
appleFreebsdNodeGetCPUCount(void)
{
@@ -96,6 +98,75 @@ appleFreebsdNodeGetMemorySize(unsigned long *memory)
return 0;
}
+
+static int
+appleFreebsdNodeGetMemoryStats(virNodeMemoryStatsPtr params,
+ int *nparams)
+{
+ size_t i, j = 0;
+ unsigned long pagesize = getpagesize() >> 10;
+ long bufpages;
+ size_t bufpages_size = sizeof(bufpages);
+ struct field_sysctl_map {
+ const char *field;
+ const char *sysctl_name;
+ } sysctl_map[] = {
+ {VIR_NODE_MEMORY_STATS_TOTAL, "vm.stats.vm.v_page_count"},
+ {VIR_NODE_MEMORY_STATS_FREE, "vm.stats.vm.v_free_count"},
+ {VIR_NODE_MEMORY_STATS_CACHED, "vm.stats.vm.v_cache_count"},
+ {NULL, NULL}
+ };
+
+ if ((*nparams) == 0) {
+ *nparams = BSD_MEMORY_STATS_ALL;
+ return 0;
+ }
+
+ if ((*nparams) != BSD_MEMORY_STATS_ALL) {
+ virReportInvalidArg(nparams,
+ _("nparams in %s must be %d"),
+ __FUNCTION__, BSD_MEMORY_STATS_ALL);
+ return -1;
+ }
+
+ for (i = 0; sysctl_map[i].field != NULL; i++) {
+ u_int value;
+ size_t value_size = sizeof(value);
+ virNodeMemoryStatsPtr param;
+
+ if (sysctlbyname(sysctl_map[i].sysctl_name, &value,
+ &value_size, NULL, 0) < 0) {
+ virReportSystemError(errno, "%s", _("sysctl failed"));
s/"%s", _("sysctl failed")/_("sysctl failed for
'%s'"),
sysctl_map[i].sysctl_name/
That way it'd be easier to determine which failed...
+ return -1;
+ }
+
+ param = ¶ms[j++];
+ if (virStrcpyStatic(param->field, sysctl_map[i].field) == NULL) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Field '%s' too long for destination"),
+ sysctl_map[i].field);
+ return -1;
+ }
+ param->value = (unsigned long long)value * pagesize;
+ }
+
+ {
+ virNodeMemoryStatsPtr param = ¶ms[j];
Not that it's necessary now, but perhaps a j++ here just in case another
parameter was ever added and someone neglected this one...
+
+ if (sysctlbyname("vfs.bufspace", &bufpages, &bufpages_size,
NULL, 0) < 0) {
Probably should add an error message here
+ return -1;
+ }
+ if (virStrcpyStatic(param->field, VIR_NODE_MEMORY_STATS_BUFFERS) == NULL) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Field '%s' too long for destination"),
+ VIR_NODE_MEMORY_STATS_BUFFERS);
+ return -1;
+ }
+ param->value = (unsigned long long)bufpages >> 10;
+ }
+
+ return 0;
+}
#endif
Add a "/* defined(__FreeBSD__) || defined(__APPLE__) */" to the #endif -
just as a way to be clear.
#ifdef __linux__
@@ -1041,6 +1112,8 @@ int nodeGetMemoryStats(int cellNum ATTRIBUTE_UNUSED,
return ret;
}
+#elif defined(__FreeBSD__) || defined(__APPLE__)
+ return appleFreebsdNodeGetMemoryStats(params, nparams);
#else
virReportError(VIR_ERR_NO_SUPPORT, "%s",
_("node memory stats not implemented on this platform"));