[libvirt] [PATCH 0/3] Build and other fixes
So I broke the build on non-Linux. Sorry. But I did that *because* I was considering other platforms in the code. Also there was a bad dereference somewhere. So because it's three patches, I'm rather sending it for review than just pushing it. Martin Kletzander (3): tests: Properly dereference cpus pointer in virnumamock.c virhostcpu: Expose virHostCPUGetOnline on non-Linux virhostcpu: Make only defined symbols available src/util/virhostcpu.c | 35 +++++++++++++++++------------------ src/util/virhostcpu.h | 5 ++++- tests/virnumamock.c | 2 +- 3 files changed, 22 insertions(+), 20 deletions(-) -- 2.12.1
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- tests/virnumamock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/virnumamock.c b/tests/virnumamock.c index 89e420fa4173..210d15d6adf0 100644 --- a/tests/virnumamock.c +++ b/tests/virnumamock.c @@ -181,7 +181,7 @@ virNumaGetNodeCPUs(int node, virBitmapPtr *cpus) return -1; *cpus = virBitmapParseUnlimited(cpulist); - if (!cpus) + if (!*cpus) goto cleanup; ret = virBitmapCountBits(*cpus); -- 2.12.1
On Mon, Mar 27, 2017 at 04:21:08PM +0200, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- tests/virnumamock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
ACK Pavel
Previously, this function must've been called only on Linux in order to fail gracefully. That lead to #ifdef mess in callers, so the function was redesigned so it failed gracefully on non-existing files. However that commit forgot to define the function outside the __linux__ ifdef. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/util/virhostcpu.c | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 47f930cdbf3c..02b9fc8eb94f 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -262,24 +262,6 @@ virHostCPUGetCore(unsigned int cpu, unsigned int *core) return 0; } -int -virHostCPUGetOnline(unsigned int cpu, bool *online) -{ - unsigned int tmp = 0; - int ret = virSysfsGetCpuValueUint(cpu, "online", &tmp); - - - /* If the file is not there, it's online (doesn't support offlining) */ - if (ret == -2) - tmp = 1; - else if (ret < 0) - return -1; - - *online = tmp; - - return 0; -} - virBitmapPtr virHostCPUGetSiblingsList(unsigned int cpu) { @@ -880,6 +862,23 @@ virHostCPUParseCountLinux(void) } #endif +int +virHostCPUGetOnline(unsigned int cpu, bool *online) +{ + unsigned int tmp = 0; + int ret = virSysfsGetCpuValueUint(cpu, "online", &tmp); + + + /* If the file is not there, it's online (doesn't support offlining) */ + if (ret == -2) + tmp = 1; + else if (ret < 0) + return -1; + + *online = tmp; + + return 0; +} int virHostCPUStatsAssign(virNodeCPUStatsPtr param, -- 2.12.1
On Mon, Mar 27, 2017 at 04:21:09PM +0200, Martin Kletzander wrote:
Previously, this function must've been called only on Linux in order to fail gracefully. That lead to #ifdef mess in callers, so the function was redesigned so it failed gracefully on non-existing files. However that commit forgot to define the function outside the __linux__ ifdef.
It would be nice to mention the commit id in the commit message.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/util/virhostcpu.c | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-)
ACK Pavel
That way you get the error from the compiler before the linker. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/util/virhostcpu.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/util/virhostcpu.h b/src/util/virhostcpu.h index a4ce655d6a2e..e9c22eecc9c9 100644 --- a/src/util/virhostcpu.h +++ b/src/util/virhostcpu.h @@ -57,10 +57,13 @@ int virHostCPUStatsAssign(virNodeCPUStatsPtr param, const char *name, unsigned long long value); +# ifdef __linux__ int virHostCPUGetSocket(unsigned int cpu, unsigned int *socket); int virHostCPUGetCore(unsigned int cpu, unsigned int *core); -int virHostCPUGetOnline(unsigned int cpu, bool *online); virBitmapPtr virHostCPUGetSiblingsList(unsigned int cpu); +# endif + +int virHostCPUGetOnline(unsigned int cpu, bool *online); #endif /* __VIR_HOSTCPU_H__*/ -- 2.12.1
On Mon, Mar 27, 2017 at 04:21:10PM +0200, Martin Kletzander wrote:
That way you get the error from the compiler before the linker.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/util/virhostcpu.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
ACK Pavel
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- Just to make it a teeny tiny nicer, how about this one as well. I know the whole file needs way more refactoring and cleaning up, but we can do that by small pieces. src/libvirt_linux.syms | 3 +++ src/libvirt_private.syms | 2 -- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libvirt_linux.syms b/src/libvirt_linux.syms index 3d66f013062b..5fa2c790efc1 100644 --- a/src/libvirt_linux.syms +++ b/src/libvirt_linux.syms @@ -3,7 +3,10 @@ # # util/virhostcpu.h +virHostCPUGetCore; virHostCPUGetInfoPopulateLinux; +virHostCPUGetSiblingsList; +virHostCPUGetSocket; virHostCPUGetStatsLinux; # Let emacs know we want case-insensitive sorting diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 4c14ef59012a..605a6e6785be 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1713,7 +1713,6 @@ virHookPresent; # util/virhostcpu.h -virHostCPUGetCore; virHostCPUGetCount; virHostCPUGetInfo; virHostCPUGetKVMMaxVCPUs; @@ -1721,7 +1720,6 @@ virHostCPUGetMap; virHostCPUGetOnline; virHostCPUGetOnlineBitmap; virHostCPUGetPresentBitmap; -virHostCPUGetSocket; virHostCPUGetStats; virHostCPUGetThreadsPerSubcore; virHostCPUHasBitmap; -- 2.12.1
On Mon, Mar 27, 2017 at 05:17:57PM +0200, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- Just to make it a teeny tiny nicer, how about this one as well. I know the whole file needs way more refactoring and cleaning up, but we can do that by small pieces.
ACK, this also fixes a potential issue that the virHostCPUGetSiblingsList is not listed in any of the *.syms files. Pavel
participants (2)
-
Martin Kletzander -
Pavel Hrdina