[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2] sysinfo: Fix reports on ARM



On Wed, May 13, 2015 at 01:55:43PM +0200, Michal Privoznik wrote:
> Due to a kernel commit (b4b8f770e), cpuinfo format has changed on
> ARMs. Firstly, 'Processor: ...' may not be reported, it's
> replaced by 'model name: ...'. Secondly, the "Processor" string
> may occur in CPU name, e.g. 'ARMv7 Processor rev 5 (v7l)'.
> Therefore, we must firstly look for 'model name' and then for
> 'Processor' if not found.
> Moreover, lines in the cpuinfo file are shuffled, so we better
> not manipulate the pointer to start of internal buffer as we may
> lost some info.
> 
> Signed-off-by: Michal Privoznik <mprivozn redhat com>

[snip]

> diff --git a/tests/sysinfodata/arm-rpi2cpuinfo.data b/tests/sysinfodata/arm-rpi2cpuinfo.data
> new file mode 100644
> index 0000000..41fde00
> --- /dev/null
> +++ b/tests/sysinfodata/arm-rpi2cpuinfo.data
> @@ -0,0 +1,43 @@
> +processor       : 0
> +model name      : ARMv7 Processor rev 5 (v7l)
> +BogoMIPS        : 38.40
> +Features        : half thumb fastmult vfp edsp neon vfpv3 tls vfpv4 idiva idivt vfpd32 lpae evtstrm 
> +CPU implementer : 0x41
> +CPU architecture: 7
> +CPU variant     : 0x0
> +CPU part        : 0xc07
> +CPU revision    : 5

It occurs to me that our code would clearer and more robust if we changed the
way we dealt with the cpu info files from proc. Instead of doing all the insane
strchr/strstr searches and pointer manipulation we could usefully have a generic
parsing method that does:

 - Read file contents
 - Split file into lines
 - For each line
   - Split on ':'
   - Strip whitespace from each part
   - Store left hand side as key, right hand side as value in a virTypedParameter
 - Returns an array of arrays of virTypedParameter

Then, the code for extract information from the file merely has to iterate
over the outer array to get each processor. For each processor it can then
do exact matches on the key name. This avoids the kind of issue where
we're matching on 'processor' but that can occurr in the value too.


I've no objection to your proposed patch though. This is just a thought
for someone motivated enough to cleanup this area of code.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]