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 <[email protected]> > 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
