Re: [PATCH v2 8/8] virsh: Extend virsh dominfo to display if managed state exists

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/15/2011 03:06 AM, Osier Yang wrote:
---
  tools/virsh.c |    8 ++++++++
  1 files changed, 8 insertions(+), 0 deletions(-)

Your rebuttals to my arguments about positioning are sound, so I'm okay with leaving the positioning of this output prior to "Security" strings. However, on re-reading it, I see one more issue:


diff --git a/tools/virsh.c b/tools/virsh.c
index 8a62612..120f3c8 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -2366,6 +2366,7 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd)
      int autostart;
      unsigned int id;
      char *str, uuid[VIR_UUID_STRING_BUFLEN];
+    int has_managed_state = 0;

      if (!vshConnectionUsability(ctl, ctl->conn))
          return false;
@@ -2430,6 +2431,13 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd)
                   autostart ? _("enable") : _("disable") );
      }

+    has_managed_state = virDomainHasManagedSaveImage(dom, 0);
+    if (has_managed_state<  0)
+        vshPrint(ctl, "%-15s %s\n", _("Has managed state:"), _("unknown"));

Here, we print "managed state", but the command that creates that state is called "managedsave", and the API we call is virDomainHasManagedSaveImage. Not to mention that it surpasses 15 columns, which makes the output not lined up with all the other rows. Also, the "Has" is a bit redundant. I'd prefer to see this output as:

Managed save:   yes|no|unknown

ACK with that nit fixed, and this can be pushed independently of the rest of the series.

--
Eric Blake   eblake@xxxxxxxxxx    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]