Re: [Xen-devel] [PATCH Remus v5 1/2] libxc/save: implement Remus checkpointed save

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 15/05/15 09:37, Yang Hongyang wrote:
>
>
> On 05/15/2015 04:29 PM, Andrew Cooper wrote:
>> On 15/05/2015 03:12, Yang Hongyang wrote:
>>>
>>>>> @@ -467,6 +477,14 @@ static int send_domain_memory_live(struct
>>>>> xc_sr_context *ctx)
>>>>>        DECLARE_HYPERCALL_BUFFER_SHADOW(unsigned long, dirty_bitmap,
>>>>>                                        &ctx->save.dirty_bitmap_hbuf);
>>>>>
>>>>> +    /*
>>>>> +     * With Remus, we will enter checkpointed save after live
>>>>> migration.
>>>>> +     * In checkpointed save loop, we skip the live part and pause
>>>>> straight
>>>>> +     * away to send dirty pages between checkpoints.
>>>>> +     */
>>>>> +    if ( !ctx->save.live )
>>>>> +        goto last_iter;
>>>>
>>>> Rather than use goto would it work to refactor everything from here to
>>>> the label into some sort of helper and just call that in the "actually
>>>> live" case?
>>>>
>>>> Or perhaps everything from the label to the end should be a helper
>>>> function which the caller can also use in thecheckpoint case
>>>> instead of
>>>> calling send_domain_memory_live (and which s_d_m_l also calls of
>>>> course).
>>>
>>> I'm going to refactor the send_domain_memory_live() as follows:
>>>
>>> split the send_domain_memory_live() into three helper function:
>>>    - send_memory_live()  do the actually live case
>>>    - suspend_and_send_dirty() suspend the guest and send dirty pages
>>>    - send_memory_verify()
>>>
>>> then:
>>>    - send_domain_memory_live() combination of those three helper
>>> functions
>>>    - send_domain_momory_checkpointed() calls
>>> suspend_and_send_dirty() and
>>>                                        send_memory_verify()
>>>    - send_domain_memory_nonlive() stay as it is
>>> Does it make sense?
>>
>> verify mode is only a debugging tool, and shouldn't be used in general.
>> (It is actually a lot less useful in retrospect than one would imagine.)
>
> Do you mean we do not need this in checkpointed stream?
> Currently, tt is conditioned under 'if ( ctx->save.debug )', both in
> live migration and checkpointed stream.

Ah - you absolutely don't want it in a checkpointed stream.  As soon as
that option has been set, the restoring side can no longer resume the
domain.

Basically, it tells the restore side to start doing memcmp() instead of
memcpy() on the incoming page data.  The initial hope was to detect
memory corruption, but it turns out that there is natural memory
corruption to be had anyway when PV drivers are in use.

(This is another area which is around for compatibility with legacy,
rather than actually of being much use in practice)

~Andrew

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