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

[Stefan Bader]

On 10.04.2012 15:33, Cole Robinson wrote:
> On 04/10/2012 09:32 AM, Stefan Bader wrote:
>> On 10.04.2012 15:22, Cole Robinson wrote:
>>> On 04/10/2012 04:46 AM, Stefan Bader wrote:
>>>> On 04/08/2012 03:08 PM, Cole Robinson wrote:
>>>>> On 04/02/2012 10:38 AM, Stefan Bader wrote:
>>>>>> 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
>>>>>>
>>>>>> Signed-off-by: Stefan Bader <stefan.bader at canonical.com>
>>>>>>
>>>>>> Index: libvirt-0.9.8/src/xen/xend_internal.c
>>>>>> ===================================================================
>>>>>> --- libvirt-0.9.8.orig/src/xen/xend_internal.c	2011-12-04 08:15:00.000000000 +0100
>>>>>> +++ libvirt-0.9.8/src/xen/xend_internal.c	2012-03-23 11:07:43.575529377 +0100
>>>>>> @@ -989,9 +989,11 @@
>>>>>>              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 (sexpr_int(root, "domain/status") == 0) {
>>>>>
>>>>> Maybe this should be
>>>>>
>>>>> (domain->id < 0 || sexpr_int(root, ...
>>>>>
>>>> It would not matter. Since the status is zero for all non-running domains it
>>>> covers those with domain->id < 0 as well.
>>>>
>>>
>>> Even for RHEL5 vintage xen? Since we historically try to maintain
>>> compatibility with that. It may well work, but unless it's tested I don't
>>> think there's much harm in keeping the id < 0 check to preserve old behavior.
>>>
>>> Thanks,
>>> Cole
>>
>> I checked against CentOS5.5 (close enough). But right, it should not harm to
>> have it. I re-submit the patch as soon as I have recovered my failed attempt to
>> recover a raid failure... :/
>>
> 
> If you checked against a centos5 host, I'm fine with that. So ACK to this patch.
> 
> - Cole
> 
Thinking about it I remember that my installation uses a Xen version from gitco
(3.4) while the original one was 3.0 iirc. So better have that safety check in,
just to be extra safe.

-Stefan

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