[libvirt] [PATCH][PowerPC] Implement sysinfo for libvirt on PowerPC

From: Prerna Saxena <prerna@linux.vnet.ibm.com> Date: Tue, 7 Feb 2012 16:55:26 +0530 Subject: [PATCH] Implement sysinfo on PowerPC. Libvirt on x86 parses 'dmidecode' to gather characteristics of host system, which are then reflected to libvirt users by virSysinfoRead(), invoked by 'virsh sysinfo'. This patch implements it on PowerPC by reading /proc/cpuinfo. The presently available fields in 'sysinfo' are strongly tied to dmidecode output fields. This patch attempts to retrofit the information available in PowerPC to appropriate sysinfo fields. I will be happy to change the organization of this information to if there are expected outputs for individual fields. (I couldnt find any documentation which explained what each sysinfo field was expected to convey.) TODOS: 1. Adding Memory DIMM information 2) Firmware (<bios>) details. 3) Expand <processor> tag to have more fields available. Example output on PowerPC : virsh # sysinfo <sysinfo type='smbios'> <system> <entry name='version'>PowerNV 8246-L2C</entry> <entry name='serial'>8246-L2C</entry> <entry name='family'>PowerNV</entry> </system> <processor> <entry name='socket_destination'>0</entry> <entry name='type'>POWER7 (raw), altivec supported</entry> <entry name='version'>2.3 (pvr 003f 0203)</entry> </processor> <processor> <entry name='socket_destination'>4</entry> <entry name='type'>POWER7 (raw), altivec supported</entry> <entry name='version'>2.3 (pvr 003f 0203)</entry> </processor> ...... --- src/util/sysinfo.c | 146 +++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 144 insertions(+), 2 deletions(-) diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c index de3108a..4a2ac63 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,151 @@ void virSysinfoDefFree(virSysinfoDefPtr def) * * Returns: a filled up sysinfo structure or NULL in case of error */ -#if defined(WIN32) || \ + +#if defined(__powerpc__) +static char* +virSysinfoParseSystem(char *base, virSysinfoDefPtr ret) +{ + char *cur, *eol = NULL; + + if ((cur = strstr(base, "platform")) == NULL) + return base; + + base = cur; + /* Account for format 'platform : XXXX'*/ + cur = strchr(cur, ':') + 1; + eol = strchr(cur, '\n'); + if ((eol) && + ((ret->system_family = strndup(cur, eol - cur)) == NULL)) + goto no_memory; + virSkipSpaces(&(ret->system_family)); + + if ((cur = strstr(base, "model")) != NULL) { + cur = strchr(cur, ':') + 1; + eol = strchr(cur, '\n'); + if ((eol) && ((ret->system_serial = strndup(cur, eol - cur)) == NULL)) + goto no_memory; + virSkipSpaces(&(ret->system_serial)); + } + + if ((cur = strstr(base, "machine")) != NULL) { + cur = strchr(cur, ':') + 1; + eol = strchr(cur, '\n'); + if ((eol) && ((ret->system_version = strndup(cur, eol - cur)) == NULL)) + goto no_memory; + virSkipSpaces(&(ret->system_version)); + } + + ret->system_manufacturer = NULL; + ret->system_product = NULL; + ret->system_uuid = NULL; + ret->system_sku = NULL; + + return cur; + +no_memory: + return NULL; +} + +static char * +virSysinfoParseProcessor(char *base, virSysinfoDefPtr ret) +{ + char *cur, *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]; + if ((eol) && + ((processor->processor_socket_destination = strndup(cur, eol - cur)) == NULL)) + goto no_memory; + virSkipSpaces(&(processor->processor_socket_destination)); + + if ((cur = strstr(base, "cpu")) != NULL) { + cur = strchr(cur, ':') + 1; + eol = strchr(cur, '\n'); + if ((eol) && + ((processor->processor_type = strndup(cur, eol - cur)) == NULL)) + goto no_memory; + virSkipSpaces(&(processor->processor_type)); + } + + if ((cur = strstr(base, "revision")) != NULL) { + cur = strchr(cur, ':') + 1; + eol = strchr(cur, '\n'); + if ((eol) && + ((processor->processor_version = strndup(cur, eol - cur)) == NULL)) + goto no_memory; + virSkipSpaces(&(processor->processor_version)); + } + + /* Mark non-required fields as 'NULL' */ + processor->processor_family = NULL; + processor->processor_manufacturer = NULL; + processor->processor_signature = NULL; + processor->processor_external_clock = NULL; + processor->processor_max_speed = NULL; + processor->processor_status = NULL; + processor->processor_serial_number = NULL; + processor->processor_part_number = NULL; + + base = cur; + } + + return base; + +no_memory: + return NULL; +} + +/* virSysinfoRead for PowerPC */ +virSysinfoDefPtr +virSysinfoRead(void) { + virSysinfoDefPtr ret = NULL; + char *outbuf = NULL; + + if (VIR_ALLOC(ret) < 0) + goto no_memory; + + /* mark irrelevant fields as 'NULL' */ + ret->bios_vendor = NULL; + ret->bios_version = NULL; + ret->bios_date = NULL; + ret->bios_release = NULL; + + 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) == NULL) + goto no_memory; + + if (virSysinfoParseSystem(outbuf, ret) == NULL) + 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 Awaiting feedback, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India

On 02/09/2012 01:47 AM, Prerna Saxena wrote:
From: Prerna Saxena <prerna@linux.vnet.ibm.com> Date: Tue, 7 Feb 2012 16:55:26 +0530 Subject: [PATCH] Implement sysinfo on PowerPC.
Libvirt on x86 parses 'dmidecode' to gather characteristics of host system, which are then reflected to libvirt users by virSysinfoRead(), invoked by 'virsh sysinfo'. This patch implements it on PowerPC by reading /proc/cpuinfo.
The presently available fields in 'sysinfo' are strongly tied to dmidecode output fields. This patch attempts to retrofit the information available in PowerPC to appropriate sysinfo fields. I will be happy to change the organization of this information to if there are expected outputs for individual fields. (I couldnt find any documentation which explained what each sysinfo field was expected to convey.)
At this point, I think it might be worth waiting until post-0.9.10, and rebasing this on the latest. In particular,
+ +#if defined(__powerpc__) +static char* +virSysinfoParseSystem(char *base, virSysinfoDefPtr ret)
We just recently changed the signature of this function in the x86 case, so the two implementations should probably have a similar signature.
+ + ret->system_manufacturer = NULL; + ret->system_product = NULL;
Setting these to NULL is redundant, since ret was just freshly allocated.
+ ret->system_uuid = NULL;
Is there any alternative way to obtain the host UUID that we could fill in here?
+/* virSysinfoRead for PowerPC */ +virSysinfoDefPtr +virSysinfoRead(void) { + virSysinfoDefPtr ret = NULL; + char *outbuf = NULL; + + if (VIR_ALLOC(ret) < 0) + goto no_memory; + + /* mark irrelevant fields as 'NULL' */ + ret->bios_vendor = NULL; + ret->bios_version = NULL; + ret->bios_date = NULL; + ret->bios_release = NULL;
Again, redundant, since VIR_ALLOC() already took care of that.
+ + if(virFileReadAll(CPUINFO, 2048, &outbuf) < 0) { + virSmbiosReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to open %s"),CPUINFO);
Nit: space after ','. Overall, the idea looks nice. I'm also wondering how much of that implementation of parsing /proc/cpuinfo can be reused on x86 in the cases where dmidecode is not accessible (such as when running qemu:///session as non-root)? Filling out what we can seems like a good idea. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Hi Eric, Thanks for taking a look. On 02/10/2012 04:33 AM, Eric Blake wrote:
On 02/09/2012 01:47 AM, Prerna Saxena wrote:
From: Prerna Saxena <prerna@linux.vnet.ibm.com> Date: Tue, 7 Feb 2012 16:55:26 +0530 Subject: [PATCH] Implement sysinfo on PowerPC.
Libvirt on x86 parses 'dmidecode' to gather characteristics of host system, which are then reflected to libvirt users by virSysinfoRead(), invoked by 'virsh sysinfo'. This patch implements it on PowerPC by reading /proc/cpuinfo.
The presently available fields in 'sysinfo' are strongly tied to dmidecode output fields. This patch attempts to retrofit the information available in PowerPC to appropriate sysinfo fields. I will be happy to change the organization of this information to if there are expected outputs for individual fields. (I couldnt find any documentation which explained what each sysinfo field was expected to convey.)
At this point, I think it might be worth waiting until post-0.9.10, and rebasing this on the latest. In particular,
I'll rebase the patch once the merge window is open again :)
+ +#if defined(__powerpc__) +static char* +virSysinfoParseSystem(char *base, virSysinfoDefPtr ret)
We just recently changed the signature of this function in the x86 case, so the two implementations should probably have a similar signature.
Sure, I'll modify this.
+ + ret->system_manufacturer = NULL; + ret->system_product = NULL;
Setting these to NULL is redundant, since ret was just freshly allocated.
Thanks for pointing this out, will remove.
+ ret->system_uuid = NULL;
Is there any alternative way to obtain the host UUID that we could fill in here?
I checked that 'UUID' is a part of SMBIOS specification, and therefore not available on PowerPC system. We do specify a system serial -- maybe that would suffice ?
+/* virSysinfoRead for PowerPC */ +virSysinfoDefPtr +virSysinfoRead(void) { + virSysinfoDefPtr ret = NULL; + char *outbuf = NULL; + + if (VIR_ALLOC(ret) < 0) + goto no_memory; + + /* mark irrelevant fields as 'NULL' */ + ret->bios_vendor = NULL; + ret->bios_version = NULL; + ret->bios_date = NULL; + ret->bios_release = NULL;
Again, redundant, since VIR_ALLOC() already took care of that.
Will remove this.
+ + if(virFileReadAll(CPUINFO, 2048, &outbuf) < 0) { + virSmbiosReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to open %s"),CPUINFO);
Nit: space after ','.
I'll clean this up.
Overall, the idea looks nice.
I'm also wondering how much of that implementation of parsing /proc/cpuinfo can be reused on x86 in the cases where dmidecode is not accessible (such as when running qemu:///session as non-root)? Filling out what we can seems like a good idea.
It would be nice to do this. However, the ppc-specific parsing cannot be reused since format of /proc/cpuinfo is very much tied to architecture. We'll separately need to write a parser that understands the format of /proc/cpuinfo for x86. Thanks, -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India
participants (2)
-
Eric Blake
-
Prerna Saxena