10.09.2019 13:23, Vladimir Sementsov-Ogievskiy wrote: > Split copying code part from backup to "block-copy", including separate > state structure and function renaming. This is needed to share it with > backup-top filter driver in further commits. > > Notes: > > 1. As BlockCopyState keeps own BlockBackend objects, remaining > job->common.blk users only use it to get bs by blk_bs() call, so clear > job->commen.blk permissions set in block_job_create and add > job->source_bs to be used instead of blk_bs(job->common.blk), to keep > it more clear which bs we use when introduce backup-top filter in > further commit. > > 2. Rename s/initializing_bitmap/skip_unallocated/ to sound a bit better > as interface to BlockCopyState > > 3. Split is not very clean: there left some duplicated fields, backup > code uses some BlockCopyState fields directly, let's postpone it for > further improvements and keep this comment simpler for review. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> > ---
[..] > + > +static BlockCopyState *block_copy_state_new( > + BlockDriverState *source, BlockDriverState *target, > + int64_t cluster_size, BdrvRequestFlags write_flags, > + ProgressBytesCallbackFunc progress_bytes_callback, > + ProgressResetCallbackFunc progress_reset_callback, > + void *progress_opaque, Error **errp) > +{ > + BlockCopyState *s; > + int ret; > + uint64_t no_resize = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE | > + BLK_PERM_WRITE_UNCHANGED | BLK_PERM_GRAPH_MOD; > + BdrvDirtyBitmap *copy_bitmap; > + > + copy_bitmap = bdrv_create_dirty_bitmap(source, cluster_size, NULL, errp); > + if (!copy_bitmap) { > + return NULL; > + } > + bdrv_disable_dirty_bitmap(copy_bitmap); > + > + s = g_new(BlockCopyState, 1); > + *s = (BlockCopyState) { > + .source = blk_new(bdrv_get_aio_context(source), > + BLK_PERM_CONSISTENT_READ, no_resize), > + .target = blk_new(bdrv_get_aio_context(target), > + BLK_PERM_WRITE, no_resize), > + .copy_bitmap = copy_bitmap, > + .cluster_size = cluster_size, > + .len = bdrv_dirty_bitmap_size(copy_bitmap), > + .write_flags = write_flags, > + .use_copy_range = !(write_flags & BDRV_REQ_WRITE_COMPRESSED), > + .progress_bytes_callback = progress_bytes_callback, > + .progress_reset_callback = progress_reset_callback, > + .progress_opaque = progress_opaque, > + }; > + > + s->copy_range_size = QEMU_ALIGN_UP(MIN(blk_get_max_transfer(s->source), > + blk_get_max_transfer(s->target)), > + s->cluster_size); preexistent, but it obviously should be QEMU_ALIGN_DOWN. I can resend with a separate fix, it may be fixed while queuing (if resend is not needed for other reasons) or I'll send a follow-up fix later, whichever you prefer. > + > + /* > + * We just allow aio context change on our block backends. block_copy() > user > + * (now it's only backup) is responsible for source and target being in > same > + * aio context. > + */ > + blk_set_disable_request_queuing(s->source, true); > + blk_set_allow_aio_context_change(s->source, true); > + blk_set_disable_request_queuing(s->target, true); > + blk_set_allow_aio_context_change(s->target, true); > + [..] > @@ -760,21 +860,19 @@ BlockJob *backup_job_create(const char *job_id, > BlockDriverState *bs, > * For more information see commit f8d59dfb40bb and test > * tests/qemu-iotests/222 [..] > job->cluster_size = cluster_size; > - job->copy_bitmap = copy_bitmap; > - copy_bitmap = NULL; > - job->use_copy_range = !compress; /* compression isn't supported for it */ > - job->copy_range_size = > MIN_NON_ZERO(blk_get_max_transfer(job->common.blk), > - blk_get_max_transfer(job->target)); > - job->copy_range_size = MAX(job->cluster_size, > - QEMU_ALIGN_UP(job->copy_range_size, > - job->cluster_size)); > - > - /* Required permissions are already taken with target's blk_new() */ > + > + /* Required permissions are already taken by block-copy-state target */ > block_job_add_bdrv(&job->common, "target", target, 0, BLK_PERM_ALL, > &error_abort); > job->len = len; -- Best regards, Vladimir