Re: [PATCH v4 09/13] migration/ram: Consolidate variable reset after placement in ram_load_postcopy()

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Dr. David Alan Gilbert (dgilbert@redhat.com) wrote:
> * David Hildenbrand (david@redhat.com) wrote:
> > Let's consolidate resetting the variables.
> > 
> > Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > Cc: Juan Quintela <quintela@redhat.com>
> > Cc: Peter Xu <peterx@redhat.com>
> > Signed-off-by: David Hildenbrand <david@redhat.com>
> 
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Queued this one only; I had to do manual patch application due to some
renaming in 91ba442; but I think it's still OK.

> > ---
> >  migration/ram.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/migration/ram.c b/migration/ram.c
> > index 2a2165b478..7eca3165c8 100644
> > --- a/migration/ram.c
> > +++ b/migration/ram.c
> > @@ -3150,7 +3150,7 @@ static int ram_load_postcopy(QEMUFile *f)
> >      /* Temporary page that is later 'placed' */
> >      void *postcopy_host_page = mis->postcopy_tmp_page;
> >      void *host_page = NULL;
> > -    bool all_zero = false;
> > +    bool all_zero = true;
> >      int target_pages = 0;
> >  
> >      while (!ret && !(flags & RAM_SAVE_FLAG_EOS)) {
> > @@ -3176,7 +3176,6 @@ static int ram_load_postcopy(QEMUFile *f)
> >          addr &= TARGET_PAGE_MASK;
> >  
> >          trace_ram_load_postcopy_loop((uint64_t)addr, flags);
> > -        place_needed = false;
> >          if (flags & (RAM_SAVE_FLAG_ZERO | RAM_SAVE_FLAG_PAGE |
> >                       RAM_SAVE_FLAG_COMPRESS_PAGE)) {
> >              block = ram_block_from_stream(f, flags);
> > @@ -3204,9 +3203,7 @@ static int ram_load_postcopy(QEMUFile *f)
> >               */
> >              page_buffer = postcopy_host_page +
> >                            host_page_offset_from_ram_block_offset(block, addr);
> > -            /* If all TP are zero then we can optimise the place */
> >              if (target_pages == 1) {
> > -                all_zero = true;
> >                  host_page = host_page_from_ram_block_offset(block, addr);
> >              } else if (host_page != host_page_from_ram_block_offset(block,
> >                                                                      addr)) {
> > @@ -3223,7 +3220,6 @@ static int ram_load_postcopy(QEMUFile *f)
> >               */
> >              if (target_pages == (block->page_size / TARGET_PAGE_SIZE)) {
> >                  place_needed = true;
> > -                target_pages = 0;
> >              }
> >              place_source = postcopy_host_page;
> >          }
> > @@ -3300,6 +3296,10 @@ static int ram_load_postcopy(QEMUFile *f)
> >                  ret = postcopy_place_page(mis, host_page, place_source,
> >                                            block);
> >              }
> > +            place_needed = false;
> > +            target_pages = 0;
> > +            /* Assume we have a zero page until we detect something different */
> > +            all_zero = true;
> >          }
> >      }
> >  
> > -- 
> > 2.25.1
> > 
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK