Re: [Qemu-devel] [PATCH v3 09/12] mirror: switch mirror_iteration to AIO

[Kevin Wolf]

Am 21.01.2013 17:09, schrieb Paolo Bonzini:
> There is really no change in the behavior of the job here, since
> there is still a maximum of one in-flight I/O operation between
> the source and the target.  However, this patch already introduces
> the AIO callbacks (which are unmodified in the next patch)
> and some of the logic to count in-flight operations and only
> complete the job when there is none.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>         v2->v3: Only set cow_bitmap after successful write. [Kevin]
>         Fix resolution of SLICE_TIME in comment. [Kevin]
>         Remove extraneous change from "goto" to "break". [Kevin]
>         Rename "nb_sectors_chunk" to "sectors_per_chunk" [Kevin]
>         and consistently use "chunks" in all related variables.
> 
>  block/mirror.c | 166 
> +++++++++++++++++++++++++++++++++++++++++++--------------
>  trace-events   |   2 +
>  2 files changed, 127 insertions(+), 41 deletions(-)
> 
> diff --git a/block/mirror.c b/block/mirror.c
> index ca0c5a1..dd7436b 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -33,8 +33,19 @@ typedef struct MirrorBlockJob {
>      unsigned long *cow_bitmap;
>      HBitmapIter hbi;
>      uint8_t *buf;
> +
> +    int in_flight;
> +    int ret;
>  } MirrorBlockJob;
>  
> +typedef struct MirrorOp {
> +    MirrorBlockJob *s;
> +    QEMUIOVector qiov;
> +    struct iovec iov;
> +    int64_t sector_num;
> +    int nb_sectors;
> +} MirrorOp;
> +
>  static BlockErrorAction mirror_error_action(MirrorBlockJob *s, bool read,
>                                              int error)
>  {
> @@ -48,15 +59,69 @@ static BlockErrorAction 
> mirror_error_action(MirrorBlockJob *s, bool read,
>      }
>  }
>  
> -static int coroutine_fn mirror_iteration(MirrorBlockJob *s,
> -                                         BlockErrorAction *p_action)
> +static void mirror_iteration_done(MirrorOp *op)
> +{
> +    MirrorBlockJob *s = op->s;
> +    int64_t chunk_num;
> +    int nb_chunks, sectors_per_chunk;
> +
> +    s->in_flight--;
> +    sectors_per_chunk = s->granularity >> BDRV_SECTOR_BITS;
> +    chunk_num = op->sector_num / sectors_per_chunk;
> +    nb_chunks = op->nb_sectors / sectors_per_chunk;
> +    if (s->cow_bitmap) {

&& ret >= 0? (ret would have to be passed by the callers)

> +        bitmap_set(s->cow_bitmap, chunk_num, nb_chunks);
> +    }
> +
> +    trace_mirror_iteration_done(s, op->sector_num, op->nb_sectors);
> +    g_slice_free(MirrorOp, op);
> +    qemu_coroutine_enter(s->common.co, NULL);
> +}

Kevin