[libvirt] [Resending][PATCH v2 0/2] Sysinfo improvements for PowerPC & x86.
Rebasing and resending the sysinfo fixes. 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 from v1: ------------------ * Rebased and cleanups done. -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India
From: Prerna Saxena <prerna@linux.vnet.ibm.com> Date: Tue, 13 Mar 2012 16:55: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 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
From: Prerna Saxena <prerna@linux.vnet.ibm.com> Date: Tue, 13 Mar 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: (http://www.redhat.com/archives/libvir-list/2012-February/msg00509.html) Acked-by: Daniel P Berrange <berrange@redhat.com> Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com> --- src/util/sysinfo.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 93 insertions(+), 4 deletions(-) diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c index 78efc32..290b69f 100644 --- a/src/util/sysinfo.c +++ b/src/util/sysinfo.c @@ -598,6 +598,98 @@ 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; + 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, "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; + } + + 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 +699,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 Tue, Mar 13, 2012 at 11:10:16AM +0530, Prerna Saxena wrote:
From: Prerna Saxena <prerna@linux.vnet.ibm.com> Date: Tue, 13 Mar 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: (http://www.redhat.com/archives/libvir-list/2012-February/msg00509.html)
Acked-by: Daniel P Berrange <berrange@redhat.com> Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com> --- src/util/sysinfo.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 93 insertions(+), 4 deletions(-)
diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c index 78efc32..290b69f 100644 --- a/src/util/sysinfo.c +++ b/src/util/sysinfo.c @@ -598,6 +598,98 @@ no_memory: return -1; }
+/* If a call to 'dmidecode' fails, + * extract basic sysinfo from /proc/cpuinfo */ [...] virSysinfoDefPtr virSysinfoRead(void) { char *path; @@ -607,10 +699,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);
Hi Prerna, that sounds like a good idea, and the patch seems to work but I have doubt with the usefulness in its current form. Let me explain: with dmidecode available on my system I get: ... <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> ... without dmidecode and your patch plugged in I get <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> <processor> <entry name='socket_destination'>1</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> so basically we get informations, some are available in both case but differently, and worse, in the fallback case we get 2 physical processor entries (I have only one) which is of course different from the single processor that we get with dmidecode. So 1/ is seems to me the fallback data can't be parsed programmatically as a replacement of the original ones 2/ the data may be misunderstood and lead to erroneous decision for example a schedule may start to stack 2 time more load on my machine based on the difference of report. So I'm a bit worried about applying it as-is, I'm afraid we need to reconcile the output (as much as possible considering there is less data) between both cases. That said I think patch 1/2 looks fine to me, and could probably be applied as-is, 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/
Hi Daniel, Thank you for taking a look. On 03/14/2012 02:29 PM, Daniel Veillard wrote:
On Tue, Mar 13, 2012 at 11:10:16AM +0530, Prerna Saxena wrote:
From: Prerna Saxena <prerna@linux.vnet.ibm.com> Date: Tue, 13 Mar 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: (http://www.redhat.com/archives/libvir-list/2012-February/msg00509.html)
Acked-by: Daniel P Berrange <berrange@redhat.com> Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com> --- src/util/sysinfo.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 93 insertions(+), 4 deletions(-)
... [snip]..
Hi Prerna,
that sounds like a good idea, and the patch seems to work but I have doubt with the usefulness in its current form. Let me explain:
with dmidecode available on my system I get:
... <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> ...
without dmidecode and your patch plugged in I get
<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> <processor> <entry name='socket_destination'>1</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>
so basically we get informations, some are available in both case but differently, and worse, in the fallback case we get 2 physical processor entries (I have only one) which is of course different from the single processor that we get with dmidecode.
So 1/ is seems to me the fallback data can't be parsed programmatically as a replacement of the original ones 2/ the data may be misunderstood and lead to erroneous decision for example a schedule may start to stack 2 time more load on my machine based on the difference of report.
So I'm a bit worried about applying it as-is, I'm afraid we need to reconcile the output (as much as possible considering there is less data) between both cases.
Thanks for pointing this out. I investigated this discrepancy, and discovered that 'dmidecode' presents a listing of processor *cores*. However, for /proc/cpuinfo, all hardware threads in a processor show up as independent processors. So, while dmidecode correctly reads that my system has a single core, /proc/cpuinfo reports two hardware threads in the core as two independent logical CPUs. To sort this out, one alternative would be to parse the physical_id in /proc/cpuinfo -- this would be identical for all logical processors in a given core, and thus can be used to report the number of cores in the system. Will send a modified patch asap.
That said I think patch 1/2 looks fine to me, and could probably be applied as-is,
Thanks! Would you want to apply it as-is, or shall I send a rebased version ?
Daniel
Regards, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India
On Thu, Mar 15, 2012 at 11:21:09AM +0530, Prerna Saxena wrote:
Hi Daniel, Thank you for taking a look.
On 03/14/2012 02:29 PM, Daniel Veillard wrote:
On Tue, Mar 13, 2012 at 11:10:16AM +0530, Prerna Saxena wrote:
From: Prerna Saxena <prerna@linux.vnet.ibm.com> Date: Tue, 13 Mar 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: (http://www.redhat.com/archives/libvir-list/2012-February/msg00509.html)
Acked-by: Daniel P Berrange <berrange@redhat.com> Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com> --- src/util/sysinfo.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 93 insertions(+), 4 deletions(-)
... [snip]..
Hi Prerna,
that sounds like a good idea, and the patch seems to work but I have doubt with the usefulness in its current form. Let me explain:
with dmidecode available on my system I get:
... <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> ...
without dmidecode and your patch plugged in I get
<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> <processor> <entry name='socket_destination'>1</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>
so basically we get informations, some are available in both case but differently, and worse, in the fallback case we get 2 physical processor entries (I have only one) which is of course different from the single processor that we get with dmidecode.
So 1/ is seems to me the fallback data can't be parsed programmatically as a replacement of the original ones 2/ the data may be misunderstood and lead to erroneous decision for example a schedule may start to stack 2 time more load on my machine based on the difference of report.
So I'm a bit worried about applying it as-is, I'm afraid we need to reconcile the output (as much as possible considering there is less data) between both cases.
Thanks for pointing this out. I investigated this discrepancy, and discovered that 'dmidecode' presents a listing of processor *cores*. However, for /proc/cpuinfo, all hardware threads in a processor show up as independent processors. So, while dmidecode correctly reads that my system has a single core, /proc/cpuinfo reports two hardware threads in the core as two independent logical CPUs. To sort this out, one alternative would be to parse the physical_id in /proc/cpuinfo -- this would be identical for all logical processors in a given core, and thus can be used to report the number of cores in the system. Will send a modified patch asap.
That said I think patch 1/2 looks fine to me, and could probably be applied as-is,
Thanks! Would you want to apply it as-is, or shall I send a rebased version ?
Well if you're fixing 2/2 before end of next week, I suggest to apply both together and hence rebase 1/2 when you submit 2/2 v3 :) 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 03/15/2012 11:37 AM, Daniel Veillard wrote:
On Thu, Mar 15, 2012 at 11:21:09AM +0530, Prerna Saxena wrote:
Hi Daniel, Thank you for taking a look.
On 03/14/2012 02:29 PM, Daniel Veillard wrote:
On Tue, Mar 13, 2012 at 11:10:16AM +0530, Prerna Saxena wrote:
From: Prerna Saxena <prerna@linux.vnet.ibm.com> Date: Tue, 13 Mar 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: (http://www.redhat.com/archives/libvir-list/2012-February/msg00509.html)
Acked-by: Daniel P Berrange <berrange@redhat.com> Signed-off-by: Prerna Saxena <prerna@linux.vnet.ibm.com> --- src/util/sysinfo.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 93 insertions(+), 4 deletions(-)
... [snip]..
Hi Prerna,
that sounds like a good idea, and the patch seems to work but I have doubt with the usefulness in its current form. Let me explain:
with dmidecode available on my system I get:
... <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> ...
without dmidecode and your patch plugged in I get
<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> <processor> <entry name='socket_destination'>1</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>
so basically we get informations, some are available in both case but differently, and worse, in the fallback case we get 2 physical processor entries (I have only one) which is of course different from the single processor that we get with dmidecode.
So 1/ is seems to me the fallback data can't be parsed programmatically as a replacement of the original ones 2/ the data may be misunderstood and lead to erroneous decision for example a schedule may start to stack 2 time more load on my machine based on the difference of report.
So I'm a bit worried about applying it as-is, I'm afraid we need to reconcile the output (as much as possible considering there is less data) between both cases.
Thanks for pointing this out. I investigated this discrepancy, and discovered that 'dmidecode' presents a listing of processor *cores*. However, for /proc/cpuinfo, all hardware threads in a processor show up as independent processors. So, while dmidecode correctly reads that my system has a single core, /proc/cpuinfo reports two hardware threads in the core as two independent logical CPUs. To sort this out, one alternative would be to parse the physical_id in /proc/cpuinfo -- this would be identical for all logical processors in a given core, and thus can be used to report the number of cores in the system. Will send a modified patch asap.
Hi Daniel, I just realised a correction in the explanation above -- dmidecode only reveals a per-socket listing, while /proc/cpuinfo lists all the physical cores within a socket. So if demidecode lists single entry for a processor, it can be inferred that the machine in question has one socket. /proc/cpuinfo will have listings for each physical core in that socket, plus the hardware threads available. As mentioned earlier, I will be happy to spin a patch that uses 'physical_id' (constant for all cores in a socket) to provide a socket-level information. This will attain parity with 'dmidecode' output and will report *one socket* for such a machine, under the 'processor' XML tag.( Which could be a little misleading) However, I am curious -- what benefit would the number of sockets be to a libvirt user? I expect users would mostly care about number of available CPU cores to take scheduling decisions. Am I missing a use-case for exclusive need of socket-level information?
That said I think patch 1/2 looks fine to me, and could probably be applied as-is,
Thanks! Would you want to apply it as-is, or shall I send a rebased version ?
Well if you're fixing 2/2 before end of next week, I suggest to apply both together and hence rebase 1/2 when you submit 2/2 v3 :)
Daniel
Regards, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India
On Thu, Mar 15, 2012 at 12:28:05PM +0530, Prerna Saxena wrote:
On 03/15/2012 11:37 AM, Daniel Veillard wrote:
On Thu, Mar 15, 2012 at 11:21:09AM +0530, Prerna Saxena wrote:
Hi Daniel, Thank you for taking a look. [...] Thanks for pointing this out. I investigated this discrepancy, and discovered that 'dmidecode' presents a listing of processor *cores*. However, for /proc/cpuinfo, all hardware threads in a processor show up as independent processors. So, while dmidecode correctly reads that my system has a single core, /proc/cpuinfo reports two hardware threads in the core as two independent logical CPUs. To sort this out, one alternative would be to parse the physical_id in /proc/cpuinfo -- this would be identical for all logical processors in a given core, and thus can be used to report the number of cores in the system. Will send a modified patch asap.
Hi Daniel, I just realised a correction in the explanation above -- dmidecode only reveals a per-socket listing, while /proc/cpuinfo lists all the physical cores within a socket.
So if demidecode lists single entry for a processor, it can be inferred that the machine in question has one socket. /proc/cpuinfo will have listings for each physical core in that socket, plus the hardware threads available.
As mentioned earlier, I will be happy to spin a patch that uses 'physical_id' (constant for all cores in a socket) to provide a socket-level information. This will attain parity with 'dmidecode' output and will report *one socket* for such a machine, under the 'processor' XML tag.( Which could be a little misleading)
However, I am curious -- what benefit would the number of sockets be to a libvirt user? I expect users would mostly care about number of available CPU cores to take scheduling decisions. Am I missing a use-case for exclusive need of socket-level information?
The question at this point is not whether the information is better or not, it must be the same in the fallback case. The informations won't be missing, it can be gathered by 'virsh nodeinfo' and equivalent API. Point of patch 2/2 is to provide some identical informations in the case dmidecode is missing. 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 03/15/2012 12:40 PM, Daniel Veillard wrote:
On Thu, Mar 15, 2012 at 12:28:05PM +0530, Prerna Saxena wrote:
On 03/15/2012 11:37 AM, Daniel Veillard wrote:
On Thu, Mar 15, 2012 at 11:21:09AM +0530, Prerna Saxena wrote:
Hi Daniel, Thank you for taking a look. [...] Thanks for pointing this out. I investigated this discrepancy, and discovered that 'dmidecode' presents a listing of processor *cores*. However, for /proc/cpuinfo, all hardware threads in a processor show up as independent processors. So, while dmidecode correctly reads that my system has a single core, /proc/cpuinfo reports two hardware threads in the core as two independent logical CPUs. To sort this out, one alternative would be to parse the physical_id in /proc/cpuinfo -- this would be identical for all logical processors in a given core, and thus can be used to report the number of cores in the system. Will send a modified patch asap.
Hi Daniel, I just realised a correction in the explanation above -- dmidecode only reveals a per-socket listing, while /proc/cpuinfo lists all the physical cores within a socket.
So if demidecode lists single entry for a processor, it can be inferred that the machine in question has one socket. /proc/cpuinfo will have listings for each physical core in that socket, plus the hardware threads available.
As mentioned earlier, I will be happy to spin a patch that uses 'physical_id' (constant for all cores in a socket) to provide a socket-level information. This will attain parity with 'dmidecode' output and will report *one socket* for such a machine, under the 'processor' XML tag.( Which could be a little misleading)
However, I am curious -- what benefit would the number of sockets be to a libvirt user? I expect users would mostly care about number of available CPU cores to take scheduling decisions. Am I missing a use-case for exclusive need of socket-level information?
The question at this point is not whether the information is better or not, it must be the same in the fallback case. The informations won't be missing, it can be gathered by 'virsh nodeinfo' and equivalent API. Point of patch 2/2 is to provide some identical informations in the case dmidecode is missing.
Okay. I'll send out a v3 of my patch series incorporating changes asap :) Regards, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India
participants (3)
-
Daniel Veillard -
Prerna -
Prerna Saxena