[Qemu-devel] Re: [PATCH 4/8] kvm: Rework dirty bitmap synchronization

[Jan Kiszka <jan.kiszka@siemens.com>]

Avi Kivity wrote:
> Jan Kiszka wrote:
>> Extend kvm_physical_sync_dirty_bitmap() so that is can sync across
>> multiple slots. Useful for updating the whole dirty log during
>> migration. Moreover, properly pass down errors the whole call chain.
>>
>>  
>> -void cpu_physical_sync_dirty_bitmap(target_phys_addr_t start_addr,
>> target_phys_addr_t end_addr);
>> +int cpu_physical_sync_dirty_bitmap(target_phys_addr_t start_addr,
>> +                                   target_phys_addr_t end_addr);
>>  
>>   
> 
> This is defined in terms of physical addresses, but in patch 5, you
> 
>> @@ -3248,13 +3248,18 @@ static int ram_save_live(QEMUFile *f, int
>> stage, void *opaque)
>>  {
>>      ram_addr_t addr;
>>  
>> +    if (cpu_physical_sync_dirty_bitmap(0, last_ram_offset) != 0) {
>> +        qemu_file_set_error(f);
>> +        return 0;
>> +    }
>> +
>>     
> 
> Which is in terms of ram addresses; so this will fail with large memory,
> where the last phys address is larger than the last ram address.

True, will fix.

> 
> Maybe this can be as easy as passing -1 for the end address.  But in
> this case I suggest adding a helper to sync all of memory instead.

IMHO, defining a separate helper is overkill as it would also mean
providing a specialized version of kvm_physical_sync_dirty_bitmap. I
think I will go for TARGET_PHYS_ADDR_MAX.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux