Re: [libvirt] [PATCH] virsh: Don't output node frequency if unknown
On 07/15/2015 08:17 AM, Martin Kletzander wrote:
On Tue, Jul 14, 2015 at 09:03:59AM -0400, John Ferlan wrote:
>
>
> On 07/10/2015 05:07 AM, Martin Kletzander wrote:
>> Commit ed8155eafbff5c5ca0bdfe84a8388f58b718c2f9 documented that
>> mhz field in virNodeInfo might be 0 if the frequency is unknown. Modify
>> virsh to know about that.
>>
>
> While 0 may be (or is) an invalid value would it perhaps be better to
> document in virsh.pod rather than just not supply it? Or perhaps do
> both? By just not supplying it could lead to the question of why is it
> there for this architecture, but not the other.
>
> Another option would be to print "(unknown)" rather than nothing (and of
> course document it).
>
> If you add some sort of documentation element to this, then I'm fine
> with it. Maybe this'll cause others to provide feedback/thoughts too.
>
> John
>
Would squashing in the following diff suffice?
diff --git i/tools/virsh.pod w/tools/virsh.pod
index bcfa165ada15..6351b7d67dc3 100644
--- i/tools/virsh.pod
+++ w/tools/virsh.pod
@@ -295,7 +295,8 @@ Print the XML representation of the hypervisor
sysinfo, if available.
Returns basic information about the node, like number and type of CPU,
and size of the physical memory. The output corresponds to virNodeInfo
structure. Specifically, the "CPU socket(s)" field means number of CPU
-sockets per NUMA cell.
+sockets per NUMA cell. Specific information might me omitted in case
+libvirt was unable to gather it.
=item B<nodecpumap> [I<--pretty>]
--
How about "The information libvirt displays is dependent upon what each
architecture may provide." (or just provides instead of may provide).
Whatever works best to indicate that something could be missing.
John
>
>> Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
>> ---
>> tools/virsh-host.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/virsh-host.c b/tools/virsh-host.c
>> index 66f7fd9e62e4..7a223931b152 100644
>> --- a/tools/virsh-host.c
>> +++ b/tools/virsh-host.c
>> @@ -606,7 +606,8 @@ cmdNodeinfo(vshControl *ctl, const vshCmd *cmd
>> ATTRIBUTE_UNUSED)
>> }
>> vshPrint(ctl, "%-20s %s\n", _("CPU model:"),
info.model);
>> vshPrint(ctl, "%-20s %d\n", _("CPU(s):"), info.cpus);
>> - vshPrint(ctl, "%-20s %d MHz\n", _("CPU frequency:"),
info.mhz);
>> + if (info.mhz)
>> + vshPrint(ctl, "%-20s %d MHz\n", _("CPU frequency:"),
info.mhz);
>> vshPrint(ctl, "%-20s %d\n", _("CPU socket(s):"),
info.sockets);
>> vshPrint(ctl, "%-20s %d\n", _("Core(s) per socket:"),
info.cores);
>> vshPrint(ctl, "%-20s %d\n", _("Thread(s) per core:"),
>> info.threads);
>>