From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43156) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WO6gB-0002Yg-J1 for qemu-devel@nongnu.org; Thu, 13 Mar 2014 10:32:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WO6g5-0005Zo-SJ for qemu-devel@nongnu.org; Thu, 13 Mar 2014 10:32:15 -0400 Received: from lputeaux-656-01-25-125.w80-12.abo.wanadoo.fr ([80.12.84.125]:56128 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WO6g5-0005Zj-Io for qemu-devel@nongnu.org; Thu, 13 Mar 2014 10:32:09 -0400 Date: Thu, 13 Mar 2014 15:32:08 +0100 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140313143208.GG4534@irqsave.net> References: <1394605864-32237-1-git-send-email-famz@redhat.com> <1394605864-32237-9-git-send-email-famz@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <1394605864-32237-9-git-send-email-famz@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 8/9] qmp: Add support of "dirty-bitmap" sync mode for drive-backup List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: kwolf@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com The Wednesday 12 Mar 2014 =E0 14:31:03 (+0800), Fam Zheng wrote : > For "dirty-bitmap" sync mode, the block job will iterate through the > given dirty bitmap to decide if a sector needs backup (backup all the > dirty clusters and skip clean ones), just as allocation conditions of > "top" sync mode. >=20 > There are two bitmap use modes for sync=3Ddirty-bitmap: >=20 > - reset: backup job makes a copy of bitmap and resets the original > one. > - consume: back job makes the original anonymous (invisible to user) s/back job/backup job/ ?=20 > and releases it after use. >=20 > Signed-off-by: Fam Zheng > --- > block/backup.c | 51 +++++++++++++++++++++++++++++++++++++++= +++++++- > block/mirror.c | 4 ++++ > blockdev.c | 9 ++++++++- > hmp.c | 4 +++- > include/block/block_int.h | 3 +++ > qapi-schema.json | 30 ++++++++++++++++++++++++---- > qmp-commands.hx | 7 ++++--- > 7 files changed, 98 insertions(+), 10 deletions(-) >=20 > diff --git a/block/backup.c b/block/backup.c > index 15a2e55..6b05429 100644 > --- a/block/backup.c > +++ b/block/backup.c > @@ -37,6 +37,8 @@ typedef struct CowRequest { > typedef struct BackupBlockJob { > BlockJob common; > BlockDriverState *target; > + BdrvDirtyBitmap *sync_bitmap; > + int sync_bitmap_gran; Maybe a comment would help. Reading this variable name for the first time I wonder what the _gran suf= fix mean. > MirrorSyncMode sync_mode; > RateLimit limit; > BlockdevOnError on_source_error; > @@ -263,7 +265,7 @@ static void coroutine_fn backup_run(void *opaque) > job->common.busy =3D true; > } > } else { > - /* Both FULL and TOP SYNC_MODE's require copying.. */ > + /* FULL, TOP and DIRTY_BITMAP SYNC_MODE's require copying.. */ > for (; start < end; start++) { > bool error_is_read; > =20 > @@ -317,7 +319,21 @@ static void coroutine_fn backup_run(void *opaque) > if (alloced =3D=3D 0) { > continue; > } > + } else if (job->sync_mode =3D=3D MIRROR_SYNC_MODE_DIRTY_BI= TMAP) { > + int i, dirty =3D 0; > + for (i =3D 0; i < BACKUP_SECTORS_PER_CLUSTER; > + i +=3D job->sync_bitmap_gran) { Ok it's _gran for _granularity. > + if (bdrv_get_dirty(bs, job->sync_bitmap, > + start * BACKUP_SECTORS_PER_CLUSTER + i)) { > + dirty =3D 1; > + break; > + } > + } > + if (!dirty) { > + continue; > + } > } > + > /* FULL sync mode we copy the whole drive. */ > ret =3D backup_do_cow(bs, start * BACKUP_SECTORS_PER_CLUST= ER, > BACKUP_SECTORS_PER_CLUSTER, &error_is_read); > @@ -341,6 +357,9 @@ static void coroutine_fn backup_run(void *opaque) > qemu_co_rwlock_wrlock(&job->flush_rwlock); > qemu_co_rwlock_unlock(&job->flush_rwlock); > =20 > + if (job->sync_bitmap) { > + bdrv_release_dirty_bitmap(bs, job->sync_bitmap); > + } > hbitmap_free(job->bitmap); > =20 > bdrv_iostatus_disable(target); > @@ -351,12 +370,15 @@ static void coroutine_fn backup_run(void *opaque) > =20 > void backup_start(BlockDriverState *bs, BlockDriverState *target, > int64_t speed, MirrorSyncMode sync_mode, > + BdrvDirtyBitmap *sync_bitmap, > + BitmapUseMode bitmap_mode, > BlockdevOnError on_source_error, > BlockdevOnError on_target_error, > BlockDriverCompletionFunc *cb, void *opaque, > Error **errp) > { > int64_t len; > + BdrvDirtyBitmap *original; > =20 > assert(bs); > assert(target); > @@ -369,6 +391,28 @@ void backup_start(BlockDriverState *bs, BlockDrive= rState *target, > return; > } > =20 > + if (sync_mode =3D=3D MIRROR_SYNC_MODE_DIRTY_BITMAP && !sync_bitmap= ) { > + error_setg(errp, "must provide a valid bitmap name for \"dirty= -bitmap\"" > + "sync mode"); > + return; > + } > + > + if (sync_bitmap) { > + switch (bitmap_mode) { > + case BITMAP_USE_MODE_RESET: > + original =3D sync_bitmap; > + sync_bitmap =3D bdrv_copy_dirty_bitmap(bs, sync_bitmap, NU= LL); > + bdrv_reset_dirty_bitmap(bs, original); > + break; > + case BITMAP_USE_MODE_CONSUME: > + bdrv_dirty_bitmap_make_anon(bs, sync_bitmap); > + break; > + default: > + assert(0); > + } > + bdrv_disable_dirty_bitmap(bs, sync_bitmap); > + } > + > len =3D bdrv_getlength(bs); > if (len < 0) { > error_setg_errno(errp, -len, "unable to get length for '%s'", > @@ -386,6 +430,11 @@ void backup_start(BlockDriverState *bs, BlockDrive= rState *target, > job->on_target_error =3D on_target_error; > job->target =3D target; > job->sync_mode =3D sync_mode; > + job->sync_bitmap =3D sync_bitmap; > + if (sync_bitmap) { > + job->sync_bitmap_gran =3D > + bdrv_dirty_bitmap_granularity(bs, job->sync_bitmap); > + } > job->common.len =3D len; > job->common.co =3D qemu_coroutine_create(backup_run); > qemu_coroutine_enter(job->common.co, job); > diff --git a/block/mirror.c b/block/mirror.c > index be8b2a1..6e4f3f3 100644 > --- a/block/mirror.c > +++ b/block/mirror.c > @@ -615,6 +615,10 @@ void mirror_start(BlockDriverState *bs, BlockDrive= rState *target, > bool is_none_mode; > BlockDriverState *base; > =20 > + if (mode =3D=3D MIRROR_SYNC_MODE_DIRTY_BITMAP) { > + error_setg(errp, "Sync mode 'dirty-bitmap' not supported"); > + return; > + } > is_none_mode =3D mode =3D=3D MIRROR_SYNC_MODE_NONE; > base =3D mode =3D=3D MIRROR_SYNC_MODE_TOP ? bs->backing_hd : NULL; > mirror_start_job(bs, target, speed, granularity, buf_size, > diff --git a/blockdev.c b/blockdev.c > index aa3ee55..4120dee 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -1384,6 +1384,8 @@ static void drive_backup_prepare(BlkTransactionSt= ate *common, Error **errp) > backup->sync, > backup->has_mode, backup->mode, > backup->has_speed, backup->speed, > + backup->has_bitmap, backup->bitmap, > + backup->has_bitmap_use_mode, backup->bitmap_use_m= ode, > backup->has_on_source_error, backup->on_source_er= ror, > backup->has_on_target_error, backup->on_target_er= ror, > &local_err); > @@ -2032,6 +2034,8 @@ void qmp_drive_backup(const char *device, const c= har *target, > enum MirrorSyncMode sync, > bool has_mode, enum NewImageMode mode, > bool has_speed, int64_t speed, > + bool has_bitmap, const char *bitmap, > + bool has_bitmap_use_mode, enum BitmapUseMode bit= map_mode, > bool has_on_source_error, BlockdevOnError on_sou= rce_error, > bool has_on_target_error, BlockdevOnError on_tar= get_error, > Error **errp) > @@ -2129,7 +2133,10 @@ void qmp_drive_backup(const char *device, const = char *target, > return; > } > =20 > - backup_start(bs, target_bs, speed, sync, on_source_error, on_targe= t_error, > + backup_start(bs, target_bs, speed, sync, > + has_bitmap ? bdrv_find_dirty_bitmap(bs, bitmap) : NUL= L, > + has_bitmap_use_mode ? bitmap_mode : BITMAP_USE_MODE_R= ESET, > + on_source_error, on_target_error, > block_job_cb, bs, &local_err); > if (local_err !=3D NULL) { > bdrv_unref(target_bs); > diff --git a/hmp.c b/hmp.c > index 2f279c4..2c84377 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -950,7 +950,9 @@ void hmp_drive_backup(Monitor *mon, const QDict *qd= ict) > =20 > qmp_drive_backup(device, filename, !!format, format, > full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_T= OP, > - true, mode, false, 0, false, 0, false, 0, &errp); > + true, mode, false, 0, false, NULL, > + false, 0, > + false, 0, false, 0, &errp); > hmp_handle_error(mon, &errp); > } > =20 > diff --git a/include/block/block_int.h b/include/block/block_int.h > index 0bcf1c9..d02c090 100644 > --- a/include/block/block_int.h > +++ b/include/block/block_int.h > @@ -484,6 +484,7 @@ void mirror_start(BlockDriverState *bs, BlockDriver= State *target, > * @target: Block device to write to. > * @speed: The maximum speed, in bytes per second, or 0 for unlimited. > * @sync_mode: What parts of the disk image should be copied to the de= stination. > + * @sync_bitmap: The dirty bitmap if sync_mode is MIRROR_SYNC_MODE_DIR= TY_BITMAP. > * @on_source_error: The action to take upon error reading from the so= urce. > * @on_target_error: The action to take upon error writing to the targ= et. > * @cb: Completion function for the job. > @@ -494,6 +495,8 @@ void mirror_start(BlockDriverState *bs, BlockDriver= State *target, > */ > void backup_start(BlockDriverState *bs, BlockDriverState *target, > int64_t speed, MirrorSyncMode sync_mode, > + BdrvDirtyBitmap *sync_bitmap, > + BitmapUseMode bitmap_mode, > BlockdevOnError on_source_error, > BlockdevOnError on_target_error, > BlockDriverCompletionFunc *cb, void *opaque, > diff --git a/qapi-schema.json b/qapi-schema.json > index f8cd6b8..e4e66cb 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -1503,7 +1503,7 @@ > # Since: 1.3 > ## > { 'enum': 'MirrorSyncMode', > - 'data': ['top', 'full', 'none'] } > + 'data': ['top', 'full', 'none', 'dirty-bitmap'] } > =20 > ## > # @BlockJobType: > @@ -1879,6 +1879,22 @@ > 'data': { 'device': 'str', 'name': 'str' } } > =20 > ## > +# @BitmapUseMode > +# > +# An enumeration that tells QEMU what operation to take when consuming= a bitmap > +# a new image file. > +# > +# @consume: QEMU should just consume the bitmap and release it after u= sing > +# > +# @reset: QEMU should keep an internal copy of the bitmap and reset th= e > +# dirty record from original one > +# > +# Since: 2.1 > +## > +{ 'enum': 'BitmapUseMode', > + 'data': [ 'consume', 'reset' ] } > + > +## > # @DriveBackup > # > # @device: the name of the device which should be copied. > @@ -1891,14 +1907,19 @@ > # probe if @mode is 'existing', else the format of the source > # > # @sync: what parts of the disk image should be copied to the destinat= ion > -# (all the disk, only the sectors allocated in the topmost imag= e, or > -# only new I/O). > +# (all the disk, only the sectors allocated in the topmost imag= e, from a > +# dirty bitmap, or only new I/O). > # > # @mode: #optional whether and how QEMU should create a new image, def= ault is > # 'absolute-paths'. > # > # @speed: #optional the maximum speed, in bytes per second > # > +# @bitmap: #optional the name of dirty bitmap if sync is "dirty-bitmap= " (Since 2.1) > +# > +# @bitmap-use-mode: #optional which operation to take when consuming @= bitmap, > +# default is reset. (Since 2.1) > +# > # @on-source-error: #optional the action to take on an error on the so= urce, > # default 'report'. 'stop' and 'enospc' can only be= used > # if the block device supports io-status (see BlockI= nfo). > @@ -1916,7 +1937,8 @@ > { 'type': 'DriveBackup', > 'data': { 'device': 'str', 'target': 'str', '*format': 'str', > 'sync': 'MirrorSyncMode', '*mode': 'NewImageMode', > - '*speed': 'int', > + '*speed': 'int', '*bitmap': 'str', > + '*bitmap-use-mode': 'BitmapUseMode', > '*on-source-error': 'BlockdevOnError', > '*on-target-error': 'BlockdevOnError' } } > =20 > diff --git a/qmp-commands.hx b/qmp-commands.hx > index 205343b..4d8271a 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -1031,7 +1031,7 @@ EQMP > { > .name =3D "drive-backup", > .args_type =3D "sync:s,device:B,target:s,speed:i?,mode:s?,for= mat:s?," > - "on-source-error:s?,on-target-error:s?", > + "bitmap:s?,on-source-error:s?,on-target-error:s?= ", > .mhandler.cmd_new =3D qmp_marshal_input_drive_backup, > }, > =20 > @@ -1058,8 +1058,9 @@ Arguments: > (json-string, optional) > - "sync": what parts of the disk image should be copied to the destina= tion; > possibilities include "full" for all the disk, "top" for only the se= ctors > - allocated in the topmost image, or "none" to only replicate new I/O > - (MirrorSyncMode). > + allocated in the topmost image, "dirty-bitmap" for only sync the dir= ty > + sectors in the bitmap, or "none" to only replicate new I/O (MirrorSy= ncMode). > +- "bitmap": bitmap name to use for sync=3D=3Ddirty-bitmap > - "mode": whether and how QEMU should create a new image > (NewImageMode, optional, default 'absolute-paths') > - "speed": the maximum speed, in bytes per second (json-int, optional) > --=20 > 1.9.0 >=20 >=20