Re: [libvirt] [PATCH] tests: Only format the CPU frequency if it's known
On 01/08/2018 09:50 AM, Peter Krempa wrote:
On Mon, Jan 08, 2018 at 15:10:29 +0100, Andrea Bolognani wrote:
> Instead of formatting 'MHz: 0', which can be confusing, skip the
> field altogether. This behavior is consistent with that of 'virsh
> nodeinfo'.
>
> Suggested-by: John Ferlan <jferlan(a)redhat.com>
> Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
> ---
> .../linux-aarch64-f21-mustang.expected | 2 +-
> .../linux-aarch64-rhel74-moonshot.expected | 2 +-
> .../linux-aarch64-rhelsa-3.19.0-mustang.expected | 2 +-
> .../linux-armv6l-raspberrypi.expected | 2 +-
> tests/virhostcputest.c | 28 +++++++++++++++++-----
> 5 files changed, 26 insertions(+), 10 deletions(-)
Well, these are tests, so confusion really should not be a problem.
Formatting all the values unconditionally has a benefit that you don't
have to look at the code to see when some are formatted, but rather know
the raw value instead.
I'd suggest to not push this.
My suggestion was less based on confusion and more on what's the
purpose. While I understand Peter's point - looking at the code and
digging into the data would still perhaps be necessary because we don't
know if 0 was because we couldn't get data or if it's not relevant.
Distinguishing between 0 as a read value (haha) and 0 as a non read
value is not possible.
As I read Andrea's commit message from patch 5/5 (now commit id
a63ea8141) it seemed perhaps a better thing to do to not report MHz
since it's either not reported or incorrectly reported.
The crux being if MHz is something that one cannot ascertain from an ARM
processor at all, then why report it at all. In this case, there is no
valid raw value.
If you want it, you have my ACK/R-B.
John