On Tue, May 20, 2008 at 02:15:59PM +0200, Jim Meyering wrote:
> "Daniel P. Berrange" <berrange@xxxxxxxxxx> wrote:
> > This patch includes NUMA topology info in the QEMU driver capabilities
> > XML output. It also implements the free memory driver APIs. This is done
> > with the LGPL'd numactl library. The configure script probes for it and
> > only enables this functionality if it is found. The numactl library has
> > been around for quite a while - RHEL-3 vintage at least
>
> Looks fine, modulo a few nits...
>
> ...
> > + for (ncpus = 0, i = 0 ; i < MAX_CPUS ; i++)
> > + if ((mask[(i / MAX_CPUS_MASK_SIZE)] >> (i % MAX_CPUS_MASK_SIZE)) & 1)
> ...
> > + for (ncpus = 0, i = 0 ; i < MAX_CPUS ; i++)
> > + if ((mask[(i / MAX_CPUS_MASK_SIZE)] >> (i % MAX_CPUS_MASK_SIZE)) & 1)
>
> The above bit-is-set query deserves a macro.
Here's a update with that macro-ized, and the stupid fprintf()'s that rich
noticed removed.
> If the above diagnostics needn't be different,
> you could do this:
>
> fail=0
> AC_CHECK_HEADER([numa.h], [], [fail=1])
> AC_CHECK_LIB([numa], [numa_available], [], [fail=1])
> test $fail = 1 &&
> AC_MSG_ERROR([You must install the numactl development package in order to compile libvirt])
[snip]
> No big deal, but these are underquoted, and technically will cause trouble
> if there's ever an m4 macro with a conflicting name, so this syntax is
> preferred:
These 2 configure changes are candidates for global cleanup - the same duplicate
diagnostics & underquoting are present through-out
configure.in | 39 +++++++++++++++++++++++++++++++
libvirt.spec.in | 3 ++
src/Makefile.am | 2 +
src/qemu_conf.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
src/qemu_driver.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 175 insertions(+)
Dan
diff -r f912d3498d1b configure.in
--- a/configure.in Tue May 20 10:49:20 2008 -0400
+++ b/configure.in Tue May 20 11:54:16 2008 -0400
@@ -533,6 +533,40 @@
AM_CONDITIONAL(HAVE_SELINUX, [test "$with_selinux" != "no"])
AC_SUBST(SELINUX_CFLAGS)
AC_SUBST(SELINUX_LIBS)
+
+dnl NUMA lib
+AC_ARG_WITH(numactl,
+ [ --with-numactl use numactl for host topology info],
+ [],
+ [with_numactl=check])
+
+NUMACTL_CFLAGS=
+NUMACTL_LIBS=
+if test "$with_qemu" = "yes" -a "$with_numactl" != "no"; then
+ old_cflags="$CFLAGS"
+ old_libs="$LIBS"
+ if test "$with_numactl" = "check"; then
+ AC_CHECK_HEADER([numa.h],[],[with_numactl=no])
+ AC_CHECK_LIB(numa, numa_available,[],[with_numactl=no])
+ if test "$with_numactl" != "no"; then
+ with_numactl="yes"
+ fi
+ else
+ AC_CHECK_HEADER([numa.h],[],
+ [AC_MSG_ERROR([You must install the numactl development package in order to compile libvirt])])
+ AC_CHECK_LIB(numa, numa_available,[],
+ [AC_MSG_ERROR([You must install the numactl development package in order to compile and run libvirt])])
+ fi
+ CFLAGS="$old_cflags"
+ LIBS="$old_libs"
+fi
+if test "$with_numactl" = "yes"; then
+ NUMACTL_LIBS="-lnuma"
+ AC_DEFINE_UNQUOTED(HAVE_NUMACTL, 1, [whether Numactl is available for security])
+fi
+AM_CONDITIONAL(HAVE_NUMACTL, [test "$with_numactl" != "no"])
+AC_SUBST(NUMACTL_CFLAGS)
+AC_SUBST(NUMACTL_LIBS)
dnl virsh libraries
AC_CHECK_HEADERS([readline/readline.h])
@@ -1001,6 +1035,11 @@
else
AC_MSG_NOTICE([ selinux: no])
fi
+if test "$with_numactl" = "yes" ; then
+AC_MSG_NOTICE([ numactl: $NUMACTL_CFLAGS $NUMACTL_LIBS])
+else
+AC_MSG_NOTICE([ numactl: no])
+fi
AC_MSG_NOTICE([])
AC_MSG_NOTICE([Miscellaneous])
AC_MSG_NOTICE([])
diff -r f912d3498d1b libvirt.spec.in
--- a/libvirt.spec.in Tue May 20 10:49:20 2008 -0400
+++ b/libvirt.spec.in Tue May 20 11:54:16 2008 -0400
@@ -67,6 +67,9 @@
BuildRequires: bridge-utils
BuildRequires: qemu
BuildRequires: cyrus-sasl-devel
+%if %{with_qemu}
+BuildRequires: numactl-devel
+%endif
%if %{with_polkit}
BuildRequires: PolicyKit-devel >= 0.6
%endif
diff -r f912d3498d1b src/Makefile.am
--- a/src/Makefile.am Tue May 20 10:49:20 2008 -0400
+++ b/src/Makefile.am Tue May 20 11:54:16 2008 -0400
@@ -9,6 +9,7 @@
$(GNUTLS_CFLAGS) \
$(SASL_CFLAGS) \
$(SELINUX_CFLAGS) \
+ $(NUMACTL_CFLAGS) \
-DBINDIR=\""$(libexecdir)"\" \
-DSBINDIR=\""$(sbindir)"\" \
-DSYSCONF_DIR="\"$(sysconfdir)\"" \
@@ -100,6 +101,7 @@
libvirt_la_SOURCES = $(CLIENT_SOURCES) $(SERVER_SOURCES)
libvirt_la_LIBADD = $(LIBXML_LIBS) $(GNUTLS_LIBS) $(SASL_LIBS) $(SELINUX_LIBS) \
+ $(NUMACTL_LIBS) \
@CYGWIN_EXTRA_LIBADD@ ../gnulib/lib/libgnu.la
libvirt_la_LDFLAGS = -Wl,--version-script=$(srcdir)/libvirt_sym.version \
-version-info @LIBVIRT_VERSION_INFO@ \
diff -r f912d3498d1b src/qemu_conf.c
--- a/src/qemu_conf.c Tue May 20 10:49:20 2008 -0400
+++ b/src/qemu_conf.c Tue May 20 11:54:16 2008 -0400
@@ -42,6 +42,10 @@
#include <libxml/xpath.h>
#include <libxml/uri.h>
+#if HAVE_NUMACTL
+#include <numa.h>
+#endif
+
#include "libvirt/virterror.h"
#include "qemu_conf.h"
@@ -49,6 +53,7 @@
#include "buf.h"
#include "conf.h"
#include "util.h"
+#include "memory.h"
#include "verify.h"
#include "c-ctype.h"
@@ -390,6 +395,65 @@
return 0;
}
+#if HAVE_NUMACTL
+#define MAX_CPUS 4096
+#define MAX_CPUS_MASK_SIZE (sizeof(unsigned long))
+#define MAX_CPUS_MASK_LEN (MAX_CPUS / MAX_CPUS_MASK_SIZE)
+#define MAX_CPUS_MASK_BYTES (MAX_CPUS / 8)
+
+#define MASK_CPU_ISSET(mask, cpu) \
+ (((mask)[((cpu) / MAX_CPUS_MASK_SIZE)] >> ((cpu) % MAX_CPUS_MASK_SIZE)) & 1)
+
+static int
+qemudCapsInitNUMA(virCapsPtr caps)
+{
+ int n, i;
+ unsigned long *mask = NULL;
+ int ncpus;
+ int *cpus = NULL;
+ int ret = -1;
+
+ if (numa_available() < 0)
+ return 0;
+
+ if (VIR_ALLOC_N(mask, MAX_CPUS_MASK_LEN) < 0)
+ goto cleanup;
+
+ for (n = 0 ; n <= numa_max_node() ; n++) {
+ if (numa_node_to_cpus(n, mask, MAX_CPUS_MASK_BYTES) < 0)
+ goto cleanup;
+
+ for (ncpus = 0, i = 0 ; i < MAX_CPUS ; i++)
+ if (MASK_CPU_ISSET(mask, i))
+ ncpus++;
+
+ if (VIR_ALLOC_N(cpus, ncpus) < 0)
+ goto cleanup;
+
+ for (ncpus = 0, i = 0 ; i < MAX_CPUS ; i++)
+ if (MASK_CPU_ISSET(mask, i))
+ cpus[ncpus++] = i;
+
+ if (virCapabilitiesAddHostNUMACell(caps,
+ n,
+ ncpus,
+ cpus) < 0)
+ goto cleanup;
+
+ VIR_FREE(cpus);
+ }
+
+ ret = 0;
+
+cleanup:
+ VIR_FREE(cpus);
+ VIR_FREE(mask);
+ return ret;
+}
+#else
+static int qemudCapsInitNUMA(virCapsPtr caps ATTRIBUTE_UNUSED) { return 0; }
+#endif
+
virCapsPtr qemudCapsInit(void) {
struct utsname utsname;
virCapsPtr caps;
@@ -400,6 +464,9 @@
if ((caps = virCapabilitiesNew(utsname.machine,
0, 0)) == NULL)
+ goto no_memory;
+
+ if (qemudCapsInitNUMA(caps) < 0)
goto no_memory;
for (i = 0 ; i < (sizeof(arch_info_hvm)/sizeof(arch_info_hvm[0])) ; i++)
diff -r f912d3498d1b src/qemu_driver.c
--- a/src/qemu_driver.c Tue May 20 10:49:20 2008 -0400
+++ b/src/qemu_driver.c Tue May 20 11:54:16 2008 -0400
@@ -45,6 +45,10 @@
#include <stdio.h>
#include <sys/wait.h>
#include <libxml/uri.h>
+
+#if HAVE_NUMACTL
+#include <numa.h>
+#endif
#include "libvirt/virterror.h"
@@ -1619,6 +1623,61 @@
}
+#if HAVE_NUMACTL
+static int
+qemudNodeGetCellsFreeMemory(virConnectPtr conn,
+ unsigned long long *freeMems,
+ int startCell,
+ int maxCells)
+{
+ int n, lastCell, numCells;
+
+ if (numa_available() < 0) {
+ qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT,
+ "%s", _("NUMA not supported on this host"));
+ return -1;
+ }
+ lastCell = startCell + maxCells - 1;
+ if (lastCell > numa_max_node())
+ lastCell = numa_max_node();
+
+ for (numCells = 0, n = startCell ; n <= lastCell ; n++) {
+ long long mem;
+ if (numa_node_size64(n, &mem) < 0) {
+ qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ "%s", _("Failed to query NUMA free memory"));
+ return -1;
+ }
+ freeMems[numCells++] = mem;
+ }
+ return numCells;
+}
+
+static unsigned long long
+qemudNodeGetFreeMemory (virConnectPtr conn)
+{
+ unsigned long long freeMem = 0;
+ int n;
+ if (numa_available() < 0) {
+ qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT,
+ "%s", _("NUMA not supported on this host"));
+ return -1;
+ }
+
+ for (n = 0 ; n <= numa_max_node() ; n++) {
+ long long mem;
+ if (numa_node_size64(n, &mem) < 0) {
+ qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ "%s", _("Failed to query NUMA free memory"));
+ return -1;
+ }
+ freeMem += mem;
+ }
+
+ return freeMem;
+}
+
+#endif
static int qemudGetProcessInfo(unsigned long long *cpuTime, int pid) {
char proc[PATH_MAX];
@@ -3182,8 +3241,13 @@
NULL, /* domainMigrateFinish */
qemudDomainBlockStats, /* domainBlockStats */
qemudDomainInterfaceStats, /* domainInterfaceStats */
+#if HAVE_NUMACTL
+ qemudNodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */
+ qemudNodeGetFreeMemory, /* getFreeMemory */
+#else
NULL, /* nodeGetCellsFreeMemory */
NULL, /* getFreeMemory */
+#endif
};
static virNetworkDriver qemuNetworkDriver = {
--
|: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list