Re: [Qemu-devel] [PATCH 29/51] ram: Move last_req_rb to RAMState

[Peter Xu]

On Thu, Mar 30, 2017 at 06:08:45PM +0200, Juan Quintela wrote:
> Peter Xu <address@hidden> wrote:
> > On Thu, Mar 23, 2017 at 09:45:22PM +0100, Juan Quintela wrote:
> >> It was on MigrationState when it is only used inside ram.c for
> >> postcopy.  Problem is that we need to access it without being able to
> >> pass it RAMState directly.
> >> 
> >> Signed-off-by: Juan Quintela <address@hidden>
> >> ---
> >>  include/migration/migration.h | 2 --
> >>  migration/migration.c         | 1 -
> >>  migration/ram.c               | 7 +++++--
> >>  3 files changed, 5 insertions(+), 5 deletions(-)
> >> 
> >> diff --git a/include/migration/migration.h b/include/migration/migration.h
> >> index 84cef4b..e032fb0 100644
> >> --- a/include/migration/migration.h
> >> +++ b/include/migration/migration.h
> >> @@ -189,8 +189,6 @@ struct MigrationState
> >>      /* Queue of outstanding page requests from the destination */
> >>      QemuMutex src_page_req_mutex;
> >>      QSIMPLEQ_HEAD(src_page_requests, MigrationSrcPageRequest) 
> >> src_page_requests;
> >> -    /* The RAMBlock used in the last src_page_request */
> >> -    RAMBlock *last_req_rb;
> >>      /* The semaphore is used to notify COLO thread that failover is 
> >> finished */
> >>      QemuSemaphore colo_exit_sem;
> >>  
> >> diff --git a/migration/migration.c b/migration/migration.c
> >> index e532430..b220941 100644
> >> --- a/migration/migration.c
> >> +++ b/migration/migration.c
> >> @@ -1118,7 +1118,6 @@ MigrationState *migrate_init(const MigrationParams 
> >> *params)
> >>      s->postcopy_after_devices = false;
> >>      s->postcopy_requests = 0;
> >>      s->migration_thread_running = false;
> >> -    s->last_req_rb = NULL;
> >>      error_free(s->error);
> >>      s->error = NULL;
> >>  
> >> diff --git a/migration/ram.c b/migration/ram.c
> >> index dd5a453..325a0f3 100644
> >> --- a/migration/ram.c
> >> +++ b/migration/ram.c
> >> @@ -203,6 +203,8 @@ struct RAMState {
> >>      QemuMutex bitmap_mutex;
> >>      /* Ram Bitmap protected by RCU */
> >>      RAMBitmap *ram_bitmap;
> >> +    /* The RAMBlock used in the last src_page_request */
> >                                                         ^ "s" missing
> >
> > Besides:
> 
> The important one is only the last one, we don't really care about the
> previous here, no?

I preferred "src_page_requests" since that's the variable name (so
then people can do symbol search on that). Anyway, that's trivial, so
please feel free to ignore it. :-)

-- peterx