Re: [Xen-devel] [PATCH RFCv3 3/8] xen: introduce XEN_DOMCTL_set_recipient

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.11.14 at 18:10, <vkuznets@xxxxxxxxxx> wrote:
> "Jan Beulich" <JBeulich@xxxxxxxx> writes:
> 
>>>>> On 25.11.14 at 16:41, <vkuznets@xxxxxxxxxx> wrote:
>>> "Jan Beulich" <JBeulich@xxxxxxxx> writes:
>>>>>>> On 07.10.14 at 15:10, <vkuznets@xxxxxxxxxx> wrote:
>>>>> @@ -1764,11 +1765,28 @@ void free_domheap_pages(struct page_info *pg, 
> unsigned int order)
>>>>>              scrub = 1;
>>>>>          }
>>>>>  
>>>>> -        if ( unlikely(scrub) )
>>>>> -            for ( i = 0; i < (1 << order); i++ )
>>>>> -                scrub_one_page(&pg[i]);
>>>>> +        if ( !d || !d->recipient || d->recipient->is_dying )
>>>>> +        {
>>>>> +            if ( unlikely(scrub) )
>>>>> +                for ( i = 0; i < (1 << order); i++ )
>>>>> +                    scrub_one_page(&pg[i]);
>>>>>  
>>>>> -        free_heap_pages(pg, order);
>>>>> +            free_heap_pages(pg, order);
>>>>> +        }
>>>>> +        else
>>>>> +        {
>>>>> +            mfn = page_to_mfn(pg);
>>>>> +            gmfn = mfn_to_gmfn(d, mfn);
>>>>> +
>>>>> +            page_set_owner(pg, NULL);
>>>>> +            assign_pages(d->recipient, pg, order, 0);
>>>>
>>>> This can fail.
>>> 
>>> .. if the recipient is dying or we're over-allocating. Shouldn't happen
>>> (if toolstack does its job right) but I'll add a check.
>>
>> You must not rely on the tool stack doing things right (see XSA-77).
>>
>>>> What's worse though is that you don't guard against
>>>> XEN_DOMCTL_set_recipient zapping d->recipient. If that really is
>>>> necessary to be supported, some synchronization will be needed.
>>>>
>>>> And finally - what's the intended protocol for the tool stack to
>>>> know that all pages got transferred (and hence the new domain
>>>> can be launched)?
>>>>
>>> 
>>> (hope this answers both questions above)
>>> 
>>> Now the protocol is:
>>> 1) Toolstack queries for all page frames (with xc_get_pfn_type_batch())
>>> for the old domain.
>>> 2) Toolstack issues XEN_DOMCTL_set_recipient with the recipient domain
>>> 3) Toolstack kills the source domain
>>> 4) Toolstack waits for awhile (loop keeps checking while we see new pages
>>> arriving + some timeout).
>>> 5) Toolstack issues XEN_DOMCTL_set_recipient with DOMID_INVALID
>>> resetting recipient.
>>> 6) Toolstack checks if all pages were transferred
>>> 7) If some pages are missing (e.g. source domain became zombie holding
>>> something) request new empty pages instead.
>>> 8) Toolstack starts new domain
>>> 
>>> So we don't actually need XEN_DOMCTL_set_recipient to switch from one
>>> recipient to the other, we just need a way to reset it.
>>
>> No, this doesn't address either question. For the first one, you again
>> assume the tool stack behaves correctly, which is wrong nowadays.
>> And for the second you give a description, but that's not really a
>> dependable protocol. Nor do I really follow why resetting the recipient
>> is necessary. Can't the tools e.g. wait for the final death of the domain
>> if there's no other notification?
> 
> Yes, it's possible and should happen in all normal cases. However my
> idea was that it's possible to start new domain even if the old one
> fails to die holding several (presumably special) pages and we're fine
> with replacing those with empty pages. In case we go for 'always wait
> for the original domain to die' solution resetting
> XEN_DOMCTL_set_recipient is not necessary (it is necessary in case we
> don't as we can recieve a page after we already requested a new one
> instead).

I think this "always wait" is imo the only reasonable solution: How
would replacing some pages with empty ones fit the kexec/kdump
purposes? However, it may not be necessary to wait for the
domain to completely disappear, it may be sufficient to wait for
its d->tot_pages to become zero.

>>> From a general (hypervisor) point of view we don't actually care if the
>>> domain is dying or not. We just want to recieve all freed pages from
>>> this domain (so they go to some other domain instead of trash).
>>
>> We do care I think, primarily because we want a correct GMFN <->
>> MFN mapping. Seeing multiple pages for the same GMFN is for
>> example going to cause the whole process in its current form to fail.
> 
> Can adding a check that nothing is mapped to the GMFN before mapping new
> MFN there be a solution?

Not checking for this is presumably the better approach - the new
domain would get what the old one had last at a given GMFN. What
it may not get are pages that intermediately got moved around.
But again, all that is relevant only if the old domain can still alter its
memory layout while the transfer is already in progress, which (as
said before) I think should be avoided.

>> And considering the need for such a correct mapping - is it always
>> the case that the mapping gets updated after a page got removed
>> from a guest? (I can see that the mapping doesn't change anymore
>> for a dying guest, but you explicitly say that you want/need this to
>> work before the guest is actually marked dying.)
> 
> Actual reassignment here happens for a dying guest only as
> XEN_DOMCTL_set_recipient does nothing. If you think it's safer to depend
> on the fact that dying flag is always set we can couple
> XEN_DOMCTL_set_recipient with XEN_DOMCTL_destroydomain in one call. It
> is possible if we go for 'always wait for the original domain to die'
> solution (so no reset is required).

I indeed think that's the safer and more correct route.

Jan

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