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

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

 



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@xxxxxxxxxx>

[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 :|

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]