On Thu, May 04, 2017 at 12:54:40PM +0200, Daniel Kucera wrote: Seems like a logical extension along the same lines as the backup block job's dirty bitmap sync mode.
> parameter bitmap chooses existing dirtymap instead of newly created
> in mirror_start_job
>
> Signed-off-by: Daniel Kucera <daniel.kuc...@gmail.com>
> ---
> block/mirror.c | 47
> ++++++++++++++++++++++++++++++-----------------
> blockdev.c | 16 +++++++++++++++-
> include/block/block_int.h | 4 +++-
> qapi/block-core.json | 12 ++++++++++--
> 4 files changed, 58 insertions(+), 21 deletions(-)
This patch modifies the user's bitmap in-place and does not
differentiate successful completion from failure. I suggest following
the same bitmap lifecycle as the backup block job so that
cancellation/failure offers useful behavior. (If the job fails due to
intermittent errors on the target it should be possible to start the job
again - the bitmap must not be lost!).
> diff --git a/block/mirror.c b/block/mirror.c
> index 9f5eb69..1bfbe2e 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -49,7 +49,7 @@ typedef struct MirrorBlockJob {
> BlockDriverState *to_replace;
> /* Used to block operations on the drive-mirror-replace target */
> Error *replace_blocker;
> - bool is_none_mode;
> + MirrorSyncMode sync_mode;
> BlockMirrorBackingMode backing_mode;
> BlockdevOnError on_source_error, on_target_error;
> bool synced;
> @@ -523,7 +523,9 @@ static void mirror_exit(BlockJob *job, void *opaque)
> bdrv_child_try_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL,
> &error_abort);
> if (s->backing_mode == MIRROR_SOURCE_BACKING_CHAIN) {
> - BlockDriverState *backing = s->is_none_mode ? src : s->base;
> + BlockDriverState *backing =
> + (s->sync_mode == MIRROR_SYNC_MODE_INCREMENTAL) ||
> + (s->sync_mode == MIRROR_SYNC_MODE_NONE) ? src : s->base;
Please update the comment in include/block/block_int.h:typedef enum
BlockMirrorBackingMode.
> @@ -1213,9 +1217,21 @@ static void mirror_start_job(const char *job_id,
> BlockDriverState *bs,
> s->should_complete = true;
> }
>
> - s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, NULL, errp);
> - if (!s->dirty_bitmap) {
> - goto fail;
> + if (s->sync_mode == MIRROR_SYNC_MODE_INCREMENTAL) {
> + if (bitmap == NULL) {
> + error_setg(errp, "Mode incremental requires parameter 'bitmap'");
> + goto fail;
> + }
> + s->dirty_bitmap = bdrv_find_dirty_bitmap(bs, bitmap);
> + if (!s->dirty_bitmap) {
> + error_setg(errp, "Bitmap '%s' not found", bitmap);
> + goto fail;
> + }
> + } else {
> + s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, NULL,
> errp);
> + if (!s->dirty_bitmap) {
> + goto fail;
> + }
> }
Do we need to check that granularity is compatible with the pre-existing
dirty bitmap's granularity when sync_mode is
MIRROR_SYNC_MODE_INCREMENTAL?
signature.asc
Description: PGP signature
