Re: [libvirt] [PATCH 5/8] virsh: Prefer virDomainGetState over virDomainGetInfo
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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 619 bytes)