Re: [Qemu-devel] [PATCH v2 31/45] mirror: add support for on-source-error/on-target-error

[Kevin Wolf]

Am 26.09.2012 17:56, schrieb 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 <address@hidden>
> ---
>         v1->v2: error handling for bdrv_flush, introduce mirror_error_action
> 
>  block/mirror.c   | 95 
> +++++++++++++++++++++++++++++++++++++++++++-------------
>  block_int.h      |  4 +++
>  blockdev.c       | 14 +++++++--
>  hmp.c            |  3 +-
>  qapi-schema.json | 11 ++++++-
>  qmp-commands.hx  |  8 ++++-
>  6 file modificati, 109 inserzioni(+), 26 rimozioni(-)
> 
> diff --git a/block/mirror.c b/block/mirror.c
> index 939834d..caec272 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 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);

Here we produce an event that reports an error on s->bs, i.e. on the
source, even though the error was on the target. This makes some sense
today that the target doesn't have a name, but once it has, we would
better use the target name here.

Can we change this later on? If not, what's the way forward?

Kevin