On 01/09/2018 04:06 AM, Andrea Bolognani wrote:
> On Mon, 2018-01-08 at 11:19 -0500, John Ferlan wrote:
>> 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'.
>>>
>>> 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.
>
> On the other hand, the virNodeInfo struct is part of the public API
> and clients are going to have to deal sensibly with zeros being in
> there. It's even documented:
>
> struct _virNodeInfo {
> ...
> unsigned int mhz; /* expected CPU frequency, 0 if not known or
> on unusual architectures */
>
> virsh should of course avoid formatting the information altogether
> in that case, and it does. But when it comes to tests, we don't need
> to make the output pretty or omit information: we're basically just
> dumping the contents of the structure, so it's okay for the zero to
> be there.
>
> So I guess what I'm trying to say is that Peter convinced me this
> patch might not be such a good idea after all. Are you okay with
> dropping it?
>
I'm fine with dropping it.
John
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
