Re: [Qemu-block] [Qemu-devel] [PATCH 5/7] blockdev: refactor block-dirty-bitmap-clear transaction

[Vladimir Sementsov-Ogievskiy]

26.03.2018 14:44, Paolo Bonzini wrote:
> On 26/03/2018 13:20, Vladimir Sementsov-Ogievskiy wrote:
> > bdrv_clear_dirty_bitmap do not fail, so we can call it in transaction
> > commit, avoiding any rollback.
> >
> > Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> > ---
> >   blockdev.c | 16 +---------------
> >   1 file changed, 1 insertion(+), 15 deletions(-)
> >
> > diff --git a/blockdev.c b/blockdev.c
> > index c31bf3d98d..88eae60c1c 100644
> > --- a/blockdev.c
> > +++ b/blockdev.c
> > @@ -2050,7 +2050,6 @@ typedef struct BlockDirtyBitmapState {
> >       BlkActionState common;
> >       BdrvDirtyBitmap *bitmap;
> >       BlockDriverState *bs;
> > -    HBitmap *backup;
> >       bool prepared;
> >   } BlockDirtyBitmapState;
> >   
> > @@ -2129,18 +2128,6 @@ static void block_dirty_bitmap_clear_prepare(BlkActionState *common,
> >           error_setg(errp, "Cannot clear a readonly bitmap");
> >           return;
> >       }
> > -
> > -    bdrv_clear_dirty_bitmap(state->bitmap, &state->backup);
> > -}
> > -
> > -static void block_dirty_bitmap_clear_abort(BlkActionState *common)
> > -{
> > -    BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
> > -                                             common, common);
> > -
> > -    if (state->backup) {
> > -        bdrv_undo_clear_dirty_bitmap(state->bitmap, state->backup);
> > -    }
> Isn't bdrv_undo_clear_dirty_bitmap new unused?

hmm, yes. I'll remove it in v2.

> Thanks,
>
> Paolo
>
> >   }
> >   
> >   static void block_dirty_bitmap_clear_commit(BlkActionState *common)
> > @@ -2148,7 +2135,7 @@ static void 
> > block_dirty_bitmap_clear_commit(BlkActionState *common)
> >       BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
> >                                                common, common);
> >   
> > -    hbitmap_free(state->backup);
> > +    bdrv_clear_dirty_bitmap(state->bitmap, NULL);
> >   }
> >   
> >   static void abort_prepare(BlkActionState *common, Error **errp)
> > @@ -2210,7 +2197,6 @@ static const BlkActionOps actions[] = {
> >           .instance_size = sizeof(BlockDirtyBitmapState),
> >           .prepare = block_dirty_bitmap_clear_prepare,
> >           .commit = block_dirty_bitmap_clear_commit,
> > -        .abort = block_dirty_bitmap_clear_abort,
> >       }
> >   };
> >   


--
Best regards,
Vladimir