[libvirt] [PATCH] xend_internal: Use domain/status for shutdown check

[Cole Robinson]

On 04/12/2012 09:42 AM, Stefan Bader wrote:
> As promised this version does keep the domid < 0 check in
> order to be clearly keeping the old behavior.
> 
> -Stefan
> 
>>From 18d398d98dc0dc2d9148ffb8673c651248d1bca5 Mon Sep 17 00:00:00 2001
> From: Stefan Bader <stefan.bader at canonical.com>
> Date: Thu, 12 Apr 2012 09:59:56 +0000
> Subject: [PATCH] xend_internal: Use domain/status for shutdown check
> 
> On newer xend (v3.x and after) there is no state and domid reported
> for inactive domains. When initially creating connections this is
> handled in various places by assigning domain->id = -1.
> But once an instance has been running, the id is set to the current
> domain id. And it does not change when the instance is shut down.
> So when querying the domain info, the hypervisor driver, which gets
> asked first will indicate it cannot find information, then the
> xend driver is asked and will set the status to NOSTATE because it
> checks for the -1 domain id.
> Checking domain/status for 0 seems to be more reliable for that.
> 
> One note: I am not sure whether the domain->id also should get set
> back to -1 whenever any sub-driver thinks the instance is no longer
> running.
> 
> BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=746007
> BugLink: http://bugs.launchpad.net/bugs/929626
> 
> [v2: Keep old id check just in case]
> Signed-off-by: Stefan Bader <stefan.bader at canonical.com>
> ---
>  src/xen/xend_internal.c |   11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
> index 6526af4..f1aa9b6 100644
> --- a/src/xen/xend_internal.c
> +++ b/src/xen/xend_internal.c
> @@ -989,9 +989,14 @@ sexpr_to_xend_domain_state(virDomainPtr domain, const struct sexpr *root)
>              state = VIR_DOMAIN_BLOCKED;
>          else if (strchr(flags, 'r'))
>              state = VIR_DOMAIN_RUNNING;
> -    } else if (domain->id < 0) {
> -        /* Inactive domains don't have a state reported, so
> -           mark them SHUTOFF, rather than NOSTATE */
> +    } else if (domain->id < 0 || sexpr_int(root, "domain/status") == 0) {
> +        /* As far as I can see the domain->id is a bad sign for checking
> +         * inactive domains as this is inaccurate after the domain has
> +         * been running once. However domain/status from xend seems to
> +         * be always present and 0 for inactive domains.
> +         * (keeping the check for id < 0 to be extra safe about backward
> +         * compatibility)
> +         */
>          state = VIR_DOMAIN_SHUTOFF;
>      }
>  

ACK

Thanks,
Cole