Re: [Qemu-devel] [PATCH 45/51] ram: Use page number instead of an address for the bitmap operations

[Juan Quintela]

"Dr. David Alan Gilbert" <address@hidden> wrote:
> * Juan Quintela (address@hidden) wrote:
>> We use an unsigned long for the page number.  Notice that our bitmaps
>> already got that for the index, so we have that limit.
>> 
>> Signed-off-by: Juan Quintela <address@hidden>
>> ---
>>  migration/ram.c | 76 
>> ++++++++++++++++++++++++++-------------------------------
>>  1 file changed, 34 insertions(+), 42 deletions(-)
>> 
>> diff --git a/migration/ram.c b/migration/ram.c
>> index 6cd77b5..b1a031e 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -611,13 +611,12 @@ static int save_xbzrle_page(RAMState *rs, uint8_t 
>> **current_data,
>>   * @rs: current RAM state
>>   * @rb: RAMBlock where to search for dirty pages
>>   * @start: starting address (typically so we can continue from previous 
>> page)
>> - * @ram_addr_abs: pointer into which to store the address of the dirty page
>> - *                within the global ram_addr space
>> + * @page: pointer into where to store the dirty page
>
> I'd prefer if you could call it 'page_abs' - it often gets tricky to know
> whether we're talking about a page offset within a RAMBlock or an
> offset within
> the whole bitmap.

I don't really care.  Changed.

> (I wish we had different index types)

This is C man!!
>> -                trace_get_queued_page(block->idstr,
>> -                                      (uint64_t)offset,
>> -                                      (uint64_t)*ram_addr_abs);
>> +                trace_get_queued_page(block->idstr, (uint64_t)offset,
>> +                                     *page);
>
> I think you need to fix the trace_ definitions for get_queued_page
> and get_queued_page_not_dirty they're currently taking uint64_t's for
> ram_addr and they now need to be long's (with the format changes).

Done.

Thanks, Juan.