Re: [Qemu-devel] [Qemu-block] [PATCH 16/17] block: Make bdrv_is_allocated() byte-based

[John Snow]

On 04/19/2017 01:54 PM, Eric Blake wrote:
> 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.
> 

Err, right... temporary brain schism. DIV_ROUND_UP not ALIGN_UP, my mistake.

>