[libvirt] [PATCH] Bug fix: Allow sysinfo to display processor information
From: Prerna Saxena <prerna@linux.vnet.ibm.com> Date: Mon, 6 Feb 2012 13:46:10 +0530 Subject: [PATCH] Bugfix: Allow sysinfo to display 'processor' information Calls to virSysinfoParseBIOS(), virSysinfoParseSystem() would update the buffer pointer 'base', so the processor information would be lost before virSysinfoParseProcessor() was called. Sysinfo would therefore not be able to display processor details -- It only described <bios>, <system> and <memory_device> details. Before the fix: --------------- virsh # sysinfo <sysinfo type='smbios'> <bios> .... </bios> <system> .... </system> <memory_device> .... </memory_device> After the fix: ------------- virsh # sysinfo <sysinfo type='smbios'> <bios> .... </bios> <system> .... </system> <processor> .... </processor> <memory_device> .... </memory_device> --- src/util/sysinfo.c | 9 +++++---- 1 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c index de3108a..d92d655 100644 --- a/src/util/sysinfo.c +++ b/src/util/sysinfo.c @@ -483,16 +483,17 @@ virSysinfoRead(void) { base = outbuf; + ret->nprocessor = 0; + ret->processor = NULL; + if ((base = virSysinfoParseProcessor(base, ret)) == NULL) + goto no_memory; + if ((base = virSysinfoParseBIOS(base, ret)) == NULL) goto no_memory; if ((base = virSysinfoParseSystem(base, ret)) == NULL) goto no_memory; - ret->nprocessor = 0; - ret->processor = NULL; - if ((base = virSysinfoParseProcessor(base, ret)) == NULL) - goto no_memory; ret->nmemory = 0; ret->memory = NULL; -- 1.7.3.1 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India
On 02/06/2012 01:36 AM, Prerna Saxena wrote:
From: Prerna Saxena <prerna@linux.vnet.ibm.com> Date: Mon, 6 Feb 2012 13:46:10 +0530 Subject: [PATCH] Bugfix: Allow sysinfo to display 'processor' information
Calls to virSysinfoParseBIOS(), virSysinfoParseSystem() would update the buffer pointer 'base', so the processor information would be lost before virSysinfoParseProcessor() was called. Sysinfo would therefore not be able to display processor details -- It only described <bios>, <system> and <memory_device> details.
What version of dmidecode are you using? My version, dmidecode-2.11-5.fc16.x86_64 from Fedora 16, outputs processor after system, not before bios, when executing 'dmidecode -q -t 0,1,4,17'. This probably means that not all dmidecode versions output data in the same order. NAK to your patch (since while it would fix life on your machine, it would break life on my machine), but ACK to the bug report that we should make the code be more tolerant of varying order. I'm thinking the simplest thing would be to change all of the parser functions to quit returning an end pointer where they stopped parsing, and instead to always scan the entire string. After all, that was only being done as an optimization to try and do strstr() on smaller portions of the overall output string.
+++ b/src/util/sysinfo.c @@ -483,16 +483,17 @@ virSysinfoRead(void) {
base = outbuf;
+ ret->nprocessor = 0; + ret->processor = NULL; + if ((base = virSysinfoParseProcessor(base, ret)) == NULL) + goto no_memory; + if ((base = virSysinfoParseBIOS(base, ret)) == NULL) goto no_memory;
That is, make calls such as: if (virSysinfoParseBIOS(outbuf, ret) < 0) goto no_memory after changing virSysinfoParseBIOS to return an int rather than a pointer inside its incoming argument. Can you rework this patch along those lines? -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 02/07/2012 03:18 AM, Eric Blake wrote:
On 02/06/2012 01:36 AM, Prerna Saxena wrote:
From: Prerna Saxena <prerna@linux.vnet.ibm.com> Date: Mon, 6 Feb 2012 13:46:10 +0530 Subject: [PATCH] Bugfix: Allow sysinfo to display 'processor' information
Calls to virSysinfoParseBIOS(), virSysinfoParseSystem() would update the buffer pointer 'base', so the processor information would be lost before virSysinfoParseProcessor() was called. Sysinfo would therefore not be able to display processor details -- It only described <bios>, <system> and <memory_device> details.
What version of dmidecode are you using? My version, dmidecode-2.11-5.fc16.x86_64 from Fedora 16, outputs processor after system, not before bios, when executing 'dmidecode -q -t 0,1,4,17'.
I chanced upon this while using dmidecode-2.10-2.fc14.x86_64. Didnt know that dmidecode versions might differ in their output.
This probably means that not all dmidecode versions output data in the same order. NAK to your patch (since while it would fix life on your machine, it would break life on my machine), but ACK to the bug report that we should make the code be more tolerant of varying order.
I'm thinking the simplest thing would be to change all of the parser functions to quit returning an end pointer where they stopped parsing, and instead to always scan the entire string. After all, that was only being done as an optimization to try and do strstr() on smaller portions of the overall output string.
+++ b/src/util/sysinfo.c @@ -483,16 +483,17 @@ virSysinfoRead(void) {
base = outbuf;
+ ret->nprocessor = 0; + ret->processor = NULL; + if ((base = virSysinfoParseProcessor(base, ret)) == NULL) + goto no_memory; + if ((base = virSysinfoParseBIOS(base, ret)) == NULL) goto no_memory;
That is, make calls such as:
if (virSysinfoParseBIOS(outbuf, ret) < 0) goto no_memory
after changing virSysinfoParseBIOS to return an int rather than a pointer inside its incoming argument.
Can you rework this patch along those lines?
Thanks, this seems logical. Will spin a quick patch right away. Regards, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India
From: Prerna Saxena <prerna@linux.vnet.ibm.com> Date: Tue, 7 Feb 2012 17:05:37 +0530 Subject: [PATCH] On systems with dmidecode version 2.10 or older, dmidecode displays processor information, followed by BIOS, system and memory-DIMM details. Calls to virSysinfoParseBIOS(), virSysinfoParseSystem() would update the buffer pointer 'base', so the processor information would be lost before virSysinfoParseProcessor() was called. Sysinfo would therefore not be able to display processor details -- It only described <bios>, <system> and <memory_device> details. This patch attempts to insulate sysinfo from ordering of dmidecode output. Before the fix: --------------- virsh # sysinfo <sysinfo type='smbios'> <bios> .... </bios> <system> .... </system> <memory_device> .... </memory_device> After the fix: ------------- virsh # sysinfo <sysinfo type='smbios'> <bios> .... </bios> <system> .... </system> <processor> .... </processor> <memory_device> .... </memory_device> --- src/util/sysinfo.c | 12 +++++------- 1 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c index de3108a..0e55d7e 100644 --- a/src/util/sysinfo.c +++ b/src/util/sysinfo.c @@ -453,7 +453,7 @@ no_memory: virSysinfoDefPtr virSysinfoRead(void) { - char *path, *base; + char *path; virSysinfoDefPtr ret = NULL; char *outbuf = NULL; virCommandPtr cmd; @@ -481,22 +481,20 @@ virSysinfoRead(void) { ret->type = VIR_SYSINFO_SMBIOS; - base = outbuf; - - if ((base = virSysinfoParseBIOS(base, ret)) == NULL) + if ((virSysinfoParseBIOS(outbuf, ret)) == NULL) goto no_memory; - if ((base = virSysinfoParseSystem(base, ret)) == NULL) + if ((virSysinfoParseSystem(outbuf, ret)) == NULL) goto no_memory; ret->nprocessor = 0; ret->processor = NULL; - if ((base = virSysinfoParseProcessor(base, ret)) == NULL) + if ((virSysinfoParseProcessor(outbuf, ret)) == NULL) goto no_memory; ret->nmemory = 0; ret->memory = NULL; - if (virSysinfoParseMemory(base, ret) == NULL) + if (virSysinfoParseMemory(outbuf, ret) == NULL) goto no_memory; cleanup: -- 1.7.3.1 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India
On 02/07/2012 04:43 AM, Prerna Saxena wrote:
From: Prerna Saxena <prerna@linux.vnet.ibm.com> Date: Tue, 7 Feb 2012 17:05:37 +0530 Subject: [PATCH] On systems with dmidecode version 2.10 or older, dmidecode displays processor information, followed by BIOS, system and memory-DIMM details. Calls to virSysinfoParseBIOS(), virSysinfoParseSystem() would update the buffer pointer 'base', so the processor information would be lost before virSysinfoParseProcessor() was called. Sysinfo would therefore not be able to display processor details -- It only described <bios>, <system> and <memory_device> details. This patch attempts to insulate sysinfo from ordering of dmidecode output.
I was thinking of going one step further - since we no longer care about the result, there's no longer a need to return a char *, and we can instead return an int (0 for success, -1 for failure). But that can be a separate cleanup; your patch is a minimal fix for the bug at hand, so ACK and pushed.
- base = outbuf; - - if ((base = virSysinfoParseBIOS(base, ret)) == NULL) + if ((virSysinfoParseBIOS(outbuf, ret)) == NULL) goto no_memory;
-- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Prerna Saxena