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@xxxxxxxxxxxxx> >>>>>> >>>>>> 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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list