Re: xl shutdown --wait "racy"

[George Dunlap <george.dunlap@eu.citrix.com>]

On 04/16/2014 04:02 PM, Ian Campbell wrote:
> On Wed, 2014-04-16 at 16:55 +0200, Sander Eikelenboom wrote:
>> Wednesday, April 16, 2014, 4:33:30 PM, you wrote:
>>
>>> On Wed, 2014-04-16 at 16:26 +0200, Sander Eikelenboom wrote:
>>>> Wednesday, April 16, 2014, 4:13:59 PM, you wrote:
>>>>
>>>>> On Wed, 2014-04-16 at 16:08 +0200, Sander Eikelenboom wrote:
>>>>>> Hi Ian (C|J) Konrad,
>>>>>>
>>>>>> I'm currently trying to workaround the pci-(detach|assignable-remove) issues i
>>>>>> reported earlier.
>>>>>>
>>>>>> The workaround i thought of was:
>>>>>> - shutting down the guest
>>>>>> - starting it without 1 of the original devices passed through
>>>>>> - use xl pci-assignable-remove and bind the device to the dom0 driver.
>>>>>>
>>>>>> But during this i noticed that a "xl shutdown --wait" does wait .. but returns:
>>>>>> - Before the domain is removed from for instance "xl list", it still listed there in
>>>>>> "--ps--" state.
>>>>>> - before pciback has done it's restore config space magic.
>>>>>>
>>>>>> So it seems the wait loop is exiting somewhat prematurely, is this expected ?
>>>>> It is waiting for the domain to be shutdown (state 's') not for the
>>>>> domain to be destroyed. So it's doing what it said it would (I
>>>>> appreciate you might not find this distinction helpful under the
>>>>> circumstances...)
>>>> It's at least not entirely what i expected ;-)
>>>>
>>>> Is it because there can be different "follow-up actions" due to the
>>>> "on_poweroff=" config option ?
>>> Not really, those are somewhat unrelated.
>>> shutdown and destroy are two distinct events. Once a domain has shutdown
>>> (called the shutdown hypercall etc) it goes into state "shutdown" and an
>>> event is generated from the hypervisor to the toolstack. The toolstack's
>>> response to this is to actually destroy the domain, that is to tear down
>>> the resources it is using etc.
>>> on_* only matter for the destroy phase since they tell the toolstack
>>> what it should do (restart, preserve, really destroy etc).
>> Hmm ok, it should be called "--wait_until_halfway" then ;-)
> ;-)
>
>> On the more serious side .. would patches be accepted that:
>>
>> a) differentiate when it returns from waiting based on the on_*
>>
>>          preserve: this could probably stay as is .. after the shutdown event
>>          destroy:
>>          restart:
>>          rename-restart:
>>          coredump-destroy:
>>          coredump-restart:
>>
>>          for the other ones .. i don't know if there actually are events in libxl
>>          that could be 'easily' coupled ?
> Might be tricky, since on_* is processed by the daemonised xl which is
> monitoring the domain, not the xl shutdown process.

Store it in xenstore somewhere?

>
>> b) make it possible for the xl commandline to overrule the on_* from the configfile
> I guess you mean the xl shutdown command. This will also be tricky, for
> the same reasons as a.
>
>> c) also introduce a -w/--wait for xl destroy
> Yes.
>
> I'll add:
>
> d) Make "xl shutdown --wait" actually wait for the domain to be
> destroyed.

This seems like the most reasonable thing to do.

  -George