From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48424) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z8NMv-0003Nl-Sz for qemu-devel@nongnu.org; Fri, 26 Jun 2015 02:44:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z8NMq-0000Mf-P2 for qemu-devel@nongnu.org; Fri, 26 Jun 2015 02:44:09 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50178) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z8NMq-0000Lv-IU for qemu-devel@nongnu.org; Fri, 26 Jun 2015 02:44:04 -0400 Date: Fri, 26 Jun 2015 14:44:01 +0800 From: Fam Zheng Message-ID: <20150626064401.GD19264@ad.nay.redhat.com> References: <1435234332-581-1-git-send-email-stefanha@redhat.com> <1435234332-581-8-git-send-email-stefanha@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1435234332-581-8-git-send-email-stefanha@redhat.com> Subject: Re: [Qemu-devel] [PATCH 07/10] block/backup: support block job transactions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Kevin Wolf , Jeff Cody , qemu-devel@nongnu.org, mreitz@redhat.com, vsementsov@parallels.com, John Snow On Thu, 06/25 13:12, Stefan Hajnoczi wrote: > Join the transaction when the backup block job is in incremental backup > mode. > > This ensures that the sync bitmap is not thrown away if another block > job in the transaction is cancelled or fails. This is critical so > incremental backup with multiple disks can be retried in case of > cancellation/failure. > > Signed-off-by: Stefan Hajnoczi Reviewed-by: Fam Zheng > --- > block/backup.c | 7 +++++- > blockdev.c | 73 ++++++++++++++++++++++++++++++++++++++++++---------------- > 2 files changed, 59 insertions(+), 21 deletions(-) > > diff --git a/block/backup.c b/block/backup.c > index ddf8424..fa86b0e 100644 > --- a/block/backup.c > +++ b/block/backup.c > @@ -429,6 +429,8 @@ static void coroutine_fn backup_run(void *opaque) > qemu_co_rwlock_wrlock(&job->flush_rwlock); > qemu_co_rwlock_unlock(&job->flush_rwlock); > > + block_job_txn_prepare_to_complete(job->common.txn, &job->common, ret); > + > if (job->sync_bitmap) { > BdrvDirtyBitmap *bm; > if (ret < 0 || block_job_is_cancelled(&job->common)) { > @@ -457,7 +459,7 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target, > BlockdevOnError on_source_error, > BlockdevOnError on_target_error, > BlockCompletionFunc *cb, void *opaque, > - Error **errp) > + BlockJobTxn *txn, Error **errp) > { > int64_t len; > > @@ -537,6 +539,9 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target, > job->sync_mode = sync_mode; > job->sync_bitmap = sync_mode == MIRROR_SYNC_MODE_DIRTY_BITMAP ? > sync_bitmap : NULL; > + if (job->sync_bitmap) { > + block_job_txn_add_job(txn, &job->common); > + } > job->common.len = len; > job->common.co = qemu_coroutine_create(backup_run); > qemu_coroutine_enter(job->common.co, job); > diff --git a/blockdev.c b/blockdev.c > index 453f3ec..4f27c35 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -1586,6 +1586,18 @@ typedef struct DriveBackupState { > BlockJob *job; > } DriveBackupState; > > +static void do_drive_backup(const char *device, const char *target, > + bool has_format, const char *format, > + enum MirrorSyncMode sync, > + bool has_mode, enum NewImageMode mode, > + bool has_speed, int64_t speed, > + bool has_bitmap, const char *bitmap, > + bool has_on_source_error, > + BlockdevOnError on_source_error, > + bool has_on_target_error, > + BlockdevOnError on_target_error, > + BlockJobTxn *txn, Error **errp); > + > static void drive_backup_prepare(BlkActionState *common, Error **errp) > { > DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common); > @@ -1609,15 +1621,16 @@ static void drive_backup_prepare(BlkActionState *common, Error **errp) > state->aio_context = bdrv_get_aio_context(bs); > aio_context_acquire(state->aio_context); > > - qmp_drive_backup(backup->device, backup->target, > - backup->has_format, backup->format, > - backup->sync, > - backup->has_mode, backup->mode, > - backup->has_speed, backup->speed, > - backup->has_bitmap, backup->bitmap, > - backup->has_on_source_error, backup->on_source_error, > - backup->has_on_target_error, backup->on_target_error, > - &local_err); > + do_drive_backup(backup->device, backup->target, > + backup->has_format, backup->format, > + backup->sync, > + backup->has_mode, backup->mode, > + backup->has_speed, backup->speed, > + backup->has_bitmap, backup->bitmap, > + backup->has_on_source_error, backup->on_source_error, > + backup->has_on_target_error, backup->on_target_error, > + common->block_job_txn, > + &local_err); > if (local_err) { > error_propagate(errp, local_err); > return; > @@ -2585,15 +2598,17 @@ out: > aio_context_release(aio_context); > } > > -void qmp_drive_backup(const char *device, const char *target, > - bool has_format, const char *format, > - enum MirrorSyncMode sync, > - bool has_mode, enum NewImageMode mode, > - bool has_speed, int64_t speed, > - bool has_bitmap, const char *bitmap, > - bool has_on_source_error, BlockdevOnError on_source_error, > - bool has_on_target_error, BlockdevOnError on_target_error, > - Error **errp) > +static void do_drive_backup(const char *device, const char *target, > + bool has_format, const char *format, > + enum MirrorSyncMode sync, > + bool has_mode, enum NewImageMode mode, > + bool has_speed, int64_t speed, > + bool has_bitmap, const char *bitmap, > + bool has_on_source_error, > + BlockdevOnError on_source_error, > + bool has_on_target_error, > + BlockdevOnError on_target_error, > + BlockJobTxn *txn, Error **errp) > { > BlockBackend *blk; > BlockDriverState *bs; > @@ -2710,7 +2725,7 @@ void qmp_drive_backup(const char *device, const char *target, > > backup_start(bs, target_bs, speed, sync, bmap, > on_source_error, on_target_error, > - block_job_cb, bs, &local_err); > + block_job_cb, bs, txn, &local_err); > if (local_err != NULL) { > bdrv_unref(target_bs); > error_propagate(errp, local_err); > @@ -2721,6 +2736,24 @@ out: > aio_context_release(aio_context); > } > > +void qmp_drive_backup(const char *device, const char *target, > + bool has_format, const char *format, > + enum MirrorSyncMode sync, > + bool has_mode, enum NewImageMode mode, > + bool has_speed, int64_t speed, > + bool has_bitmap, const char *bitmap, > + bool has_on_source_error, BlockdevOnError on_source_error, > + bool has_on_target_error, BlockdevOnError on_target_error, > + Error **errp) > +{ > + return do_drive_backup(device, target, has_format, format, sync, > + has_mode, mode, has_speed, speed, > + has_bitmap, bitmap, > + has_on_source_error, on_source_error, > + has_on_target_error, on_target_error, > + NULL, errp); > +} > + > BlockDeviceInfoList *qmp_query_named_block_nodes(Error **errp) > { > return bdrv_named_nodes_list(errp); > @@ -2771,7 +2804,7 @@ void qmp_blockdev_backup(const char *device, const char *target, > bdrv_ref(target_bs); > bdrv_set_aio_context(target_bs, aio_context); > backup_start(bs, target_bs, speed, sync, NULL, on_source_error, > - on_target_error, block_job_cb, bs, &local_err); > + on_target_error, block_job_cb, bs, NULL, &local_err); > if (local_err != NULL) { > bdrv_unref(target_bs); > error_propagate(errp, local_err); > -- > 2.4.3 > >