[libvirt] [PATCH] Use virDomainGetState in migration APIs

[Daniel P. Berrange]

On Thu, May 19, 2011 at 03:36:30PM +0200, Jiri Denemark wrote:
> We are only interested in domain state so no need to call
> virDomainGetInfo for that.
> ---
>  src/libvirt.c |   18 +++++++++---------
>  1 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/src/libvirt.c b/src/libvirt.c
> index ff16c48..3d1b314 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -3513,10 +3513,10 @@ virDomainMigrateVersion1 (virDomainPtr domain,
>      char *uri_out = NULL;
>      char *cookie = NULL;
>      int cookielen = 0, ret;
> -    virDomainInfo info;
> +    int state;
>  
> -    ret = virDomainGetInfo (domain, &info);
> -    if (ret == 0 && info.state == VIR_DOMAIN_PAUSED) {
> +    ret = virDomainGetState(domain, &state, NULL, 0);
> +    if (ret == 0 && state == VIR_DOMAIN_PAUSED) {
>          flags |= VIR_MIGRATE_PAUSED;
>      }
>  
> @@ -3601,7 +3601,7 @@ virDomainMigrateVersion2 (virDomainPtr domain,
>      char *cookie = NULL;
>      char *dom_xml = NULL;
>      int cookielen = 0, ret;
> -    virDomainInfo info;
> +    int state;
>      virErrorPtr orig_err = NULL;
>      int cancelled;
>  
> @@ -3632,8 +3632,8 @@ virDomainMigrateVersion2 (virDomainPtr domain,
>      if (!dom_xml)
>          return NULL;
>  
> -    ret = virDomainGetInfo (domain, &info);
> -    if (ret == 0 && info.state == VIR_DOMAIN_PAUSED) {
> +    ret = virDomainGetState(domain, &state, NULL, 0);
> +    if (ret == 0 && state == VIR_DOMAIN_PAUSED) {
>          flags |= VIR_MIGRATE_PAUSED;
>      }
>  
> @@ -3732,7 +3732,7 @@ virDomainMigrateVersion3(virDomainPtr domain,
>      int cookieinlen = 0;
>      int cookieoutlen = 0;
>      int ret;
> -    virDomainInfo info;
> +    int state;
>      virErrorPtr orig_err = NULL;
>      int cancelled;
>  
> @@ -3753,8 +3753,8 @@ virDomainMigrateVersion3(virDomainPtr domain,
>      if (!dom_xml)
>          goto done;
>  
> -    ret = virDomainGetInfo (domain, &info);
> -    if (ret == 0 && info.state == VIR_DOMAIN_PAUSED) {
> +    ret = virDomainGetState(domain, &state, NULL, 0);
> +    if (ret == 0 && state == VIR_DOMAIN_PAUSED) {
>          flags |= VIR_MIGRATE_PAUSED;
>      }

NACK to this, since it breaks migration compatibility. Much of the
'virDomainMigrate' code runs in the client app, and the source
libvirtd it is talking to can't be expected to have virDomainGetState
support. Only the v3 migration could be safely allowed to do this,
and I don't think it is worth it. The reason for using GetState
instead of GetInfo is to avoid calling the monitor, in case QEMU
is deadlocked/blocked, but this isn't useful because the very
next step in migration is going to use the monitor anyway.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|