[libvirt] [PATCH 0/3] Fix build without NUMA
Even though these patches would qualify as build breaker fixes, I don't think they are that critical to be pushed without somebody else's taking a look on them. Michal Privoznik (3): virnuma: Add some more comments private.syms: Export virDomainNumatuneSpecifiedMaxNode qemuxml2argvmock: Mock virNumaNodesetIsAvailable src/libvirt_private.syms | 1 + src/util/virnuma.c | 14 +++++++------- tests/qemuxml2argvmock.c | 9 +++++++++ 3 files changed, 17 insertions(+), 7 deletions(-) -- 2.0.4
Especially add comments to mark ifdef, else and endif sections. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virnuma.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/util/virnuma.c b/src/util/virnuma.c index 89435de..ee1c4af 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -74,7 +74,7 @@ virNumaGetAutoPlacementAdvice(unsigned short vcpus, virCommandFree(cmd); return output; } -#else +#else /* !HAVE_NUMAD */ char * virNumaGetAutoPlacementAdvice(unsigned short vcpus ATTRIBUTE_UNUSED, unsigned long long balloon ATTRIBUTE_UNUSED) @@ -83,7 +83,7 @@ virNumaGetAutoPlacementAdvice(unsigned short vcpus ATTRIBUTE_UNUSED, _("numad is not available on this host")); return NULL; } -#endif +#endif /* !HAVE_NUMAD */ #if WITH_NUMACTL int @@ -339,7 +339,8 @@ virNumaGetNodeCPUs(int node, # undef MASK_CPU_ISSET # undef n_bits -#else +#else /* !WITH_NUMACTL */ + int virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune, virBitmapPtr nodemask ATTRIBUTE_UNUSED) @@ -404,8 +405,7 @@ virNumaGetNodeCPUs(int node ATTRIBUTE_UNUSED, _("NUMA isn't available on this host")); return -1; } -#endif - +#endif /* !WITH_NUMACTL */ /** * virNumaGetMaxCPUs: @@ -494,8 +494,8 @@ virNumaGetDistances(int node, return ret; } +#else /* !(WITH_NUMACTL && HAVE_NUMA_BITMASK_ISBITSET) */ -#else bool virNumaNodeIsAvailable(int node) { @@ -519,7 +519,7 @@ virNumaGetDistances(int node ATTRIBUTE_UNUSED, VIR_DEBUG("NUMA distance information isn't available on this host"); return 0; } -#endif +#endif /* !(WITH_NUMACTL && HAVE_NUMA_BITMASK_ISBITSET) */ /* currently all the huge page stuff below is linux only */ -- 2.0.4
On 11/05/14 18:07, Michal Privoznik wrote:
Especially add comments to mark ifdef, else and endif sections.
'Especially' sounds a bit odd here since the comments you are adding are only else/ifdef macros.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virnuma.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
ACK Peter
As of 90286418 the function is introduced. However, it's missing an entry in the libvirt_private.syms so it can't be mocked. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt_private.syms | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 73b8167..8895ba1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -609,6 +609,7 @@ virDomainNumatuneParseXML; virDomainNumatunePlacementTypeFromString; virDomainNumatunePlacementTypeToString; virDomainNumatuneSet; +virDomainNumatuneSpecifiedMaxNode; # conf/nwfilter_conf.h -- 2.0.4
On 11/05/14 18:07, Michal Privoznik wrote:
As of 90286418 the function is introduced. However, it's missing an entry in the libvirt_private.syms so it can't be mocked.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt_private.syms | 1 + 1 file changed, 1 insertion(+)
ACK, Peter
And yet again one fix of 90286418. The problem is, if libvirt is built with NUMA disabled (--without-numad --without-numactl), the testsuite doesn't count on that and some tests fail. This is because the virNumaNodesetIsAvailable() function is taken from another section of virnuma.c which contains functions stubs for case NUMA is disabled at build time. And the function basically yields an error. However, we want our testsuite to be deterministic and hence we ought to mock the function. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuxml2argvmock.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c index 7218747..7b8bdd6 100644 --- a/tests/qemuxml2argvmock.c +++ b/tests/qemuxml2argvmock.c @@ -39,3 +39,12 @@ virNumaGetMaxNode(void) return maxnodesNum; } + +bool +virNumaNodesetIsAvailable(virDomainNumatunePtr numatune) +{ + int maxGuestNode = virDomainNumatuneSpecifiedMaxNode(numatune); + int maxHostNode = virNumaGetMaxNode(); + + return maxGuestNode <= maxHostNode; +} -- 2.0.4
On 11/05/14 18:07, Michal Privoznik wrote:
And yet again one fix of 90286418. The problem is, if libvirt is built with NUMA disabled (--without-numad --without-numactl), the testsuite doesn't count on that and some tests fail. This is because the virNumaNodesetIsAvailable() function is taken from another section of virnuma.c which contains functions stubs for case NUMA is disabled at build time. And the function basically yields an error. However, we want our testsuite to be deterministic and hence we ought to mock the function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuxml2argvmock.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c index 7218747..7b8bdd6 100644 --- a/tests/qemuxml2argvmock.c +++ b/tests/qemuxml2argvmock.c @@ -39,3 +39,12 @@ virNumaGetMaxNode(void)
return maxnodesNum; } + +bool +virNumaNodesetIsAvailable(virDomainNumatunePtr numatune) +{ + int maxGuestNode = virDomainNumatuneSpecifiedMaxNode(numatune); + int maxHostNode = virNumaGetMaxNode(); + + return maxGuestNode <= maxHostNode; +}
The reason the function needs to be in the conditionally compiled section is that the function uses the NUMA_NUM_NODES macro. The rest of the used symbols has a stub and/or is already mocked. So we need to mock this too unfortunately. ACK Peter
On Wed, Nov 05, 2014 at 06:07:18PM +0100, Michal Privoznik wrote:
And yet again one fix of 90286418. The problem is, if libvirt is built with NUMA disabled (--without-numad --without-numactl), the testsuite doesn't count on that and some tests fail. This is because the virNumaNodesetIsAvailable() function is taken from another section of virnuma.c which contains functions stubs for case NUMA is disabled at build time. And the function basically yields an error. However, we want our testsuite to be deterministic and hence we ought to mock the function.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuxml2argvmock.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c index 7218747..7b8bdd6 100644 --- a/tests/qemuxml2argvmock.c +++ b/tests/qemuxml2argvmock.c @@ -39,3 +39,12 @@ virNumaGetMaxNode(void)
return maxnodesNum; } + +bool +virNumaNodesetIsAvailable(virDomainNumatunePtr numatune) +{ + int maxGuestNode = virDomainNumatuneSpecifiedMaxNode(numatune); + int maxHostNode = virNumaGetMaxNode(); + + return maxGuestNode <= maxHostNode; +} -- 2.0.4
NACK, we shouldn't need to mock this function. I went over the commit once more and I've realized it doesn't support non-contiguous nodesets even if it could. And since it will use only functions that have both versions (better one with NUMA support and a stub without it) it can be moved out of those #ifdefs. I have the patch almost ready :) I'll also remove the need for util to have connection to conf (it will make more sense, hopefully. Martin
participants (3)
-
Martin Kletzander -
Michal Privoznik -
Peter Krempa