[libvirt] [PATCH 5/8] virsh: Prefer virDomainGetState over virDomainGetInfo

[Eric Blake]

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 at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110504/4712fb37/attachment-0001.sig>