On 01.07.19 22:13, John Snow wrote: > It is used to do transactional movement of the bitmap (which is > possible in conjunction with merge command). Transactional bitmap > movement is needed in scenarios with external snapshot, when we don't > want to leave copy of the bitmap in the base image. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> > Signed-off-by: John Snow <js...@redhat.com> > --- > qapi/transaction.json | 2 + > include/block/dirty-bitmap.h | 3 +- > block.c | 2 +- > block/dirty-bitmap.c | 16 +++---- > blockdev.c | 79 +++++++++++++++++++++++++++++++--- > migration/block-dirty-bitmap.c | 2 +- > 6 files changed, 87 insertions(+), 17 deletions(-)
[...] > diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c > index 95a9c2a5d8..8551f8219e 100644 > --- a/block/dirty-bitmap.c > +++ b/block/dirty-bitmap.c > @@ -48,10 +48,9 @@ struct BdrvDirtyBitmap { > bool inconsistent; /* bitmap is persistent, but inconsistent. > It cannot be used at all in any way, > except > a QMP user can remove it. */ > - bool migration; /* Bitmap is selected for migration, it > should > - not be stored on the next inactivation > - (persistent flag doesn't matter until next > - invalidation).*/ > + bool squelch_persistence; /* We are either migrating or deleting this > + * bitmap; it should not be stored on the > next > + * inactivation. */ I like the English lessons you give me, but why not just dont_store? Or skip_storing? > QLIST_ENTRY(BdrvDirtyBitmap) list; > }; > [...] > diff --git a/blockdev.c b/blockdev.c > index 01248252ca..4143ab7bbb 100644 > --- a/blockdev.c > +++ b/blockdev.c [...] > +static void block_dirty_bitmap_remove_abort(BlkActionState *common) > +{ > + BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState, > + common, common); > + > + if (state->bitmap) { > + bdrv_dirty_bitmap_squelch_persistence(state->bitmap, false); > + bdrv_dirty_bitmap_set_busy(state->bitmap, false); Don’t you need to undo the removal? Like, re-add it to state->bs, and re-store it? [...] > @@ -2892,13 +2944,28 @@ void qmp_block_dirty_bitmap_remove(const char *node, > const char *name, > aio_context_acquire(aio_context); > bdrv_remove_persistent_dirty_bitmap(bs, name, &local_err); > aio_context_release(aio_context); > + > if (local_err != NULL) { > error_propagate(errp, local_err); > - return; > + return NULL; > } > } > > - bdrv_release_dirty_bitmap(bs, bitmap); > + if (release) { > + bdrv_release_dirty_bitmap(bs, bitmap); > + } > + > + if (bitmap_bs) { > + *bitmap_bs = bs; > + } > + > + return bitmap; I’d prefer “release ? NULL : bitmap”. Max > +} > + > +void qmp_block_dirty_bitmap_remove(const char *node, const char *name, > + Error **errp) > +{ > + do_block_dirty_bitmap_remove(node, name, true, NULL, errp); > } > > /**
signature.asc
Description: OpenPGP digital signature