[libvirt] [PATCH v3 5/5] util: Don't report CPU frequency for ARM hosts

[John Ferlan]

On 12/14/2017 07:34 AM, Andrea Bolognani wrote:
> Some ARM platforms, such as the original Raspberry Pi, report the
> CPU frequency in the BogoMIPS field of /proc/cpuinfo, so libvirt
> parsed that field and returned it through its API.
> 
> However, not only many more boards don't report any value there,
> but several - including ARMv8-based server hardware, and even the
> more recent Raspberry Pi 3 - use this field as originally intended:
> to report the BogoMIPS value instead of the CPU frequency.
> 
> Since we have no way of detecting how the field is being used,
> it's better to report no information at all rather than something
> ludicrous like "your shiny 96-core aarch64 virtualization host's
> CPUs are running at a whopping 100 MHz".
> 
> Signed-off-by: Andrea Bolognani <abologna at redhat.com>
> ---
>  src/util/virhostcpu.c                                       | 6 ++++--
>  tests/virhostcpudata/linux-aarch64-rhel74-moonshot.expected | 2 +-
>  tests/virhostcpudata/linux-armv6l-raspberrypi.expected      | 2 +-
>  3 files changed, 6 insertions(+), 4 deletions(-)
> 

Suggestion - follow cmdNodeinfo for virhostcputest.c and don't print the
MHz if it's 0.  It's the indication that the data wasn't obtainable. I
think printing 0 is perhaps just as bad as printing the erroneous 100.

I trust you can make the adjustment without another round of patches.

Reviewed-by: John Ferlan <jferlan at redhat.com>

John

> diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
> index 9c9f362de..1c9ee5921 100644
> --- a/src/util/virhostcpu.c
> +++ b/src/util/virhostcpu.c
> @@ -590,12 +590,14 @@ virHostCPUParseFrequency(FILE *cpuinfo,
>      const char *prefix = NULL;
>      char line[1024];
>  
> +    /* No sensible way to retrieve CPU frequency */
> +    if (ARCH_IS_ARM(arch))
> +        return 0;
> +
>      if (ARCH_IS_X86(arch))
>          prefix = "cpu MHz";
>      else if (ARCH_IS_PPC(arch))
>          prefix = "clock";
> -    else if (ARCH_IS_ARM(arch))
> -        prefix = "BogoMIPS";
>  
>      if (!prefix) {
>          VIR_WARN("%s is not supported by the %s parser",
> diff --git a/tests/virhostcpudata/linux-aarch64-rhel74-moonshot.expected b/tests/virhostcpudata/linux-aarch64-rhel74-moonshot.expected
> index 24ff0ea0b..6776aa6c2 100644
> --- a/tests/virhostcpudata/linux-aarch64-rhel74-moonshot.expected
> +++ b/tests/virhostcpudata/linux-aarch64-rhel74-moonshot.expected
> @@ -1 +1 @@
> -CPUs: 8/8, MHz: 100, Nodes: 1, Sockets: 1, Cores: 8, Threads: 1
> +CPUs: 8/8, MHz: 0, Nodes: 1, Sockets: 1, Cores: 8, Threads: 1
> diff --git a/tests/virhostcpudata/linux-armv6l-raspberrypi.expected b/tests/virhostcpudata/linux-armv6l-raspberrypi.expected
> index 146bd073e..1c4c713d5 100644
> --- a/tests/virhostcpudata/linux-armv6l-raspberrypi.expected
> +++ b/tests/virhostcpudata/linux-armv6l-raspberrypi.expected
> @@ -1 +1 @@
> -CPUs: 1/1, MHz: 697, Nodes: 1, Sockets: 1, Cores: 1, Threads: 1
> +CPUs: 1/1, MHz: 0, Nodes: 1, Sockets: 1, Cores: 1, Threads: 1
>