[libvirt] [PATCH v2 0/2] virSysinfo: Try dmidecode on aarch64
v2 of: https://www.redhat.com/archives/libvir-list/2019-May/msg00221.html diff to v1: - Pushed 1/3 and 2/3 from v1 as they were ACKed already - New patch 1/2 which renames virSysinfoReadX86 - Patch 2/2 then uses the renamed function Michal Prívozník (2): virsysinfo: Rename virSysinfoReadX86 to virSysinfoReadDMI virSysinfoReadARM: Try reading DMI table src/libvirt_private.syms | 2 +- src/util/virsysinfo.c | 22 ++++++++++++++++++++-- src/util/virsysinfopriv.h | 2 +- tests/sysinfotest.c | 2 +- 4 files changed, 23 insertions(+), 5 deletions(-) -- 2.21.0
There's nothing x86 specific about this function. Rename the function so that it has DMI suffix which enables it to be reused on different arches (as using X86 from say ARM would look suspicious). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt_private.syms | 2 +- src/util/virsysinfo.c | 4 ++-- src/util/virsysinfopriv.h | 2 +- tests/sysinfotest.c | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 909975750c..f2b8dc445d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3070,9 +3070,9 @@ virSysinfoSystemDefFree; # util/virsysinfopriv.h virSysinfoReadARM; +virSysinfoReadDMI; virSysinfoReadPPC; virSysinfoReadS390; -virSysinfoReadX86; # util/virsystemd.h diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 6c3adc23ab..b371e8dd26 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -1139,7 +1139,7 @@ virSysinfoParseX86Memory(const char *base, virSysinfoDefPtr ret) } virSysinfoDefPtr -virSysinfoReadX86(void) +virSysinfoReadDMI(void) { char *path; virSysinfoDefPtr ret = NULL; @@ -1220,7 +1220,7 @@ virSysinfoRead(void) (defined(__x86_64__) || \ defined(__i386__) || \ defined(__amd64__)) - return virSysinfoReadX86(); + return virSysinfoReadDMI(); #else /* WIN32 || not supported arch */ /* * this can probably be extracted from Windows using API or registry diff --git a/src/util/virsysinfopriv.h b/src/util/virsysinfopriv.h index 3ef675441e..b7beb44807 100644 --- a/src/util/virsysinfopriv.h +++ b/src/util/virsysinfopriv.h @@ -39,6 +39,6 @@ virSysinfoDefPtr virSysinfoReadS390(void); virSysinfoDefPtr -virSysinfoReadX86(void); +virSysinfoReadDMI(void); #endif /* LIBVIRT_VIRSYSINFOPRIV_H */ diff --git a/tests/sysinfotest.c b/tests/sysinfotest.c index 7fa9a2dfd6..4d03fd3809 100644 --- a/tests/sysinfotest.c +++ b/tests/sysinfotest.c @@ -133,7 +133,7 @@ mymain(void) TEST("s390", virSysinfoReadS390); TEST("s390-freq", virSysinfoReadS390); TEST("ppc", virSysinfoReadPPC); - TEST_FULL("x86", virSysinfoReadX86, "/sysinfodata/dmidecode.sh"); + TEST_FULL("x86", virSysinfoReadDMI, "/sysinfodata/dmidecode.sh"); TEST("arm", virSysinfoReadARM); TEST("arm-rpi2", virSysinfoReadARM); TEST("aarch64", virSysinfoReadARM); -- 2.21.0
On Fri, 2019-05-10 at 14:20 +0200, Michal Privoznik wrote: [...]
TEST("s390", virSysinfoReadS390); TEST("s390-freq", virSysinfoReadS390); TEST("ppc", virSysinfoReadPPC); - TEST_FULL("x86", virSysinfoReadX86, "/sysinfodata/dmidecode.sh"); + TEST_FULL("x86", virSysinfoReadDMI, "/sysinfodata/dmidecode.sh");
This hunk is going to need some slight massaging after [1] has been merged, but nothing too complicated. Assuming this goes in after my series and you take care of the above before pushing, Reviewed-by: Andrea Bolognani <abologna@redhat.com> [1] https://www.redhat.com/archives/libvir-list/2019-May/msg00869.html -- Andrea Bolognani / Red Hat / Virtualization
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 b371e8dd26..83da3897ce 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 = virSysinfoReadDMI())) { + 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 02:20:25PM +0200, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1426162
Turns out, some aarch64 systems have SMBIOS info. That means we
In fact aarch64 are required to support SMBIOS if they want to be compliant with the Server Base Boot Requirements spec. This effectively covers any aarch64 machine targetted as data center servers.
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 b371e8dd26..83da3897ce 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 = virSysinfoReadDMI())) { + 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
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 :|
On Fri, 2019-05-10 at 14:20 +0200, Michal Privoznik wrote: [...]
+static bool +virSysinfoDefEmpty(const virSysinfoDef *def)
This is a predicate, so please call it virSysinfoDefIsEmpty().
+{ + return !(def->bios || def->system || def->nbaseBoard || + def->chassis || def->nprocessor || def->nmemory || def->oemStrings);
For nbaseBoard, nprocessor and nmemory you should use explicit numeric comparison, according to the recently updated contributor guidelines[1]. With the two nits above addressed, Reviewed-by: Andrea Bolognani <abologna@redhat.com> [1] https://libvirt.org/hacking.html#conditions -- Andrea Bolognani / Red Hat / Virtualization
participants (3)
-
Andrea Bolognani -
Daniel P. Berrangé -
Michal Privoznik