[libvirt] [PATCH] fix numa-related (and kernel-dependent) test failures
Following up on this thread, http://thread.gmane.org/gmane.comp.emulators.libvirt/10402 Here's a patch that appears to work for kernels before and after the CONFIG_NR_CPUS change. I've tested it via a small stand-alone program on both kernels, but via libvirt only on rawhide. Dave, can you confirm the tests pass for you on the 2.6.27.x-based kernel?
From b78a48c7a401ecdf37de95b0b01435578ad30e12 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Fri, 12 Dec 2008 16:22:34 +0100 Subject: [PATCH] fix numa-related (and kernel-dependent) test failures
This change is required on some kernels due to the way a change in the kernel's CONFIG_NR_CPUS propagates through the numa library. * src/qemu_conf.c (qemudCapsInitNUMA): Pass numa_all_cpus_ptr->size/8 as the buffer-length-in-bytes in the call to numa_node_to_cpus, since that's what is required on second and subseqent calls. * src/uml_conf.c (umlCapsInitNUMA): Likewise. --- src/qemu_conf.c | 2 +- src/uml_conf.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 59171e7..206fb0b 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -324,7 +324,7 @@ qemudCapsInitNUMA(virCapsPtr caps) for (n = 0 ; n <= numa_max_node() ; n++) { - if (numa_node_to_cpus(n, mask, MAX_CPUS_MASK_LEN / 8) < 0) + if (numa_node_to_cpus(n, mask, MAX_CPUS_MASK_LEN) < 0) goto cleanup; for (ncpus = 0, i = 0 ; i < MAX_CPUS ; i++) diff --git a/src/uml_conf.c b/src/uml_conf.c index 3659c6b..7eb630d 100644 --- a/src/uml_conf.c +++ b/src/uml_conf.c @@ -80,7 +80,7 @@ umlCapsInitNUMA(virCapsPtr caps) for (n = 0 ; n <= numa_max_node() ; n++) { - if (numa_node_to_cpus(n, mask, MAX_CPUS_MASK_LEN / 8) < 0) + if (numa_node_to_cpus(n, mask, MAX_CPUS_MASK_LEN) < 0) goto cleanup; for (ncpus = 0, i = 0 ; i < MAX_CPUS ; i++) -- 1.6.1.rc2.316.geb2f0
Jim Meyering <jim@meyering.net> wrote:
Following up on this thread,
http://thread.gmane.org/gmane.comp.emulators.libvirt/10402
Here's a patch that appears to work for kernels before and after the CONFIG_NR_CPUS change. I've tested it via a small stand-alone program on both kernels, but via libvirt only on rawhide.
Dave, can you confirm the tests pass for you on the 2.6.27.x-based kernel?
Whoops. That was the wrong patch. Here's the patch I intended:
From e7dce174d5bc7f4097125e9bbc14dc0fa2518355 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Mon, 15 Dec 2008 17:53:32 +0100 Subject: [PATCH] .
--- src/qemu_conf.c | 4 ++-- src/uml_conf.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 59171e7..312f646 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -323,8 +323,8 @@ qemudCapsInitNUMA(virCapsPtr caps) goto cleanup; for (n = 0 ; n <= numa_max_node() ; n++) { - - if (numa_node_to_cpus(n, mask, MAX_CPUS_MASK_LEN / 8) < 0) + 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++) diff --git a/src/uml_conf.c b/src/uml_conf.c index 3659c6b..00adf27 100644 --- a/src/uml_conf.c +++ b/src/uml_conf.c @@ -79,8 +79,8 @@ umlCapsInitNUMA(virCapsPtr caps) goto cleanup; for (n = 0 ; n <= numa_max_node() ; n++) { - - if (numa_node_to_cpus(n, mask, MAX_CPUS_MASK_LEN / 8) < 0) + 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++) -- 1.6.1.rc2.316.geb2f0
Jim Meyering wrote:
Jim Meyering <jim@meyering.net> wrote:
Following up on this thread,
http://thread.gmane.org/gmane.comp.emulators.libvirt/10402
Here's a patch that appears to work for kernels before and after the CONFIG_NR_CPUS change. I've tested it via a small stand-alone program on both kernels, but via libvirt only on rawhide.
Dave, can you confirm the tests pass for you on the 2.6.27.x-based kernel?
Whoops. That was the wrong patch. Here's the patch I intended:
From e7dce174d5bc7f4097125e9bbc14dc0fa2518355 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Mon, 15 Dec 2008 17:53:32 +0100 Subject: [PATCH] .
--- src/qemu_conf.c | 4 ++-- src/uml_conf.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 59171e7..312f646 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -323,8 +323,8 @@ qemudCapsInitNUMA(virCapsPtr caps) goto cleanup;
for (n = 0 ; n <= numa_max_node() ; n++) { - - if (numa_node_to_cpus(n, mask, MAX_CPUS_MASK_LEN / 8) < 0) + 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++) diff --git a/src/uml_conf.c b/src/uml_conf.c index 3659c6b..00adf27 100644 --- a/src/uml_conf.c +++ b/src/uml_conf.c @@ -79,8 +79,8 @@ umlCapsInitNUMA(virCapsPtr caps) goto cleanup;
for (n = 0 ; n <= numa_max_node() ; n++) { - - if (numa_node_to_cpus(n, mask, MAX_CPUS_MASK_LEN / 8) < 0) + 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++) -- 1.6.1.rc2.316.geb2f0
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
All tests pass for me with that patch. Looks good. Dave
On Mon, Dec 15, 2008 at 03:36:07PM -0500, Dave Allan wrote:
Jim Meyering wrote:
Whoops. That was the wrong patch. Here's the patch I intended:
From e7dce174d5bc7f4097125e9bbc14dc0fa2518355 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Mon, 15 Dec 2008 17:53:32 +0100 Subject: [PATCH] .
--- src/qemu_conf.c | 4 ++-- src/uml_conf.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 59171e7..312f646 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -323,8 +323,8 @@ qemudCapsInitNUMA(virCapsPtr caps) goto cleanup;
for (n = 0 ; n <= numa_max_node() ; n++) { - - if (numa_node_to_cpus(n, mask, MAX_CPUS_MASK_LEN / 8) < 0) + 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++) diff --git a/src/uml_conf.c b/src/uml_conf.c index 3659c6b..00adf27 100644 --- a/src/uml_conf.c +++ b/src/uml_conf.c @@ -79,8 +79,8 @@ umlCapsInitNUMA(virCapsPtr caps) goto cleanup;
for (n = 0 ; n <= numa_max_node() ; n++) { - - if (numa_node_to_cpus(n, mask, MAX_CPUS_MASK_LEN / 8) < 0) + 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++) -- 1.6.1.rc2.316.geb2f0
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
All tests pass for me with that patch. Looks good.
Same for me, +1 ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
Daniel Veillard <veillard@redhat.com> wrote: ...
All tests pass for me with that patch. Looks good.
Same for me, +1 !
Thanks. Pushed with this comment: fix numa-related (and kernel-dependent) test failures This change is required on some kernels due to the way a change in the kernel's CONFIG_NR_CPUS propagates through the numa library. * src/qemu_conf.c (qemudCapsInitNUMA): Pass numa_all_cpus_ptr->size/8 as the buffer-length-in-bytes in the call to numa_node_to_cpus, since that's what is required on second and subseqent calls. * src/uml_conf.c (umlCapsInitNUMA): Likewise.
On Wed, Dec 17, 2008 at 08:39:04AM +0100, Jim Meyering wrote:
Daniel Veillard <veillard@redhat.com> wrote: ...
All tests pass for me with that patch. Looks good.
Same for me, +1 !
Thanks. Pushed with this comment:
fix numa-related (and kernel-dependent) test failures This change is required on some kernels due to the way a change in the kernel's CONFIG_NR_CPUS propagates through the numa library. * src/qemu_conf.c (qemudCapsInitNUMA): Pass numa_all_cpus_ptr->size/8 as the buffer-length-in-bytes in the call to numa_node_to_cpus, since that's what is required on second and subseqent calls. * src/uml_conf.c (umlCapsInitNUMA): Likewise.
This change has broken the compile on Fedora 9 and earlier where the numa_all_cpus_ptr symbol does not exist. So it needs to have a test in configure.ac added, and if not present, go with our original code of a fixed mask size. Fortunately on Fedora 9's libnuma, they don't have the annoying mask size checks - that's a new Fedora 10 thing I also just noticed that its only touching the size param passed into the numa_node_to_cpus, but not the actual size that's allocated for the array. This is fairly harmless....until someone does a kernel build with NR_CPUS > 4096 Daniel -- |: 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 :|
"Daniel P. Berrange" <berrange@redhat.com> wrote:
On Wed, Dec 17, 2008 at 08:39:04AM +0100, Jim Meyering wrote:
Daniel Veillard <veillard@redhat.com> wrote: ...
All tests pass for me with that patch. Looks good.
Same for me, +1 !
Thanks. Pushed with this comment:
fix numa-related (and kernel-dependent) test failures This change is required on some kernels due to the way a change in the kernel's CONFIG_NR_CPUS propagates through the numa library. * src/qemu_conf.c (qemudCapsInitNUMA): Pass numa_all_cpus_ptr->size/8 as the buffer-length-in-bytes in the call to numa_node_to_cpus, since that's what is required on second and subseqent calls. * src/uml_conf.c (umlCapsInitNUMA): Likewise.
This change has broken the compile on Fedora 9 and earlier where the numa_all_cpus_ptr symbol does not exist. So it needs to have a test in configure.ac added, and if not present, go with our original code of a fixed mask size. Fortunately on Fedora 9's libnuma, they don't have the annoying mask size checks - that's a new Fedora 10 thing
Thanks for the heads-up. While normally I'd prefer an autoconf test, it might make sense to use an #if in this case. Maybe this will do it: #if LIBNUMA_API_VERSION <= 1 use old code #else use numa_all_cpus_ptr #endif
I also just noticed that its only touching the size param passed into the numa_node_to_cpus, but not the actual size that's allocated for the array. This is fairly harmless....until someone does a kernel build with NR_CPUS > 4096
I'll deal with this, too.
"Daniel P. Berrange" <berrange@redhat.com> wrote:
On Wed, Dec 17, 2008 at 08:39:04AM +0100, Jim Meyering wrote:
Daniel Veillard <veillard@redhat.com> wrote: ...
All tests pass for me with that patch. Looks good.
Same for me, +1 !
Thanks. Pushed with this comment:
fix numa-related (and kernel-dependent) test failures This change is required on some kernels due to the way a change in the kernel's CONFIG_NR_CPUS propagates through the numa library. * src/qemu_conf.c (qemudCapsInitNUMA): Pass numa_all_cpus_ptr->size/8 as the buffer-length-in-bytes in the call to numa_node_to_cpus, since that's what is required on second and subseqent calls. * src/uml_conf.c (umlCapsInitNUMA): Likewise.
This change has broken the compile on Fedora 9 and earlier where the numa_all_cpus_ptr symbol does not exist. So it needs to have a test in configure.ac added, and if not present, go with our original code of a fixed mask size. Fortunately on Fedora 9's libnuma, they don't have the annoying mask size checks - that's a new Fedora 10 thing
I also just noticed that its only touching the size param passed into the numa_node_to_cpus, but not the actual size that's allocated for the array. This is fairly harmless....until someone does a kernel build with NR_CPUS > 4096
This took longer than I expected. First, I just fixed the portability problem, but identically in two places. Can't have that. So factored out that duplication. That required many Makefile.am changes so as not to induce link failure for every program. It also required addition to exported sym table so libvirtd still links. Finally it all built and the daemon-conf test was failing. Phew. That was just because for other testing I'd left a non-root libvirtd running. [re-reviewing the diff, I see I left some white-space changes (removing space-before-TAB) in some Makefile.am. I can move those into a separate patch on request. ]
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. 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. * src/nodeinfo.c: Include <stdint.h>, <numa.h> and "memory.h". (virCapsInitNUMA): Rename from qemudCapsInitNUMA and umlCapsInitNUMA. (NUMA_MAX_N_CPUS): Define depending on NUMA API version. (n_bits, MASK_CPU_ISSET): Define, adjust, use uint64 rather than long. * src/nodeinfo.h: Include "capabilities.h". (virCapsInitNUMA): Declare it. * docs/examples/index.py (dump_Makefile): Add $(NUMACTL_LIBS) to LDADDS. * docs/examples/Makefile.am: Regenerate. * examples/domain-events/events-c/Makefile.am: * src/Makefile.am: Add $(NUMACTL_CFLAGS) and $(NUMACTL_LIBS) to various compile/link-related variables. * src/qemu_conf.c (qemudCapsInitNUMA): Remove duplicate code. Adjust caller. * src/uml_conf.c (umlCapsInitNUMA): Likewise. * tests/Makefile.am: Link with $(NUMACTL_LIBS). * src/lxc_conf.c (lxcCapsInit): Perform NUMA initialization here, too. * src/openvz_conf.c (openvzCapsInit): And here. * qemud/Makefile.am (libvirtd_LDADD): Add $(NUMACTL_LIBS) (libvirtd_CFLAGS): Add $(NUMACTL_CFLAGS) and align. * src/libvirt_sym.version.in: Add virCapsInitNUMA so that libvirtd can link to this function. --- docs/examples/Makefile.am | 2 +- docs/examples/index.py | 2 +- examples/domain-events/events-c/Makefile.am | 4 +- qemud/Makefile.am | 27 ++++++---- src/Makefile.am | 21 ++++---- src/libvirt_sym.version.in | 1 + src/lxc_conf.c | 3 + src/nodeinfo.c | 75 ++++++++++++++++++++++++++- src/nodeinfo.h | 4 +- src/openvz_conf.c | 3 + src/qemu_conf.c | 67 +----------------------- src/uml_conf.c | 69 +------------------------ tests/Makefile.am | 1 + 13 files changed, 119 insertions(+), 160 deletions(-) diff --git a/docs/examples/Makefile.am b/docs/examples/Makefile.am index d8e4868..f3a3e29 100644 --- a/docs/examples/Makefile.am +++ b/docs/examples/Makefile.am @@ -3,7 +3,7 @@ SUBDIRS=python INCLUDES = -I$(top_builddir)/include -I$(top_srcdir)/include -I@srcdir@/include DEPS = $(top_builddir)/src/libvirt.la -LDADDS = @STATIC_BINARIES@ $(WARN_CFLAGS) $(top_builddir)/src/libvirt.la $(COVERAGE_LDFLAGS) +LDADDS = @STATIC_BINARIES@ $(WARN_CFLAGS) $(top_builddir)/src/libvirt.la $(NUMACTL_LIBS) $(COVERAGE_LDFLAGS) rebuild: examples.xml index.html diff --git a/docs/examples/index.py b/docs/examples/index.py index 6be80c5..ad22073 100755 --- a/docs/examples/index.py +++ b/docs/examples/index.py @@ -226,7 +226,7 @@ SUBDIRS=python INCLUDES = -I$(top_builddir)/include -I$(top_srcdir)/include -I@srcdir@/include DEPS = $(top_builddir)/src/libvirt.la LDADDS = @STATIC_BINARIES@ $(WARN_CFLAGS) $(top_builddir)/src/libvirt.la \ - $(COVERAGE_LDFLAGS) + $(NUMACTL_LIBS) $(COVERAGE_LDFLAGS) rebuild: examples.xml index.html diff --git a/examples/domain-events/events-c/Makefile.am b/examples/domain-events/events-c/Makefile.am index 60b1589..4e829cb 100644 --- a/examples/domain-events/events-c/Makefile.am +++ b/examples/domain-events/events-c/Makefile.am @@ -1,5 +1,5 @@ -INCLUDES = -I@top_srcdir@/include +INCLUDES = -I$(top_srcdir)/include noinst_PROGRAMS = event-test event_test_CFLAGS = $(WARN_CFLAGS) event_test_SOURCES = event-test.c -event_test_LDADD = @top_builddir@/src/libvirt.la +event_test_LDADD = $(top_builddir)/src/libvirt.la $(NUMACTL_LIBS) diff --git a/qemud/Makefile.am b/qemud/Makefile.am index 8983416..e8278f3 100644 --- a/qemud/Makefile.am +++ b/qemud/Makefile.am @@ -77,16 +77,22 @@ libvirtd_SOURCES = $(DAEMON_SOURCES) #-D_XOPEN_SOURCE=600 -D_XOPEN_SOURCE_EXTENDED=1 -D_POSIX_C_SOURCE=199506L libvirtd_CFLAGS = \ - -I$(top_srcdir)/gnulib/lib -I../gnulib/lib \ - -I$(top_srcdir)/include -I$(top_builddir)/include \ - -I$(top_srcdir)/src \ - $(LIBXML_CFLAGS) $(GNUTLS_CFLAGS) $(SASL_CFLAGS) \ - $(POLKIT_CFLAGS) \ - $(WARN_CFLAGS) -DLOCAL_STATE_DIR="\"$(localstatedir)\"" \ - $(COVERAGE_CFLAGS) \ - -DSYSCONF_DIR="\"$(sysconfdir)\"" \ - -DQEMUD_PID_FILE="\"$(QEMUD_PID_FILE)\"" \ - -DREMOTE_PID_FILE="\"$(REMOTE_PID_FILE)\"" \ + -I$(top_srcdir)/gnulib/lib \ + -I../gnulib/lib \ + -I$(top_srcdir)/include \ + -I$(top_builddir)/include \ + -I$(top_srcdir)/src \ + $(LIBXML_CFLAGS) \ + $(GNUTLS_CFLAGS) \ + $(SASL_CFLAGS) \ + $(NUMACTL_CFLAGS) \ + $(POLKIT_CFLAGS) \ + $(WARN_CFLAGS) \ + $(COVERAGE_CFLAGS) \ + -DLOCAL_STATE_DIR="\"$(localstatedir)\"" \ + -DSYSCONF_DIR="\"$(sysconfdir)\"" \ + -DQEMUD_PID_FILE="\"$(QEMUD_PID_FILE)\"" \ + -DREMOTE_PID_FILE="\"$(REMOTE_PID_FILE)\"" \ -DGETTEXT_PACKAGE=\"$(PACKAGE)\" libvirtd_LDFLAGS = \ @@ -96,6 +102,7 @@ libvirtd_LDFLAGS = \ libvirtd_LDADD = \ $(LIBXML_LIBS) \ $(GNUTLS_LIBS) \ + $(NUMACTL_LIBS) \ $(SASL_LIBS) \ $(POLKIT_LIBS) diff --git a/src/Makefile.am b/src/Makefile.am index 68f0c01..6c65f91 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -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,13 +430,14 @@ virsh_SOURCES = \ virsh.c virsh_LDFLAGS = $(WARN_CFLAGS) $(COVERAGE_LDFLAGS) -virsh_LDADD = \ - $(STATIC_BINARIES) \ +virsh_LDADD = \ + $(STATIC_BINARIES) \ $(WARN_CFLAGS) \ + $(NUMACTL_LIBS) \ libvirt.la \ ../gnulib/lib/libgnu.la \ $(VIRSH_LIBS) -virsh_CFLAGS = $(COVERAGE_CFLAGS) $(READLINE_CFLAGS) +virsh_CFLAGS = $(COVERAGE_CFLAGS) $(READLINE_CFLAGS) $(NUMACTL_CFLAGS) BUILT_SOURCES = virsh-net-edit.c virsh-pool-edit.c virsh-net-edit.c: virsh.c Makefile.am @@ -518,11 +519,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) diff --git a/src/libvirt_sym.version.in b/src/libvirt_sym.version.in index fa9bc5a..afe8659 100644 --- a/src/libvirt_sym.version.in +++ b/src/libvirt_sym.version.in @@ -489,6 +489,7 @@ LIBVIRT_PRIVATE_@VERSION@ { # nodeinfo.h virNodeInfoPopulate; + virCapsInitNUMA; # node_device_conf.h diff --git a/src/lxc_conf.c b/src/lxc_conf.c index 0db9bb5..4fb57d3 100644 --- a/src/lxc_conf.c +++ b/src/lxc_conf.c @@ -43,6 +43,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 }); diff --git a/src/nodeinfo.c b/src/nodeinfo.c index fd58c2b..aac7d3e 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -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 conn, 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 diff --git a/src/nodeinfo.h b/src/nodeinfo.h index e7c78eb..030f0ee 100644 --- a/src/nodeinfo.h +++ b/src/nodeinfo.h @@ -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__*/ diff --git a/src/openvz_conf.c b/src/openvz_conf.c index 44a243b..49cc183 100644 --- a/src/openvz_conf.c +++ b/src/openvz_conf.c @@ -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, diff --git a/src/qemu_conf.c b/src/qemu_conf.c index c973adb..ed5bcc9 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -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" @@ -298,66 +293,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; @@ -373,7 +308,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++) diff --git a/src/uml_conf.c b/src/uml_conf.c index 00adf27..67c646a 100644 --- a/src/uml_conf.c +++ b/src/uml_conf.c @@ -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" @@ -52,68 +47,6 @@ #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, diff --git a/tests/Makefile.am b/tests/Makefile.am index 87e4235..88f44a6 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -30,6 +30,7 @@ LDADDS = \ $(GNUTLS_LIBS) \ $(SASL_LIBS) \ $(SELINUX_LIBS) \ + $(NUMACTL_LIBS) \ $(WARN_CFLAGS) \ ../src/libvirt_test.la \ ../gnulib/lib/libgnu.la \ -- 1.6.1.rc2.316.geb2f0
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 :|
"Daniel P. Berrange" <berrange@redhat.com> wrote:
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 ?
I didn't see the warnings go by. Need to configure with --enable-compile-warnings=error more religiously.
Here's a cut down patch removing the unneccessary makefile.am changes and adding the missing includes, which compiles & links for me.
Now that's really odd. I *know* I was getting link errors without those other Makefile.am changes, but can't reproduce them anymore. But this is on rawhide, and I've updated since -- and besides, maybe some build product was stale. Good catches. I'll commit the result shortly, along with... this one more change that's required to get past "make distcheck":
From 6504bf107b1aa642b35cceccac74ccf2a2c2d8d8 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Sun, 21 Dec 2008 19:00:19 +0100 Subject: * src/node_device_hal.c: Include <config.h> before everything else.
--- src/node_device_hal.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/node_device_hal.c b/src/node_device_hal.c index f2bbbef..3449ee0 100644 --- a/src/node_device_hal.c +++ b/src/node_device_hal.c @@ -21,10 +21,10 @@ * Author: David F. Lively <dlively@virtualiron.com> */ +#include <config.h> + #include <stdio.h> #include <stdlib.h> - -#include <config.h> #include <libhal.h> #include "node_device_conf.h" -- 1.6.1.rc3.350.g541da
participants (4)
-
Daniel P. Berrange -
Daniel Veillard -
Dave Allan -
Jim Meyering