On 04/02/2015 08:44 AM, Stefan Hajnoczi wrote:
On Fri, Mar 20, 2015 at 03:16:53PM -0400, John Snow wrote:+ } else if (job->sync_mode == MIRROR_SYNC_MODE_DIRTY_BITMAP) { + /* Dirty Bitmap sync has a slightly different iteration method */ + HBitmapIter hbi; + int64_t sector; + int64_t cluster; + int64_t last_cluster = -1; + bool polyrhythmic; + + bdrv_dirty_iter_init(bs, job->sync_bitmap, &hbi); + /* Does the granularity happen to match our backup cluster size? */ + polyrhythmic = (bdrv_dirty_bitmap_granularity(job->sync_bitmap) != + BACKUP_CLUSTER_SIZE); + + /* Find the next dirty /sector/ and copy that /cluster/ */ + while ((sector = hbitmap_iter_next(&hbi)) != -1) { + cluster = sector / BACKUP_SECTORS_PER_CLUSTER; + + /* Fake progress updates for any clusters we skipped, + * excluding this current cluster. */ + if (cluster != last_cluster + 1) { + job->common.offset += ((cluster - last_cluster - 1) * + BACKUP_CLUSTER_SIZE); + } + + if (yield_and_check(job)) { + goto leave; + } + + do { + ret = backup_do_cow(bs, cluster * BACKUP_SECTORS_PER_CLUSTER, + BACKUP_SECTORS_PER_CLUSTER, &error_is_read); + if ((ret < 0) && + backup_error_action(job, error_is_read, -ret) == + BLOCK_ERROR_ACTION_REPORT) { + goto leave; + } + } while (ret < 0); + + /* Advance (or rewind) our iterator if we need to. */ + if (polyrhythmic) { + bdrv_set_dirty_iter(&hbi, + (cluster + 1) * BACKUP_SECTORS_PER_CLUSTER); + } + + last_cluster = cluster; + }What happens when the dirty bitmap granularity is larger than BACKUP_SECTORS_PER_CLUSTER? |-----bitmap granularity-----| |---backup cluster---| ~~~~~~~ Will these sectors ever be copied? I think this case causes an infinite loop: cluster = hbitmap_iter_next(&hbi) / BACKUP_SECTORS_PER_CLUSTER The iterator is reset: bdrv_set_dirty_iter(&hbi, (cluster + 1) * BACKUP_SECTORS_PER_CLUSTER); So we get the same cluster ever time and never advance?
I had mistakenly thought that if I advanced to the middle of a granularity grouping that the iterator might return the next (virtual) index to me, instead of the beginning of the current group.
Anyway, I've fixed this up a bit and added in some granularity variance tests for the iotests to test what happens if the granularity is larger, equal, or smaller.
v5 is ready to send out, but I need to test it first, so I will probably send that out Wednesday night.
Thanks, --js
