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

[Juan Quintela]

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?

>
> Reviewed-by: Peter Xu <address@hidden>
>
>> +    RAMBlock *last_req_rb;
>>  };
>>  typedef struct RAMState RAMState;
>
> -- peterx

Thanks,