[libvirt] [PATCH v3 0/3] Sysinfo improvements for PowerPC & x86
This series of patches implements the following: Patch 1/2 : Implement sysinfo for PowerPC. OnPowerPC, sysinfo is obtained by reading /proc/cpuinfo since dmidecode is not available. Patch 2/2 : Based on Eric's suggestion (http://www.redhat.com/archives/libvir-list/2012-February/msg00509.html) Allow x86 to read host system's processor information from /proc/cpuinfo in the event dmidecode is not available. Changelog : ----------- v1->v2 : * Rebased and cleanups done. v2->v3: * Patch 1/2 : rebased. * Patch 2/2 : As reported by Daniel, it was found that 'dmidecode' lists socket (and not core-level) information under the <processor> XML tag. The /proc/cpuinfo fallback is also accordingly modified to display a listing of the available sockets under the '<processor>' XML tag under sysinfo. (http://www.spinics.net/linux/fedora/libvir/msg53922.html) Thanks, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India
From: Prerna Saxena <prerna@linux.vnet.ibm.com> Date: Thu, 15 Mar 2012 16:05:26 +0530 Subject: [PATCH 1/2] Implement sysinfo on PowerPC. Libvirt on x86 parses 'dmidecode' to gather characteristics of host system. On PowerPC, this is now implemented by reading /proc/cpuinfo NOTE: memory-DIMM information is not presently implemented. Acked-by: Daniel Veillard <veillard@redhat.com> Acked-by: Daniel P Berrange <berrange@redhat.com> Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com> --- src/util/sysinfo.c | 133 +++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 131 insertions(+), 2 deletions(-) diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c index 39c7581..78efc32 100644 --- a/src/util/sysinfo.c +++ b/src/util/sysinfo.c @@ -44,6 +44,7 @@ __FUNCTION__, __LINE__, __VA_ARGS__) #define SYSINFO_SMBIOS_DECODER "dmidecode" +#define CPUINFO "/proc/cpuinfo" VIR_ENUM_IMPL(virSysinfo, VIR_SYSINFO_LAST, "smbios"); @@ -113,10 +114,138 @@ void virSysinfoDefFree(virSysinfoDefPtr def) * * Returns: a filled up sysinfo structure or NULL in case of error */ -#if defined(WIN32) || \ + +#if defined(__powerpc__) +static int +virSysinfoParseSystem(const char *base, virSysinfoDefPtr ret) +{ + char *eol = NULL; + const char *cur; + + if ((cur = strstr(base, "platform")) == NULL) + return 0; + + base = cur; + /* Account for format 'platform : XXXX'*/ + cur = strchr(cur, ':') + 1; + eol = strchr(cur, '\n'); + virSkipSpaces(&cur); + if (eol && + ((ret->system_family = strndup(cur, eol - cur)) == NULL)) + goto no_memory; + + if ((cur = strstr(base, "model")) != NULL) { + cur = strchr(cur, ':') + 1; + eol = strchr(cur, '\n'); + virSkipSpaces(&cur); + if (eol && ((ret->system_serial = strndup(cur, eol - cur)) + == NULL)) + goto no_memory; + } + + if ((cur = strstr(base, "machine")) != NULL) { + cur = strchr(cur, ':') + 1; + eol = strchr(cur, '\n'); + virSkipSpaces(&cur); + if (eol && ((ret->system_version = strndup(cur, eol - cur)) + == NULL)) + goto no_memory; + } + + return 0; + +no_memory: + return -1; +} + +static int +virSysinfoParseProcessor(const char *base, virSysinfoDefPtr ret) +{ + const char *cur; + char *eol, *tmp_base; + virSysinfoProcessorDefPtr processor; + + while((tmp_base = strstr(base, "processor")) != NULL) { + base = tmp_base; + eol = strchr(base, '\n'); + cur = strchr(base, ':') + 1; + + if (VIR_EXPAND_N(ret->processor, ret->nprocessor, 1) < 0) { + goto no_memory; + } + processor = &ret->processor[ret->nprocessor - 1]; + + virSkipSpaces(&cur); + if (eol && + ((processor->processor_socket_destination = strndup + (cur, eol - cur)) == NULL)) + goto no_memory; + + if ((cur = strstr(base, "cpu")) != NULL) { + cur = strchr(cur, ':') + 1; + eol = strchr(cur, '\n'); + virSkipSpaces(&cur); + if (eol && + ((processor->processor_type = strndup(cur, eol - cur)) + == NULL)) + goto no_memory; + } + + if ((cur = strstr(base, "revision")) != NULL) { + cur = strchr(cur, ':') + 1; + eol = strchr(cur, '\n'); + virSkipSpaces(&cur); + if (eol && + ((processor->processor_version = strndup(cur, eol - cur)) + == NULL)) + goto no_memory; + } + + base = cur; + } + + return 0; + +no_memory: + return -1; +} + +/* virSysinfoRead for PowerPC + * Gathers sysinfo data from /proc/cpuinfo */ +virSysinfoDefPtr +virSysinfoRead(void) { + virSysinfoDefPtr ret = NULL; + char *outbuf = NULL; + + if (VIR_ALLOC(ret) < 0) + goto no_memory; + + if(virFileReadAll(CPUINFO, 2048, &outbuf) < 0) { + virSmbiosReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to open %s"), CPUINFO); + return NULL; + } + + ret->nprocessor = 0; + ret->processor = NULL; + if (virSysinfoParseProcessor(outbuf, ret) < 0) + goto no_memory; + + if (virSysinfoParseSystem(outbuf, ret) < 0) + goto no_memory; + + return ret; + +no_memory: + VIR_FREE(outbuf); + return NULL; +} + +#elif defined(WIN32) || \ !(defined(__x86_64__) || \ defined(__i386__) || \ - defined(__amd64__)) + defined(__amd64__) || \ + defined(__powerpc__)) virSysinfoDefPtr virSysinfoRead(void) { /* -- 1.7.7.6 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India
On Thu, Mar 15, 2012 at 04:13:26PM +0530, Prerna wrote:
From: Prerna Saxena <prerna@linux.vnet.ibm.com> Date: Thu, 15 Mar 2012 16:05:26 +0530 Subject: [PATCH 1/2] Implement sysinfo on PowerPC.
Libvirt on x86 parses 'dmidecode' to gather characteristics of host system. On PowerPC, this is now implemented by reading /proc/cpuinfo NOTE: memory-DIMM information is not presently implemented.
Acked-by: Daniel Veillard <veillard@redhat.com> Acked-by: Daniel P Berrange <berrange@redhat.com> Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com>
Okay, I pushed this but let fix the issue raised in 2/2 as this may impact the output of this patch too, and we should not change it after the release, thanks, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
From: Prerna Saxena <prerna@linux.vnet.ibm.com> Date: Thu, 16 Feb 2012 15:33:43 +0530 Subject: [PATCH 2/2] Sysinfo : Allow x86 to fetch sysinfo from /proc/cpuinfo in the event 'dmidecode' is absent in the system. Until now, libvirt on x86 flags an error message if dmidecode is not found. With this patch, the following is a sample output on x86 when dmidecode is absent: virsh # sysinfo <sysinfo type='smbios'> <processor> <entry name='socket_destination'>0</entry> <entry name='type'>Intel(R) Xeon(R) CPU X5570 @ 2.93GHz</entry> <entry name='family'>6</entry> <entry name='manufacturer'>GenuineIntel</entry> </processor> <processor> <entry name='socket_destination'>1</entry> <entry name='type'>Intel(R) Xeon(R) CPU X5570 @ 2.93GHz</entry> <entry name='family'>6</entry> <entry name='manufacturer'>GenuineIntel</entry> </processor> ... (listing for all online CPUs) </sysinfo> Based on suggestion from Eric Blake: (http://www.redhat.com/archives/libvir-list/2012-February/msg00509.html) Also updated to display only socket-level information under '<processor>' XML tags, to bring it analogous to 'dmidecode' output. (Observed by Daniel Veillard : http://www.spinics.net/linux/fedora/libvir/msg53922.html) Acked-by: Daniel P Berrange <berrange@redhat.com> Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com> --- src/util/sysinfo.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 106 insertions(+), 4 deletions(-) diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c index 78efc32..220338f 100644 --- a/src/util/sysinfo.c +++ b/src/util/sysinfo.c @@ -598,6 +598,111 @@ no_memory: return -1; } +/* If a call to 'dmidecode' fails, + * extract basic sysinfo from /proc/cpuinfo */ + +static int +virSysinfoParseCPUInfoProcessor(const char *base, virSysinfoDefPtr ret) +{ + const char *cur; + char *eol, *tmp_base, *physical_id=NULL, *tmp_physical_id; + int physical_id_len; + virSysinfoProcessorDefPtr processor; + + while((tmp_base = strstr(base, "processor")) != NULL) { + base = tmp_base; + eol = strchr(base, '\n'); + cur = strchr(base, ':') + 1; + + tmp_physical_id = strchr(strstr(base, "physical id"), ':') + 1; + physical_id_len = strchr(tmp_physical_id, '\n') - tmp_physical_id; + + if(!physical_id) { + physical_id = tmp_physical_id; + } + else if (!strncmp(physical_id, tmp_physical_id, physical_id_len)) { + base = cur; + continue; + } + + if (VIR_EXPAND_N(ret->processor, ret->nprocessor, 1) < 0) { + goto no_memory; + } + + processor = &ret->processor[ret->nprocessor - 1]; + virSkipSpaces(&cur); + if (eol && + (processor->processor_socket_destination = + strndup(cur, eol - cur)) == NULL) + goto no_memory; + + + if ((cur = strstr(base, "vendor_id")) != NULL) { + cur = strchr(cur, ':') + 1; + eol = strchr(cur, '\n'); + virSkipSpaces(&cur); + if (eol && + ((processor->processor_manufacturer = + strndup(cur, eol - cur)) == NULL)) + goto no_memory; + } + + if ((cur = strstr(base, "cpu family")) != NULL) { + cur = strchr(cur, ':') + 1; + eol = strchr(cur, '\n'); + virSkipSpaces(&cur); + if (eol && + ((processor->processor_family = strndup(cur, eol - cur)) + == NULL)) + goto no_memory; + } + + if ((cur = strstr(base, "model name")) != NULL) { + cur = strchr(cur, ':') + 1; + eol = strchr(cur, '\n'); + virSkipSpaces(&cur); + if (eol && + ((processor->processor_type = strndup(cur, eol - cur)) + == NULL)) + goto no_memory; + } + + base = cur; + physical_id = tmp_physical_id; + } + + return 0; + +no_memory: + return -1; +} + +static virSysinfoDefPtr +virCPUInfoSysinfoRead(void) { + virSysinfoDefPtr ret = NULL; + char *outbuf = NULL; + + if (VIR_ALLOC(ret) < 0) + goto no_memory; + + if(virFileReadAll(CPUINFO, 20000, &outbuf) < 0) { + virSmbiosReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to open %s"), CPUINFO); + return NULL; + } + + ret->nprocessor = 0; + ret->processor = NULL; + if (virSysinfoParseCPUInfoProcessor(outbuf, ret) < 0) + goto no_memory; + + return ret; + +no_memory: + VIR_FREE(outbuf); + return NULL; +} + virSysinfoDefPtr virSysinfoRead(void) { char *path; @@ -607,10 +712,7 @@ virSysinfoRead(void) { path = virFindFileInPath(SYSINFO_SMBIOS_DECODER); if (path == NULL) { - virSmbiosReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to find path for %s binary"), - SYSINFO_SMBIOS_DECODER); - return NULL; + return virCPUInfoSysinfoRead(); } cmd = virCommandNewArgList(path, "-q", "-t", "0,1,4,17", NULL); -- 1.7.7.6 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India
On Thu, Mar 15, 2012 at 04:17:20PM +0530, Prerna wrote:
From: Prerna Saxena <prerna@linux.vnet.ibm.com> Date: Thu, 16 Feb 2012 15:33:43 +0530 Subject: [PATCH 2/2] Sysinfo : Allow x86 to fetch sysinfo from /proc/cpuinfo in the event 'dmidecode' is absent in the system.
Until now, libvirt on x86 flags an error message if dmidecode is not found. With this patch, the following is a sample output on x86 when dmidecode is absent:
virsh # sysinfo <sysinfo type='smbios'> <processor> <entry name='socket_destination'>0</entry> <entry name='type'>Intel(R) Xeon(R) CPU X5570 @ 2.93GHz</entry> <entry name='family'>6</entry> <entry name='manufacturer'>GenuineIntel</entry> </processor> <processor> <entry name='socket_destination'>1</entry> <entry name='type'>Intel(R) Xeon(R) CPU X5570 @ 2.93GHz</entry> <entry name='family'>6</entry> <entry name='manufacturer'>GenuineIntel</entry> </processor> ... (listing for all online CPUs) </sysinfo>
Based on suggestion from Eric Blake: (http://www.redhat.com/archives/libvir-list/2012-February/msg00509.html)
Also updated to display only socket-level information under '<processor>' XML tags, to bring it analogous to 'dmidecode' output. (Observed by Daniel Veillard : http://www.spinics.net/linux/fedora/libvir/msg53922.html)
Hum ... I tried again, there is one good news: - the output without dmidecode shows the same number of processors than with it The bad news: - the output still don't match here I get the following without dmidecode: <sysinfo type='smbios'> <processor> <entry name='socket_destination'>0</entry> <entry name='type'>Intel(R) Core(TM)2 Duo CPU E6550 @ 2.33GHz</entry> <entry name='family'>6</entry> <entry name='manufacturer'>GenuineIntel</entry> </processor> </sysinfo> and the same fields when dmidecode is present: <processor> <entry name='socket_destination'>Socket 775</entry> <entry name='type'>Central Processor</entry> <entry name='family'>Other</entry> <entry name='manufacturer'>Intel</entry> <entry name='signature'>Type 0, Family 6, Model 15, Stepping 11</entry> <entry name='version'>Intel(R) Core(TM)2 Duo CPU E6550 @ 2.33GHz</entry> <entry name='external_clock'>333 MHz</entry> <entry name='max_speed'>4000 MHz</entry> <entry name='status'>Populated, Enabled</entry> </processor> That's clearly not a subset, socket destination shows the socket number instead of the socket type, what is shown as 'type' without dmidecode is 'version' with it, family values diverge. Seems we should be able to improve this: - change the 'type' to go into 'version' - set 'status' to 'Populated, Enabled' as this is the case for any CPU shown in /proc/cpuinfo - if cpu family/model/stepping are shown in /proc/cpuinfo then try to build a subset of signature in my case I have in /proc/cpuinfo vendor_id : GenuineIntel cpu family : 6 model : 15 model name : Intel(R) Core(TM)2 Duo CPU E6550 @ 2.33GHz stepping : 11 microcode : 0xba cpu MHz : 1998.000 cache size : 4096 KB physical id : 0 we should be able to generate from it <processor> <entry name='manufacturer'>GenuineIntel</entry> <entry name='signature'>Family 6, Model 15, Stepping 11</entry> <entry name='version'>Intel(R) Core(TM)2 Duo CPU E6550 @ 2.33GHz</entry> <entry name='status'>Populated, Enabled</entry> </processor> those informations are the ones which are really useful from a management perspective, i.e. knowing that there is a plugged CPU, and which kind of CPU. More importantly the format would stay the same, allowing proper processing by an application. But currently with the current version of the patch a program parsing "signature " and "version" to make guest scheduling informations would be at best confused by the current output if dmidecode is not present. So I still suggest to refine the patch there, it's not urgent, I will apply 1/2 since that's important for PPC but let's try to fix this next week so we can carry both patches in 0.9.11 :-) I would rather fix this next week so that if we need to change the PPC output we should do it before the release. thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Thu, Mar 15, 2012 at 04:17:20PM +0530, Prerna wrote:
From: Prerna Saxena <prerna@linux.vnet.ibm.com> Date: Thu, 16 Feb 2012 15:33:43 +0530 Subject: [PATCH 2/2] Sysinfo : Allow x86 to fetch sysinfo from /proc/cpuinfo in the event 'dmidecode' is absent in the system.
Until now, libvirt on x86 flags an error message if dmidecode is not found. With this patch, the following is a sample output on x86 when dmidecode is absent: [...] + if(!physical_id) { + physical_id = tmp_physical_id; + } + else if (!strncmp(physical_id, tmp_physical_id, physical_id_len)) { + base = cur; + continue; + }
BTW this strncmp shold be replace by the following patch, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (2)
-
Daniel Veillard -
Prerna