From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: Xen crashing when killing a domain with no VCPUs allocated Date: Thu, 24 Jul 2014 15:04:34 +0100 Message-ID: <53D11272.3010504@linaro.org> References: <53C920DD.6060300@linaro.org> <1405701560.14973.1.camel@kazak.uk.xensource.com> <53C982FF.7070608@linaro.org> <53CCEC64.7040304@eu.citrix.com> <53CCFD9D.2050502@linaro.org> <1405947445.17850.1.camel@Solace> <53CFF17F020000780002537E@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <53CFF17F020000780002537E@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , Dario Faggioli Cc: Juergen Gross , Ian Campbell , Stefano Stabellini , George Dunlap , Tim Deegan , george.dunlap@citrix.com, xen-devel List-Id: xen-devel@lists.xenproject.org Hi, On 07/23/2014 04:31 PM, Jan Beulich wrote: >>>> On 21.07.14 at 14:57, wrote: >> On lun, 2014-07-21 at 12:46 +0100, Julien Grall wrote: >>> On 07/21/2014 11:33 AM, George Dunlap wrote: >>>> On 07/18/2014 09:26 PM, Julien Grall wrote: >> >>>>> diff --git a/xen/common/schedule.c b/xen/common/schedule.c >>>>> index e9eb0bc..c44d047 100644 >>>>> --- a/xen/common/schedule.c >>>>> +++ b/xen/common/schedule.c >>>>> @@ -311,7 +311,7 @@ int sched_move_domain(struct domain *d, struct >>>>> cpupool *c) >>>>> } >>>>> /* Do we have vcpus already? If not, no need to update >>>>> node-affinity */ >>>>> - if ( d->vcpu ) >>>>> + if ( d->vcpu && d->vcpu[0] != NULL ) >>>>> domain_update_node_affinity(d); >>>> >> >>>> Overall it seems like those checks for the existence of cpus should be >>>> moved into domain_update_node_affinity(). The ASSERT() there I think is >>>> just a sanity check to make sure we're not getting a ridiculous result >>>> out of our calculation; but of course if there actually are no vcpus, >>>> it's not ridiculous at all. >>>> >>>> One solution might be to change the ASSERT to >>>> ASSERT(!cpumask_empty(dom_cpumask) || !d->vcpu || !d->vcpu[0]). Then we >>>> could probably even remove the d->vcpu conditional when calling it. >>> >>> This solution also works for me. Which change do you prefer? >>> >> FWIW, I think I like changing the ASSERT() in >> domain_update_node_affinity(), as George suggested (and perhaps with the >> reordering Andrew suggested) better. > > +1 Thanks. I will send a patch during the next couple days to fix this issue. Regards, -- Julien Grall