[libvirt] [PATCH] virsh: Don't output node frequency if unknown
Commit ed8155eafbff5c5ca0bdfe84a8388f58b718c2f9 documented that mhz field in virNodeInfo might be 0 if the frequency is unknown. Modify virsh to know about that. Signed-off-by: Martin Kletzander <mkletzan@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); -- 2.4.5
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
Signed-off-by: Martin Kletzander <mkletzan@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);
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>] --
Signed-off-by: Martin Kletzander <mkletzan@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);
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@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);
participants (2)
-
John Ferlan -
Martin Kletzander