Re: [Xen-devel] [PATCH] libxl: a domain can be dying but not shutdown

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 23/10/14 10:42, Ian Campbell wrote:
> On Wed, 2014-10-22 at 15:08 +0100, Andrew Cooper wrote:
>> On 22/10/14 14:52, David Scott wrote:
>>> The shutdown code is only present if the domain is shutdown.
>>> If we attempt to extract it from the flags from a dying but not
>>> shutdown domain then we get values like '255' which is not a
>>> valid LIBXL_SHUTDOWN_REASON_. We should use LIBXL_SHUTDOWN_UNKNOWN
>>> in this case.
>>>
>>> Signed-off-by: David Scott <dave.scott@xxxxxxxxxx>
>>>
>>> ---
>>>
>>> This can be tested by running 2 domUs, and having one map pages from
>>> the other. I used a vchan connection, so I had a vchan_server granting
>>> pages and a vchan_client mapping them. I made sure the client is
>>> never going to unmap the pages (I used 'sleep' in a Mirage kernel but
>>> 'xl pause' from outside would probably also work) and then I
>>> 'xl destroyed' the server. The server domain ends up stuck in the dying
>>> state because the client still has a page mapped. The server domain
>>> is not shutdown.
>>>
>>> According to 'xl list':
>>>
>>> djs@st20:~/djs55/list$ sudo xl list
>>> Name                                        ID   Mem VCPUs      State   
>>> Time(s)
>>> Domain-0                                     0  5278     6     r-----   
>>> 10971.2
>>> fedora                                      12  2048     1     -b----    
>>> 5470.0
>>> (null)                                      21     0     1     -bp--d      
>>> 25.1
>>> vchan_client                                22   256     1     -b----       
>>> 0.0
>>>
>>> and according to my test program which calls libxl_list_domain:
>>>
>>> domain 0 shutdown = 0 dying = 0 shutdown_reason = -1
>>> domain 12 shutdown = 0 dying = 0 shutdown_reason = -1
>>> domain 21 shutdown = 0 dying = 1 shutdown_reason = 255
>>> domain 22 shutdown = 0 dying = 0 shutdown_reason = -1
>>>
>>> I believe this also manifests transiently during a normal 'xl destroy'.
>>>
>>> Cheers,
>>> Dave
>>> ---
>> Hmm.  Xen unconditionally sets the shutdown code in info->flags when
>> querying for dominfo.
>>
>> d->shutdown_code defaults to -1 (which explains the 255, given the
>> mask), but only becomes valid once d->is_shutting_down gets set.
>>
>> This equates to XEN_DOMINF_shutdown lower in the flags field, which is
>> translated to xlinfo->shutdown just ahead of the context below.
>>
>> It is explicitly not valid for a dying domain, as a dying domain can be
>> dying for many reasons, few of which include a valid shutdown code.
> I'm afraid I can't tell whether this constitutes an implicit ack or a
> nack of this patch.

Sorry.  The patch is good, but also wants to adjust some comments to
match reality.

>
>> Therefore, other bits of libxl.h could do with correcting, given the
>> buggy changeset 4d70c9c5
> Do you know of any specific locations? I don't see any relevant uses of
> the word "shutdown" or "reason" in libxl.h.
>
> Did you instead mean this from libxl_types.idl:
>     # Valid iff (shutdown||dying).
>     #
>     # Otherwise set to a value guaranteed not to clash with any valid
>     # LIBXL_SHUTDOWN_REASON_* constant.
>     ("shutdown_reason", libxl_shutdown_reason),
> ?

That is the primary comment I was on about.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel