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. >