Re: [Qemu-devel] [PATCH 08/12] dirty-bitmap: Change bdrv_get_dirty() to take bytes

[Eric Blake]

On 04/12/2017 07:25 PM, John Snow wrote:

>> +++ b/migration/block.c
>> @@ -537,8 +537,7 @@ static int mig_save_device_dirty(QEMUFile *f, 
>> BlkMigDevState *bmds,
>>          } else {
>>              blk_mig_unlock();
>>          }
>> -        if (bdrv_get_dirty(bs, bmds->dirty_bitmap, sector)) {
>> -
>> +        if (bdrv_get_dirty(bs, bmds->dirty_bitmap, sector * 
>> BDRV_SECTOR_SIZE)) {
> 
> This one is a little weirder now though, isn't it? We're asking for the
> dirty status of a single byte, technically. In practice, the scaling
> factor will always cover the entire sector, but it reads a lot jankier now.
> 
>>              if (total_sectors - sector < BDRV_SECTORS_PER_DIRTY_CHUNK) {
>>                  nr_sectors = total_sectors - sector;
>>              } else {
>>
> 
> Oh well, it was always janky...

True. Think of it as "is the granularity (which may be a sector, a
cluster, or even some other size) that contains 'offset' dirty?".  I
really think migration/block.c will be easier to read once converted to
byte operations everywhere, but have not yet tackled it (it was hard
enough tackling mirror, backup, commit, and stream in parallel for the
previous series).

> 
> Reviewed-by: John Snow <address@hidden>
> 

Thanks for the reviews, by the way, even if the prerequisite patches
still haven't been fully reviewed.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature