On 04/18/2017 05:15 PM, John Snow wrote:

>> @@ -279,9 +280,9 @@ static int mig_save_device_bulk(QEMUFile *f, 
>> BlkMigDevState *bmds)
>>          /* Skip unallocated sectors; intentionally treats failure as
>>           * an allocated sector */
>>          while (cur_sector < total_sectors &&
>> -               !bdrv_is_allocated(blk_bs(bb), cur_sector,
>> -                                  MAX_IS_ALLOCATED_SEARCH, &nr_sectors)) {
>> -            cur_sector += nr_sectors;
>> +               !bdrv_is_allocated(blk_bs(bb), cur_sector * BDRV_SECTOR_SIZE,
>> +                                  MAX_IS_ALLOCATED_SEARCH, &count)) {
>> +            cur_sector += DIV_ROUND_UP(count, BDRV_SECTOR_SIZE);
> 
> Hm, what's the story here, why are we rounding this up? If, in theory,
> we have a partially allocated cluster won't we advance past that?

This is rounding to sectors, not clusters (that is, the code is supposed
to advance cur_sector identically pre- and post-patch).  As to whether
the overall algorithm makes sense, or could use some tweaking by
converting migration/block.c to do everything by bytes instead of by
sectors, I haven't yet given that any serious time.


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

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to