[libvirt] [PATCH 0/3] virSysinfo: Try dmidecode on aarch64
Turns out some aarch64 machines provide SMBIOS and thus we can use dmidecode to get the sysinfo. Currently, we're parsing /proc/cpuinfo which is very sparse on ARMs. Yes, ideally everything would be exposed somewhere in sysfs, and libvirt would ditch dmidecode in favor of sysfs, but how realistic is that? Michal Prívozník (3): virSysinfoRead: Simplify #ifdef underbush virSysinfoParseX86BaseBoard: Free memory upfront if no board detected virSysinfoReadARM: Try reading DMI table src/util/virsysinfo.c | 43 +++++++++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 12 deletions(-) -- 2.21.0
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virsysinfo.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 79a935b90a..9e60d1a553 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -1212,13 +1212,12 @@ virSysinfoRead(void) return virSysinfoReadARM(); #elif defined(__s390__) || defined(__s390x__) return virSysinfoReadS390(); -#elif defined(WIN32) || \ - !(defined(__x86_64__) || \ - defined(__i386__) || \ - defined(__amd64__) || \ - defined(__arm__) || \ - defined(__aarch64__) || \ - defined(__powerpc__)) +#elif !defined(WIN32) && \ + (defined(__x86_64__) || \ + defined(__i386__) || \ + defined(__amd64__)) + return virSysinfoReadX86(); +#else /* WIN32 || not supported arch */ /* * this can probably be extracted from Windows using API or registry * http://www.microsoft.com/whdc/system/platform/firmware/SMBIOS.mspx @@ -1226,9 +1225,7 @@ virSysinfoRead(void) virReportSystemError(ENOSYS, "%s", _("Host sysinfo extraction not supported on this platform")); return NULL; -#else /* !WIN32 && x86 */ - return virSysinfoReadX86(); -#endif /* !WIN32 && x86 */ +#endif /* WIN32 || not supported arch */ } -- 2.21.0
On Fri, May 10, 2019 at 09:52:26 +0200, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virsysinfo.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-)
ACK
If no board was detected then VIR_REALLOC_N() done at the end of the function will actually free the memory (because nborads == 0), but @boards will be set to a non-NULL pointer. This makes it unnecessary harder for a caller to see if any board was detected. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virsysinfo.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 9e60d1a553..6c3adc23ab 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -844,8 +844,12 @@ virSysinfoParseX86BaseBoard(const char *base, nboards--; } - /* This is safe, as we can be only shrinking the memory */ - ignore_value(VIR_REALLOC_N(boards, nboards)); + if (nboards == 0) { + VIR_FREE(boards); + } else { + /* This is safe, as we can be only shrinking the memory */ + ignore_value(VIR_REALLOC_N(boards, nboards)); + } *baseBoard = boards; *nbaseBoard = nboards; -- 2.21.0
On Fri, May 10, 2019 at 09:52:27 +0200, Michal Privoznik wrote:
If no board was detected then VIR_REALLOC_N() done at the end of the function will actually free the memory (because nborads == 0), but @boards will be set to a non-NULL pointer. This makes it unnecessary harder for a caller to see if any board was detected.
Ah, yeah. Allocating 0 bytes of memory may get you a pointer.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virsysinfo.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 9e60d1a553..6c3adc23ab 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -844,8 +844,12 @@ virSysinfoParseX86BaseBoard(const char *base, nboards--; }
- /* This is safe, as we can be only shrinking the memory */ - ignore_value(VIR_REALLOC_N(boards, nboards)); + if (nboards == 0) { + VIR_FREE(boards);
ACK
On 5/10/19 10:08 AM, Peter Krempa wrote:
On Fri, May 10, 2019 at 09:52:27 +0200, Michal Privoznik wrote:
If no board was detected then VIR_REALLOC_N() done at the end of the function will actually free the memory (because nborads == 0), but @boards will be set to a non-NULL pointer. This makes it unnecessary harder for a caller to see if any board was detected.
Ah, yeah. Allocating 0 bytes of memory may get you a pointer.
I'm wondering if it makes sense to modify VIR_REALLOC_N() so that it calls VIR_FREE() if count == 0. If so, then this patch is not needed. Michal
On Fri, May 10, 2019 at 11:09:32AM +0200, Michal Privoznik wrote:
On 5/10/19 10:08 AM, Peter Krempa wrote:
On Fri, May 10, 2019 at 09:52:27 +0200, Michal Privoznik wrote:
If no board was detected then VIR_REALLOC_N() done at the end of the function will actually free the memory (because nborads == 0), but @boards will be set to a non-NULL pointer. This makes it unnecessary harder for a caller to see if any board was detected.
Ah, yeah. Allocating 0 bytes of memory may get you a pointer.
I'm wondering if it makes sense to modify VIR_REALLOC_N() so that it calls VIR_FREE() if count == 0. If so, then this patch is not needed.
That would makes its behaviour different from other alocation functions, which will none the less allocate 1 byte, if you ask for 0 bytes. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
https://bugzilla.redhat.com/show_bug.cgi?id=1426162 Turns out, some aarch64 systems have SMBIOS info. That means we can use dmidecode to fetch some information. If that fails, fall back to the old behaviour. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virsysinfo.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 6c3adc23ab..783440314c 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -192,6 +192,14 @@ void virSysinfoDefFree(virSysinfoDefPtr def) } +static bool +virSysinfoDefEmpty(const virSysinfoDef *def) +{ + return !(def->bios || def->system || def->nbaseBoard || + def->chassis || def->nprocessor || def->nmemory || def->oemStrings); +} + + static int virSysinfoParsePPCSystem(const char *base, virSysinfoSystemDefPtr *sysdef) { @@ -433,6 +441,16 @@ virSysinfoReadARM(void) virSysinfoDefPtr ret = NULL; char *outbuf = NULL; + /* Some ARM systems have DMI tables available. */ + if ((ret = virSysinfoReadX86())) { + if (!virSysinfoDefEmpty(ret)) + return ret; + virSysinfoDefFree(ret); + } + + /* Well, we've tried. Fall back to parsing cpuinfo */ + virResetLastError(); + if (VIR_ALLOC(ret) < 0) goto no_memory; -- 2.21.0
On Fri, May 10, 2019 at 09:52:28 +0200, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1426162
Turns out, some aarch64 systems have SMBIOS info. That means we can use dmidecode to fetch some information. If that fails, fall back to the old behaviour.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virsysinfo.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 6c3adc23ab..783440314c 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -192,6 +192,14 @@ void virSysinfoDefFree(virSysinfoDefPtr def) }
+static bool +virSysinfoDefEmpty(const virSysinfoDef *def) +{ + return !(def->bios || def->system || def->nbaseBoard || + def->chassis || def->nprocessor || def->nmemory || def->oemStrings); +} + + static int virSysinfoParsePPCSystem(const char *base, virSysinfoSystemDefPtr *sysdef) { @@ -433,6 +441,16 @@ virSysinfoReadARM(void) virSysinfoDefPtr ret = NULL; char *outbuf = NULL;
+ /* Some ARM systems have DMI tables available. */ + if ((ret = virSysinfoReadX86())) {
This is ultra fishy. If this function works on ARM it's time to rename that function first.
+ if (!virSysinfoDefEmpty(ret)) + return ret; + virSysinfoDefFree(ret); + } + + /* Well, we've tried. Fall back to parsing cpuinfo */ + virResetLastError(); + if (VIR_ALLOC(ret) < 0) goto no_memory;
-- 2.21.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (3)
-
Daniel P. Berrangé -
Michal Privoznik -
Peter Krempa