On Wed, Jul 24, 2019 at 01:10:24PM +0100, Dr. David Alan Gilbert wrote: >* Wei Yang (richardw.y...@linux.intel.com) wrote: >> On Tue, Jul 23, 2019 at 05:47:03PM +0100, Dr. David Alan Gilbert wrote: >> >* Wei Yang (richardw.y...@linux.intel.com) 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 <richardw.y...@linux.intel.com> >> >> --- >> >> 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? > >Because we don't support either compression or xbzrle with postcopy. >Compression could be fixed, but it needs to make sure it uses the >place-page function to atomically place the page. >
Thanks for the explanation. Sounds I missed some point. The place-page function to use is postcopy_place_page()? >xbzrle never gets used during the postcopy stage; it gets used >in the precopy stage in a migration that might switch to postcopy >though. Since xbzrle relies on optimising differences between >passes, it's > 1) Not needed in postcopy where there's only one final pass > 2) Since the destination is changing RAM, you can't transmit > deltas relative to the old data, since that data may have > changed. > >Dave > >> >Other than that, I think it's OK, so: >> > >> > >> >Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com> >> > >> >> -- >> Wei Yang >> Help you, Help me >-- >Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK -- Wei Yang Help you, Help me
