From patchwork Wed Oct 24 09:50:53 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Wolf X-Patchwork-Id: 193754 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 35F972C0097 for ; Wed, 24 Oct 2012 22:27:47 +1100 (EST) Received: from localhost ([::1]:36318 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TQxdw-0003hz-NK for incoming@patchwork.ozlabs.org; Wed, 24 Oct 2012 05:52:56 -0400 Received: from eggs.gnu.org ([208.118.235.92]:35882) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TQxcd-00024s-Jg for qemu-devel@nongnu.org; Wed, 24 Oct 2012 05:51:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TQxca-0006qS-WE for qemu-devel@nongnu.org; Wed, 24 Oct 2012 05:51:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:22428) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TQxca-0006qF-Lj for qemu-devel@nongnu.org; Wed, 24 Oct 2012 05:51:32 -0400 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q9O9pUbn026495 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 24 Oct 2012 05:51:31 -0400 Received: from dhcp-5-188.str.redhat.com (dhcp-200-232.str.redhat.com [10.33.200.232]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q9O9ov1q004084; Wed, 24 Oct 2012 05:51:29 -0400 From: Kevin Wolf To: anthony@codemonkey.ws Date: Wed, 24 Oct 2012 11:50:53 +0200 Message-Id: <1351072256-6112-30-git-send-email-kwolf@redhat.com> In-Reply-To: <1351072256-6112-1-git-send-email-kwolf@redhat.com> References: <1351072256-6112-1-git-send-email-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.25 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.132.183.28 Cc: kwolf@redhat.com, qemu-devel@nongnu.org Subject: [Qemu-devel] [PATCH 29/32] mirror: add support for on-source-error/on-target-error X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org From: Paolo Bonzini Error management is important for mirroring; otherwise, an error on the target (even something as "innocent" as ENOSPC) requires to start again with a full copy. Similar to on_read_error/on_write_error, two separate knobs are provided for on_source_error (reads) and on_target_error (writes). The default is 'report' for both. The 'ignore' policy will leave the sector dirty, so that it will be retried later. Thus, it will not cause corruption. Signed-off-by: Paolo Bonzini Signed-off-by: Kevin Wolf --- block/mirror.c | 94 ++++++++++++++++++++++++++++++++++++++++++------------ block_int.h | 4 ++ blockdev.c | 14 +++++++- hmp.c | 3 +- qapi-schema.json | 11 ++++++- qmp-commands.hx | 8 ++++- 6 files changed, 108 insertions(+), 26 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index 6320f6a..d6618a4 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -32,13 +32,28 @@ typedef struct MirrorBlockJob { RateLimit limit; BlockDriverState *target; MirrorSyncMode mode; + BlockdevOnError on_source_error, on_target_error; bool synced; bool should_complete; int64_t sector_num; uint8_t *buf; } MirrorBlockJob; -static int coroutine_fn mirror_iteration(MirrorBlockJob *s) +static BlockErrorAction mirror_error_action(MirrorBlockJob *s, bool read, + int error) +{ + s->synced = false; + if (read) { + return block_job_error_action(&s->common, s->common.bs, + s->on_source_error, true, error); + } else { + return block_job_error_action(&s->common, s->target, + s->on_target_error, false, error); + } +} + +static int coroutine_fn mirror_iteration(MirrorBlockJob *s, + BlockErrorAction *p_action) { BlockDriverState *source = s->common.bs; BlockDriverState *target = s->target; @@ -60,9 +75,21 @@ static int coroutine_fn mirror_iteration(MirrorBlockJob *s) trace_mirror_one_iteration(s, s->sector_num, nb_sectors); ret = bdrv_co_readv(source, s->sector_num, nb_sectors, &qiov); if (ret < 0) { - return ret; + *p_action = mirror_error_action(s, true, -ret); + goto fail; + } + ret = bdrv_co_writev(target, s->sector_num, nb_sectors, &qiov); + if (ret < 0) { + *p_action = mirror_error_action(s, false, -ret); + s->synced = false; + goto fail; } - return bdrv_co_writev(target, s->sector_num, nb_sectors, &qiov); + return 0; + +fail: + /* Try again later. */ + bdrv_set_dirty(source, s->sector_num, nb_sectors); + return ret; } static void coroutine_fn mirror_run(void *opaque) @@ -117,8 +144,9 @@ static void coroutine_fn mirror_run(void *opaque) cnt = bdrv_get_dirty_count(bs); if (cnt != 0) { - ret = mirror_iteration(s); - if (ret < 0) { + BlockErrorAction action = BDRV_ACTION_REPORT; + ret = mirror_iteration(s, &action); + if (ret < 0 && action == BDRV_ACTION_REPORT) { goto immediate_exit; } cnt = bdrv_get_dirty_count(bs); @@ -129,23 +157,25 @@ static void coroutine_fn mirror_run(void *opaque) trace_mirror_before_flush(s); ret = bdrv_flush(s->target); if (ret < 0) { - goto immediate_exit; - } - - /* We're out of the streaming phase. From now on, if the job - * is cancelled we will actually complete all pending I/O and - * report completion. This way, block-job-cancel will leave - * the target in a consistent state. - */ - s->common.offset = end * BDRV_SECTOR_SIZE; - if (!s->synced) { - block_job_ready(&s->common); - s->synced = true; + if (mirror_error_action(s, false, -ret) == BDRV_ACTION_REPORT) { + goto immediate_exit; + } + } else { + /* We're out of the streaming phase. From now on, if the job + * is cancelled we will actually complete all pending I/O and + * report completion. This way, block-job-cancel will leave + * the target in a consistent state. + */ + s->common.offset = end * BDRV_SECTOR_SIZE; + if (!s->synced) { + block_job_ready(&s->common); + s->synced = true; + } + + should_complete = s->should_complete || + block_job_is_cancelled(&s->common); + cnt = bdrv_get_dirty_count(bs); } - - should_complete = s->should_complete || - block_job_is_cancelled(&s->common); - cnt = bdrv_get_dirty_count(bs); } if (cnt == 0 && should_complete) { @@ -197,6 +227,7 @@ static void coroutine_fn mirror_run(void *opaque) immediate_exit: g_free(s->buf); bdrv_set_dirty_tracking(bs, false); + bdrv_iostatus_disable(s->target); if (s->should_complete && ret == 0) { if (bdrv_get_flags(s->target) != bdrv_get_flags(s->common.bs)) { bdrv_reopen(s->target, bdrv_get_flags(s->common.bs), NULL); @@ -219,6 +250,13 @@ static void mirror_set_speed(BlockJob *job, int64_t speed, Error **errp) ratelimit_set_speed(&s->limit, speed / BDRV_SECTOR_SIZE, SLICE_TIME); } +static void mirror_iostatus_reset(BlockJob *job) +{ + MirrorBlockJob *s = container_of(job, MirrorBlockJob, common); + + bdrv_iostatus_reset(s->target); +} + static void mirror_complete(BlockJob *job, Error **errp) { MirrorBlockJob *s = container_of(job, MirrorBlockJob, common); @@ -245,25 +283,39 @@ static BlockJobType mirror_job_type = { .instance_size = sizeof(MirrorBlockJob), .job_type = "mirror", .set_speed = mirror_set_speed, + .iostatus_reset= mirror_iostatus_reset, .complete = mirror_complete, }; void mirror_start(BlockDriverState *bs, BlockDriverState *target, int64_t speed, MirrorSyncMode mode, + BlockdevOnError on_source_error, + BlockdevOnError on_target_error, BlockDriverCompletionFunc *cb, void *opaque, Error **errp) { MirrorBlockJob *s; + if ((on_source_error == BLOCKDEV_ON_ERROR_STOP || + on_source_error == BLOCKDEV_ON_ERROR_ENOSPC) && + !bdrv_iostatus_is_enabled(bs)) { + error_set(errp, QERR_INVALID_PARAMETER, "on-source-error"); + return; + } + s = block_job_create(&mirror_job_type, bs, speed, cb, opaque, errp); if (!s) { return; } + s->on_source_error = on_source_error; + s->on_target_error = on_target_error; s->target = target; s->mode = mode; bdrv_set_dirty_tracking(bs, true); bdrv_set_enable_write_cache(s->target, true); + bdrv_set_on_error(s->target, on_target_error, on_target_error); + bdrv_iostatus_enable(s->target); s->common.co = qemu_coroutine_create(mirror_run); trace_mirror_start(bs, s, s->common.co, opaque); qemu_coroutine_enter(s->common.co, s); diff --git a/block_int.h b/block_int.h index aaa46a8..00204eb 100644 --- a/block_int.h +++ b/block_int.h @@ -337,6 +337,8 @@ void commit_start(BlockDriverState *bs, BlockDriverState *base, * @target: Block device to write to. * @speed: The maximum speed, in bytes per second, or 0 for unlimited. * @mode: Whether to collapse all images in the chain to the target. + * @on_source_error: The action to take upon error reading from the source. + * @on_target_error: The action to take upon error writing to the target. * @cb: Completion function for the job. * @opaque: Opaque pointer value passed to @cb. * @errp: Error object. @@ -348,6 +350,8 @@ void commit_start(BlockDriverState *bs, BlockDriverState *base, */ void mirror_start(BlockDriverState *bs, BlockDriverState *target, int64_t speed, MirrorSyncMode mode, + BlockdevOnError on_source_error, + BlockdevOnError on_target_error, BlockDriverCompletionFunc *cb, void *opaque, Error **errp); diff --git a/blockdev.c b/blockdev.c index 431c678..a068a4b 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1185,7 +1185,10 @@ void qmp_drive_mirror(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, Error **errp) + bool has_speed, int64_t speed, + bool has_on_source_error, BlockdevOnError on_source_error, + bool has_on_target_error, BlockdevOnError on_target_error, + Error **errp) { BlockDriverInfo bdi; BlockDriverState *bs; @@ -1200,6 +1203,12 @@ void qmp_drive_mirror(const char *device, const char *target, if (!has_speed) { speed = 0; } + if (!has_on_source_error) { + on_source_error = BLOCKDEV_ON_ERROR_REPORT; + } + if (!has_on_target_error) { + on_target_error = BLOCKDEV_ON_ERROR_REPORT; + } if (!has_mode) { mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS; } @@ -1292,7 +1301,8 @@ void qmp_drive_mirror(const char *device, const char *target, } } - mirror_start(bs, target_bs, speed, sync, block_job_cb, bs, &local_err); + mirror_start(bs, target_bs, speed, sync, on_source_error, on_target_error, + block_job_cb, bs, &local_err); if (local_err != NULL) { bdrv_delete(target_bs); error_propagate(errp, local_err); diff --git a/hmp.c b/hmp.c index e530253..4a458ac 100644 --- a/hmp.c +++ b/hmp.c @@ -795,7 +795,8 @@ void hmp_drive_mirror(Monitor *mon, const QDict *qdict) qmp_drive_mirror(device, filename, !!format, format, full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP, - true, mode, false, 0, &errp); + true, mode, false, 0, + false, 0, false, 0, &errp); hmp_handle_error(mon, &errp); } diff --git a/qapi-schema.json b/qapi-schema.json index a066cd5..6aa443e 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -1629,6 +1629,14 @@ # (all the disk, only the sectors allocated in the topmost image, or # only new I/O). # +# @on-source-error: #optional the action to take on an error on the source, +# default 'report'. 'stop' and 'enospc' can only be used +# if the block device supports io-status (see BlockInfo). +# +# @on-target-error: #optional the action to take on an error on the target, +# default 'report' (no limitations, since this applies to +# a different block device than @device). +# # Returns: nothing on success # If @device is not a valid block device, DeviceNotFound # @@ -1637,7 +1645,8 @@ { 'command': 'drive-mirror', 'data': { 'device': 'str', 'target': 'str', '*format': 'str', 'sync': 'MirrorSyncMode', '*mode': 'NewImageMode', - '*speed': 'int' } } + '*speed': 'int', '*on-source-error': 'BlockdevOnError', + '*on-target-error': 'BlockdevOnError' } } ## # @migrate_cancel diff --git a/qmp-commands.hx b/qmp-commands.hx index 614baea..c31312f 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -937,7 +937,8 @@ EQMP { .name = "drive-mirror", - .args_type = "sync:s,device:B,target:s,speed:i?,mode:s?,format:s?", + .args_type = "sync:s,device:B,target:s,speed:i?,mode:s?,format:s?," + "on-source-error:s?,on-target-error:s?", .mhandler.cmd_new = qmp_marshal_input_drive_mirror, }, @@ -965,6 +966,11 @@ Arguments: possibilities include "full" for all the disk, "top" for only the sectors allocated in the topmost image, or "none" to only replicate new I/O (MirrorSyncMode). +- "on-source-error": the action to take on an error on the source + (BlockdevOnError, default 'report') +- "on-target-error": the action to take on an error on the target + (BlockdevOnError, default 'report') + Example: