[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] fix numa-related (and kernel-dependent) test failures



On Fri, Dec 19, 2008 at 07:57:58PM +0100, Jim Meyering wrote:
> 
> From 108a21089f2d5f427eb39ba7f622fd61deafebb3 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <meyering redhat com>
> Date: Thu, 18 Dec 2008 13:53:47 +0100
> Subject: [PATCH] make NUMA-initialization code more portable and more robust
> 
> qemudCapsInitNUMA and umlCapsInitNUMA were identical, so this change
> factors them into a new function, virCapsInitNUMA, and puts it in
> nodeinfo.c.  Putting it there means several programs must now link
> against -lnuma, hence all the Makefile.am adjustments.

Most of those Makefile.am changes are unneccessary. Only targets
which compile source files using numactl needed the NUMA_CFLAGS/LIBS
adding. Specifically libvirt_driver.la and libvirt_lxc. The libnuma
ABI doesn't leak out into other things linking to libvirt.so or the
drivers, so no need to add link flags for libvirtd, virsh or the
tests / examples.

> In addition to factoring out the duplicates, this change also
> adjusts that function definition (along with its macros) so
> that it works with Fedora 9's numactl version 1, and makes it
> so the code will work even if someone builds the kernel with
> CONFIG_NR_CPUS > 4096.
> 
> Finally, also perform this NUMA initialization for the lxc
> and openvz drivers.

All the source files wre missing #include "nodeinfo.h" to actually
get the definition of virCapsInitNUMA, so not entirely sure how 
it managed to compile for you ?

Here's a cut down patch removing the unneccessary  makefile.am changes
and adding the missing includes, which compiles & links for me.

Daniel

Index: src/Makefile.am
===================================================================
RCS file: /data/cvs/libvirt/src/Makefile.am,v
retrieving revision 1.118
diff -u -p -r1.118 Makefile.am
--- src/Makefile.am	17 Dec 2008 21:39:41 -0000	1.118
+++ src/Makefile.am	21 Dec 2008 17:45:55 -0000
@@ -55,7 +55,7 @@ UTIL_SOURCES =							\
 		xml.c xml.h
 
 # Internal generic driver infrastructure
-DRIVER_SOURCES = 						\
+DRIVER_SOURCES =						\
 		driver.c driver.h				\
 		internal.h					\
 		datatypes.c datatypes.h				\
@@ -147,7 +147,7 @@ STORAGE_DRIVER_FS_SOURCES =					\
 		storage_backend_fs.h storage_backend_fs.c
 
 STORAGE_DRIVER_LVM_SOURCES =					\
-		storage_backend_logical.h 			\
+		storage_backend_logical.h			\
 		storage_backend_logical.c
 
 STORAGE_DRIVER_ISCSI_SOURCES =					\
@@ -189,8 +189,8 @@ libvirt_driver_la_SOURCES =					\
 		$(STORAGE_CONF_SOURCES)				\
 		$(NODE_DEVICE_CONF_SOURCES)
 
-libvirt_driver_la_CFLAGS = $(XEN_CFLAGS)
-libvirt_driver_la_LDFLAGS = $(XEN_LIBS)
+libvirt_driver_la_CFLAGS = $(XEN_CFLAGS) $(NUMACTL_CFLAGS)
+libvirt_driver_la_LDFLAGS = $(XEN_LIBS) $(NUMACTL_LIBS)
 
 if WITH_TEST
 if WITH_DRIVER_MODULES
@@ -430,8 +430,8 @@ virsh_SOURCES =							\
 		virsh.c
 
 virsh_LDFLAGS = $(WARN_CFLAGS) $(COVERAGE_LDFLAGS)
-virsh_LDADD = 							\
-		$(STATIC_BINARIES) 				\
+virsh_LDADD =							\
+		$(STATIC_BINARIES)				\
 		$(WARN_CFLAGS)					\
 		libvirt.la					\
 		../gnulib/lib/libgnu.la				\
@@ -518,11 +518,11 @@ libexec_PROGRAMS += libvirt_lxc
 
 libvirt_lxc_SOURCES =						\
 		$(LXC_CONTROLLER_SOURCES)			\
-		$(UTIL_SOURCES) 				\
+		$(UTIL_SOURCES)					\
 		$(DOMAIN_CONF_SOURCES)
 libvirt_lxc_LDFLAGS = $(WARN_CFLAGS) $(COVERAGE_LDCFLAGS)
-libvirt_lxc_LDADD = $(LIBXML_LIBS) ../gnulib/lib/libgnu.la
-libvirt_lxc_CFLAGS =  $(LIBPARTED_CFLAGS)
+libvirt_lxc_LDADD = $(LIBXML_LIBS) $(NUMACTL_LIBS) ../gnulib/lib/libgnu.la
+libvirt_lxc_CFLAGS =  $(LIBPARTED_CFLAGS) $(NUMACTL_CFLAGS)
 endif
 endif
 EXTRA_DIST += $(LXC_CONTROLLER_SOURCES)
Index: src/libvirt_sym.version.in
===================================================================
RCS file: /data/cvs/libvirt/src/libvirt_sym.version.in,v
retrieving revision 1.15
diff -u -p -r1.15 libvirt_sym.version.in
--- src/libvirt_sym.version.in	20 Dec 2008 13:09:45 -0000	1.15
+++ src/libvirt_sym.version.in	21 Dec 2008 17:45:55 -0000
@@ -491,6 +491,7 @@ LIBVIRT_PRIVATE_ VERSION@ {
 
 	# nodeinfo.h
 	virNodeInfoPopulate;
+	virCapsInitNUMA;
 
 
 	# node_device_conf.h
Index: src/lxc_conf.c
===================================================================
RCS file: /data/cvs/libvirt/src/lxc_conf.c,v
retrieving revision 1.23
diff -u -p -r1.23 lxc_conf.c
--- src/lxc_conf.c	7 Nov 2008 16:43:58 -0000	1.23
+++ src/lxc_conf.c	21 Dec 2008 17:45:55 -0000
@@ -29,6 +29,7 @@
 
 #include "virterror_internal.h"
 #include "lxc_conf.h"
+#include "nodeinfo.h"
 
 /* Functions */
 virCapsPtr lxcCapsInit(void)
@@ -43,6 +44,9 @@ virCapsPtr lxcCapsInit(void)
                                    0, 0)) == NULL)
         goto no_memory;
 
+    if (virCapsInitNUMA(caps) < 0)
+        goto no_memory;
+
     /* XXX shouldn't 'borrow' KVM's prefix */
     virCapabilitiesSetMacPrefix(caps, (unsigned char []){ 0x52, 0x54, 0x00 });
 
Index: src/nodeinfo.c
===================================================================
RCS file: /data/cvs/libvirt/src/nodeinfo.c,v
retrieving revision 1.14
diff -u -p -r1.14 nodeinfo.c
--- src/nodeinfo.c	4 Nov 2008 22:30:33 -0000	1.14
+++ src/nodeinfo.c	21 Dec 2008 17:45:55 -0000
@@ -26,13 +26,22 @@
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
+#include <stdint.h>
 #include <errno.h>
-#include "c-ctype.h"
+
+#if HAVE_NUMACTL
+# define NUMA_VERSION1_COMPATIBILITY 1
+# include <numa.h>
+#endif
 
 #ifdef HAVE_SYS_UTSNAME_H
 #include <sys/utsname.h>
 #endif
 
+#include "memory.h"
+
+#include "c-ctype.h"
+
 #include "virterror_internal.h"
 #include "nodeinfo.h"
 #include "physmem.h"
@@ -171,3 +180,67 @@ int virNodeInfoPopulate(virConnectPtr co
     return -1;
 #endif
 }
+
+#if HAVE_NUMACTL
+# if LIBNUMA_API_VERSION <= 1
+#  define NUMA_MAX_N_CPUS 4096
+# else
+#  define NUMA_MAX_N_CPUS (numa_all_cpus_ptr->size)
+# endif
+
+# define n_bits(var) (8 * sizeof(var))
+# define MASK_CPU_ISSET(mask, cpu) \
+  (((mask)[((cpu) / n_bits(*(mask)))] >> ((cpu) % n_bits(*(mask)))) & 1)
+
+int
+virCapsInitNUMA(virCapsPtr caps)
+{
+    int n;
+    uint64_t *mask = NULL;
+    int *cpus = NULL;
+    int ret = -1;
+    int max_n_cpus = NUMA_MAX_N_CPUS;
+
+    if (numa_available() < 0)
+        return 0;
+
+    int mask_n_bytes = max_n_cpus / 8;
+    if (VIR_ALLOC_N(mask, mask_n_bytes / sizeof *mask) < 0)
+        goto cleanup;
+
+    for (n = 0 ; n <= numa_max_node() ; n++) {
+        int i;
+        int ncpus;
+        if (numa_node_to_cpus(n, mask, mask_n_bytes) < 0)
+            goto cleanup;
+
+        for (ncpus = 0, i = 0 ; i < max_n_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_n_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
+int virCapsInitNUMA(virCapsPtr caps ATTRIBUTE_UNUSED) { return 0; }
+#endif
Index: src/nodeinfo.h
===================================================================
RCS file: /data/cvs/libvirt/src/nodeinfo.h,v
retrieving revision 1.3
diff -u -p -r1.3 nodeinfo.h
--- src/nodeinfo.h	20 Aug 2008 20:48:36 -0000	1.3
+++ src/nodeinfo.h	21 Dec 2008 17:45:55 -0000
@@ -1,7 +1,7 @@
 /*
  * nodeinfo.c: Helper routines for OS specific node information
  *
- * Copyright (C) 2006, 2007 Red Hat, Inc.
+ * Copyright (C) 2006-2008 Red Hat, Inc.
  * Copyright (C) 2006 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -25,7 +25,9 @@
 #define __VIR_NODEINFO_H__
 
 #include "libvirt/libvirt.h"
+#include "capabilities.h"
 
 int virNodeInfoPopulate(virConnectPtr conn, virNodeInfoPtr nodeinfo);
+int virCapsInitNUMA(virCapsPtr caps);
 
 #endif /* __VIR_NODEINFO_H__*/
Index: src/openvz_conf.c
===================================================================
RCS file: /data/cvs/libvirt/src/openvz_conf.c,v
retrieving revision 1.53
diff -u -p -r1.53 openvz_conf.c
--- src/openvz_conf.c	17 Dec 2008 21:13:19 -0000	1.53
+++ src/openvz_conf.c	21 Dec 2008 17:45:55 -0000
@@ -146,6 +146,9 @@ virCapsPtr openvzCapsInit(void)
                                    0, 0)) == NULL)
         goto no_memory;
 
+    if (virCapsInitNUMA(caps) < 0)
+        goto no_memory;
+
     virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x52, 0x54, 0x00 });
 
     if ((guest = virCapabilitiesAddGuest(caps,
Index: src/qemu_conf.c
===================================================================
RCS file: /data/cvs/libvirt/src/qemu_conf.c,v
retrieving revision 1.117
diff -u -p -r1.117 qemu_conf.c
--- src/qemu_conf.c	20 Dec 2008 13:09:45 -0000	1.117
+++ src/qemu_conf.c	21 Dec 2008 17:45:55 -0000
@@ -36,11 +36,6 @@
 #include <arpa/inet.h>
 #include <sys/utsname.h>
 
-#if HAVE_NUMACTL
-#define NUMA_VERSION1_COMPATIBILITY 1
-#include <numa.h>
-#endif
-
 #include "virterror_internal.h"
 #include "qemu_conf.h"
 #include "uuid.h"
@@ -51,6 +46,7 @@
 #include "verify.h"
 #include "datatypes.h"
 #include "xml.h"
+#include "nodeinfo.h"
 
 VIR_ENUM_DECL(virDomainDiskQEMUBus)
 VIR_ENUM_IMPL(virDomainDiskQEMUBus, VIR_DOMAIN_DISK_BUS_LAST,
@@ -300,66 +296,6 @@ qemudCapsInitGuest(virCapsPtr caps,
     return 0;
 }
 
-#if HAVE_NUMACTL
-#define MAX_CPUS 4096
-#define MAX_CPUS_MASK_SIZE (sizeof(unsigned long))
-#define MAX_CPUS_MASK_BITS (MAX_CPUS_MASK_SIZE * 8)
-#define MAX_CPUS_MASK_LEN (MAX_CPUS / (MAX_CPUS_MASK_BITS))
-
-#define MASK_CPU_ISSET(mask, cpu) \
-    (((mask)[((cpu) / MAX_CPUS_MASK_BITS)] >> ((cpu) % MAX_CPUS_MASK_BITS)) & 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++) {
-        int mask_n_bytes = numa_all_cpus_ptr->size / 8;
-        if (numa_node_to_cpus(n, mask, mask_n_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;
@@ -375,7 +311,7 @@ virCapsPtr qemudCapsInit(void) {
     /* Using KVM's mac prefix for QEMU too */
     virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x52, 0x54, 0x00 });
 
-    if (qemudCapsInitNUMA(caps) < 0)
+    if (virCapsInitNUMA(caps) < 0)
         goto no_memory;
 
     for (i = 0 ; i < ARRAY_CARDINALITY(arch_info_hvm) ; i++)
Index: src/uml_conf.c
===================================================================
RCS file: /data/cvs/libvirt/src/uml_conf.c,v
retrieving revision 1.4
diff -u -p -r1.4 uml_conf.c
--- src/uml_conf.c	17 Dec 2008 07:05:46 -0000	1.4
+++ src/uml_conf.c	21 Dec 2008 17:45:55 -0000
@@ -36,11 +36,6 @@
 #include <arpa/inet.h>
 #include <sys/utsname.h>
 
-#if HAVE_NUMACTL
-#define NUMA_VERSION1_COMPATIBILITY 1
-#include <numa.h>
-#endif
-
 #include "uml_conf.h"
 #include "uuid.h"
 #include "buf.h"
@@ -48,72 +43,10 @@
 #include "util.h"
 #include "memory.h"
 #include "verify.h"
-
+#include "nodeinfo.h"
 
 #define umlLog(level, msg...) fprintf(stderr, msg)
 
-
-
-#if HAVE_NUMACTL
-#define MAX_CPUS 4096
-#define MAX_CPUS_MASK_SIZE (sizeof(unsigned long))
-#define MAX_CPUS_MASK_BITS (MAX_CPUS_MASK_SIZE * 8)
-#define MAX_CPUS_MASK_LEN (MAX_CPUS / (MAX_CPUS_MASK_BITS))
-
-#define MASK_CPU_ISSET(mask, cpu) \
-    (((mask)[((cpu) / MAX_CPUS_MASK_BITS)] >> ((cpu) % MAX_CPUS_MASK_BITS)) & 1)
-
-static int
-umlCapsInitNUMA(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++) {
-        int mask_n_bytes = numa_all_cpus_ptr->size / 8;
-        if (numa_node_to_cpus(n, mask, mask_n_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 umlCapsInitNUMA(virCapsPtr caps ATTRIBUTE_UNUSED) { return 0; }
-#endif
-
 virCapsPtr umlCapsInit(void) {
     struct utsname utsname;
     virCapsPtr caps;
@@ -126,7 +59,7 @@ virCapsPtr umlCapsInit(void) {
                                    0, 0)) == NULL)
         goto no_memory;
 
-    if (umlCapsInitNUMA(caps) < 0)
+    if (virCapsInitNUMA(caps) < 0)
         goto no_memory;
 
     if ((guest = virCapabilitiesAddGuest(caps,

-- 
|: Red Hat, Engineering, London   -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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]