On 05/04/2011 08:45 AM, Jiri Denemark wrote:
--- tools/virsh.c | 195 +++++++++++++++++++++++++++++++++++++++++++++++---------- 1 files changed, 162 insertions(+), 33 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 5d8b025..6dae9fa 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -223,6 +223,8 @@ typedef struct __vshControl { int log_fd; /* log file descriptor */ char *historydir; /* readline history directory name */ char *historyfile; /* readline history file name */ + bool noGetState; /* virDomainGetState is not supported in + * current connection */
Nice - remembering the failure once, so you don't have to repeat it. However, I don't like double negatives; could we instead have: bool useGetInfo /* must use virDomainGetInfo, since virDomainGetState failed */
@@ -571,6 +575,7 @@ vshReconnect(vshControl *ctl) { else vshError(ctl, "%s", _("Reconnected to the hypervisor")); disconnected = 0; + ctl->noGetState = false;
then a default state of ctl->useGetInfo = false still makes sense,
@@ -3063,12 +3068,6 @@ cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd) return false; }
- if (virDomainGetInfo(dom, &info) != 0) { - virDomainFree(dom); - vshError(ctl, "%s", _("Unable to verify current MemorySize")); - return false; - } -
This seems like one case where we still want to keep the virDomainGetInfo and actually query the guest.
+static int +vshDomainState(vshControl *ctl, virDomainPtr dom, int *reason) +{ + virDomainInfo info; + + if (reason) + *reason = -1; + + if (!ctl->noGetState) {
Here, if you follow my rename, you avoid the double negative ("!" and "no"), and instead have: if (!ctl->useGetInfo) But overall, the patch looks nice. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org