Re: [Qemu-devel] [PATCH v12 12/17] block: Ensure consistent bitmap function prototypes

[Max Reitz]

On 2015-02-09 at 20:35, John Snow wrote:
> We often don't need the BlockDriverState for functions
> that operate on bitmaps. Remove it.
>
> Signed-off-by: John Snow <address@hidden>
> ---
>   block.c               | 13 ++++++-------
>   block/backup.c        |  2 +-
>   block/mirror.c        | 23 ++++++++++-------------
>   blockdev.c            |  2 +-
>   include/block/block.h | 11 +++++------
>   migration/block.c     |  7 +++----
>   6 files changed, 26 insertions(+), 32 deletions(-)

[snip]

> diff --git a/block/mirror.c b/block/mirror.c
> index 271dbf3..04d1710 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -125,10 +125,9 @@ static void mirror_write_complete(void *opaque, int ret)
>       MirrorOp *op = opaque;
>       MirrorBlockJob *s = op->s;
>       if (ret < 0) {
> -        BlockDriverState *source = s->common.bs;
>           BlockErrorAction action;
>   
> -        bdrv_set_dirty_bitmap(source, s->dirty_bitmap, op->sector_num,
> +        bdrv_set_dirty_bitmap(s->dirty_bitmap, op->sector_num,
>                                 op->nb_sectors);
>           action = mirror_error_action(s, false, -ret);
>           if (action == BLOCK_ERROR_ACTION_REPORT && s->ret >= 0) {
> @@ -143,10 +142,9 @@ static void mirror_read_complete(void *opaque, int ret)
>       MirrorOp *op = opaque;
>       MirrorBlockJob *s = op->s;
>       if (ret < 0) {
> -        BlockDriverState *source = s->common.bs;
>           BlockErrorAction action;
>   
> -        bdrv_set_dirty_bitmap(source, s->dirty_bitmap, op->sector_num,
> +        bdrv_set_dirty_bitmap(s->dirty_bitmap, op->sector_num,
>                                 op->nb_sectors);
>           action = mirror_error_action(s, true, -ret);
>           if (action == BLOCK_ERROR_ACTION_REPORT && s->ret >= 0) {
> @@ -170,10 +168,10 @@ static uint64_t coroutine_fn 
> mirror_iteration(MirrorBlockJob *s)
>   
>       s->sector_num = hbitmap_iter_next(&s->hbi);
>       if (s->sector_num < 0) {
> -        bdrv_dirty_iter_init(source, s->dirty_bitmap, &s->hbi);
> +        bdrv_dirty_iter_init(s->dirty_bitmap, &s->hbi);
>           s->sector_num = hbitmap_iter_next(&s->hbi);
>           trace_mirror_restart_iter(s,
> -                                  bdrv_get_dirty_count(source, 
> s->dirty_bitmap));
> +                                  bdrv_get_dirty_count(s->dirty_bitmap));

You can probably join those two lines now (maybe there are more places 
where that's possible, I haven't checked, but this looks awfully 
suspicious; I see that you did join some lines, so I know that you're 
not opposed to it in principle).

Whether you do that or not (here and in all the places possible):

Reviewed-by: Max Reitz <address@hidden>