Re: [Qemu-devel] [PATCH 2/2] migration: extract ram_load_precopy

[Wei Yang]

On Tue, Jul 23, 2019 at 05:47:03PM +0100, Dr. David Alan Gilbert wrote:
>* Wei Yang (address@hidden) wrote:
>> After cleanup, it would be clear to audience there are two cases
>> ram_load:
>> 
>>   * precopy
>>   * postcopy
>> 
>> And it is not necessary to check postcopy_running on each iteration for
>> precopy.
>> 
>> Signed-off-by: Wei Yang <address@hidden>
>> ---
>>  migration/ram.c | 73 +++++++++++++++++++++++++++++++------------------
>>  1 file changed, 46 insertions(+), 27 deletions(-)
>> 
>> diff --git a/migration/ram.c b/migration/ram.c
>> index 6bfdfae16e..5f6f07b255 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -4200,40 +4200,26 @@ static void colo_flush_ram_cache(void)
>>      trace_colo_flush_ram_cache_end();
>>  }
>>  
>> -static int ram_load(QEMUFile *f, void *opaque, int version_id)
>> +/**
>> + * ram_load_precopy: load a page in precopy case
>
>This comment is wrong - although I realise you copied it from the
>postcopy case; they don't just load a single page; they load 'pages'
>

Thanks for pointing out.

Actually, I got one confusion in these two load. Compare these two cases, I
found precopy would handle two more cases:

  * precopy:  RAM_SAVE_FLAG_ZERO | RAM_SAVE_FLAG_PAGE |
              RAM_SAVE_FLAG_COMPRESS_PAGE | RAM_SAVE_FLAG_XBZRLE
  * postcopy: RAM_SAVE_FLAG_ZERO | RAM_SAVE_FLAG_PAGE

Why postcopy doesn't need to handle the other two cases? Function
ram_save_target_page() does the same thing in precopy and postcopy. I don't
find the reason the behavior differs. Would you mind giving me a hint?

>Other than that, I think it's OK, so:
>
>
>Reviewed-by: Dr. David Alan Gilbert <address@hidden>
>

-- 
Wei Yang
Help you, Help me