| Message ID | 20190517152111.206494-2-vsementsov@virtuozzo.com |
|---|---|
| State | New |
| Headers | show |
| Series | bitmaps: merge bitmaps from differentnodes | expand |
On 5/17/19 11:21 AM, Vladimir Sementsov-Ogievskiy wrote: > Add new optional parameter making possible to merge bitmaps from > different nodes. It is needed to maintain external snapshots during > incremental backup chain history. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > qapi/block-core.json | 22 ++++++++++++++++--- > block/dirty-bitmap.c | 9 +++++--- > blockdev.c | 50 +++++++++++++++++++++++++++++--------------- > 3 files changed, 58 insertions(+), 23 deletions(-) > > diff --git a/qapi/block-core.json b/qapi/block-core.json > index 7ccbfff9d0..dcc935d655 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -2003,19 +2003,35 @@ > 'data': { 'node': 'str', 'name': 'str', '*granularity': 'uint32', > '*persistent': 'bool', '*autoload': 'bool', '*disabled': 'bool' } } > > +## > +# @BlockDirtyBitmapMergeSource: > +# > +# @local: name of the bitmap, attached to the same node as target bitmap. > +# > +# @external: bitmap with specified node > +# > +# Since: 4.1 > +## > +{ 'alternate': 'BlockDirtyBitmapMergeSource', > + 'data': { 'local': 'str', > + 'external': 'BlockDirtyBitmap' } } > + We might be able to use something more generic to name this type of thing, but I think such changes are wire compatible, so we can rename it to be more generic if we decide to use this for something else in the future, so this is good. > ## > # @BlockDirtyBitmapMerge: > # > -# @node: name of device/node which the bitmap is tracking > +# @node: name of device/node which the @target bitmap is tracking > # > # @target: name of the destination dirty bitmap > # > -# @bitmaps: name(s) of the source dirty bitmap(s) > +# @bitmaps: name(s) of the source dirty bitmap(s) at @node and/or fully > +# specifed BlockDirtyBitmap elements. The latter are supported > +# since 4.1. > # > # Since: 4.0 > ## > { 'struct': 'BlockDirtyBitmapMerge', > - 'data': { 'node': 'str', 'target': 'str', 'bitmaps': ['str'] } } > + 'data': { 'node': 'str', 'target': 'str', > + 'bitmaps': ['BlockDirtyBitmapMergeSource'] } } > > ## > # @block-dirty-bitmap-add: > diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c > index 59e6ebb861..49646a30e6 100644 > --- a/block/dirty-bitmap.c > +++ b/block/dirty-bitmap.c > @@ -816,10 +816,10 @@ void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src, > { > bool ret; > > - /* only bitmaps from one bds are supported */ > - assert(dest->mutex == src->mutex); > - > qemu_mutex_lock(dest->mutex); > + if (src->mutex != dest->mutex) { > + qemu_mutex_lock(src->mutex); > + } > > if (bdrv_dirty_bitmap_check(dest, BDRV_BITMAP_DEFAULT, errp)) { > goto out; > @@ -845,4 +845,7 @@ void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src, > > out: > qemu_mutex_unlock(dest->mutex); > + if (src->mutex != dest->mutex) { > + qemu_mutex_unlock(src->mutex); > + } > } > diff --git a/blockdev.c b/blockdev.c > index 79fbac8450..64ccef735b 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -2112,11 +2112,10 @@ static void block_dirty_bitmap_disable_abort(BlkActionState *common) > } > } > > -static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(const char *node, > - const char *target, > - strList *bitmaps, > - HBitmap **backup, > - Error **errp); > +static BdrvDirtyBitmap *do_block_dirty_bitmap_merge( > + const char *node, const char *target, > + BlockDirtyBitmapMergeSourceList *bitmaps, > + HBitmap **backup, Error **errp); > > static void block_dirty_bitmap_merge_prepare(BlkActionState *common, > Error **errp) > @@ -2965,15 +2964,14 @@ void qmp_block_dirty_bitmap_disable(const char *node, const char *name, > bdrv_disable_dirty_bitmap(bitmap); > } > > -static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(const char *node, > - const char *target, > - strList *bitmaps, > - HBitmap **backup, > - Error **errp) > +static BdrvDirtyBitmap *do_block_dirty_bitmap_merge( > + const char *node, const char *target, > + BlockDirtyBitmapMergeSourceList *bitmaps, > + HBitmap **backup, Error **errp) > { > BlockDriverState *bs; > BdrvDirtyBitmap *dst, *src, *anon; > - strList *lst; > + BlockDirtyBitmapMergeSourceList *lst; > Error *local_err = NULL; > > dst = block_dirty_bitmap_lookup(node, target, &bs, errp); > @@ -2988,11 +2986,28 @@ static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(const char *node, > } > > for (lst = bitmaps; lst; lst = lst->next) { > - src = bdrv_find_dirty_bitmap(bs, lst->value); > - if (!src) { > - error_setg(errp, "Dirty bitmap '%s' not found", lst->value); > - dst = NULL; > - goto out; > + switch (lst->value->type) { > + const char *name, *node; > + case QTYPE_QSTRING: > + name = lst->value->u.local; > + src = bdrv_find_dirty_bitmap(bs, name); > + if (!src) { > + error_setg(errp, "Dirty bitmap '%s' not found", name); > + dst = NULL; > + goto out; > + } > + break; > + case QTYPE_QDICT: > + node = lst->value->u.external.node; > + name = lst->value->u.external.name; > + src = block_dirty_bitmap_lookup(node, name, NULL, errp); > + if (!src) { > + dst = NULL; > + goto out; > + } > + break; > + default: > + abort(); > } > > bdrv_merge_dirty_bitmap(anon, src, NULL, &local_err); > @@ -3012,7 +3027,8 @@ static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(const char *node, > } > > void qmp_block_dirty_bitmap_merge(const char *node, const char *target, > - strList *bitmaps, Error **errp) > + BlockDirtyBitmapMergeSourceList *bitmaps, > + Error **errp) > { > do_block_dirty_bitmap_merge(node, target, bitmaps, NULL, errp); > } > Reviewed-by: John Snow <jsnow@redhat.com>
On 5/17/19 5:45 PM, John Snow wrote: > > > On 5/17/19 11:21 AM, Vladimir Sementsov-Ogievskiy wrote: >> Add new optional parameter making possible to merge bitmaps from >> different nodes. It is needed to maintain external snapshots during >> incremental backup chain history. >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> --- >> +# Since: 4.1 >> +## >> +{ 'alternate': 'BlockDirtyBitmapMergeSource', >> + 'data': { 'local': 'str', >> + 'external': 'BlockDirtyBitmap' } } >> + > > We might be able to use something more generic to name this type of > thing, but I think such changes are wire compatible, so we can rename it > to be more generic if we decide to use this for something else in the > future, so this is good. Correct - type names are not API, so we can rename them later if it makes sense.
diff --git a/qapi/block-core.json b/qapi/block-core.json index 7ccbfff9d0..dcc935d655 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -2003,19 +2003,35 @@ 'data': { 'node': 'str', 'name': 'str', '*granularity': 'uint32', '*persistent': 'bool', '*autoload': 'bool', '*disabled': 'bool' } } +## +# @BlockDirtyBitmapMergeSource: +# +# @local: name of the bitmap, attached to the same node as target bitmap. +# +# @external: bitmap with specified node +# +# Since: 4.1 +## +{ 'alternate': 'BlockDirtyBitmapMergeSource', + 'data': { 'local': 'str', + 'external': 'BlockDirtyBitmap' } } + ## # @BlockDirtyBitmapMerge: # -# @node: name of device/node which the bitmap is tracking +# @node: name of device/node which the @target bitmap is tracking # # @target: name of the destination dirty bitmap # -# @bitmaps: name(s) of the source dirty bitmap(s) +# @bitmaps: name(s) of the source dirty bitmap(s) at @node and/or fully +# specifed BlockDirtyBitmap elements. The latter are supported +# since 4.1. # # Since: 4.0 ## { 'struct': 'BlockDirtyBitmapMerge', - 'data': { 'node': 'str', 'target': 'str', 'bitmaps': ['str'] } } + 'data': { 'node': 'str', 'target': 'str', + 'bitmaps': ['BlockDirtyBitmapMergeSource'] } } ## # @block-dirty-bitmap-add: diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c index 59e6ebb861..49646a30e6 100644 --- a/block/dirty-bitmap.c +++ b/block/dirty-bitmap.c @@ -816,10 +816,10 @@ void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src, { bool ret; - /* only bitmaps from one bds are supported */ - assert(dest->mutex == src->mutex); - qemu_mutex_lock(dest->mutex); + if (src->mutex != dest->mutex) { + qemu_mutex_lock(src->mutex); + } if (bdrv_dirty_bitmap_check(dest, BDRV_BITMAP_DEFAULT, errp)) { goto out; @@ -845,4 +845,7 @@ void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src, out: qemu_mutex_unlock(dest->mutex); + if (src->mutex != dest->mutex) { + qemu_mutex_unlock(src->mutex); + } } diff --git a/blockdev.c b/blockdev.c index 79fbac8450..64ccef735b 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2112,11 +2112,10 @@ static void block_dirty_bitmap_disable_abort(BlkActionState *common) } } -static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(const char *node, - const char *target, - strList *bitmaps, - HBitmap **backup, - Error **errp); +static BdrvDirtyBitmap *do_block_dirty_bitmap_merge( + const char *node, const char *target, + BlockDirtyBitmapMergeSourceList *bitmaps, + HBitmap **backup, Error **errp); static void block_dirty_bitmap_merge_prepare(BlkActionState *common, Error **errp) @@ -2965,15 +2964,14 @@ void qmp_block_dirty_bitmap_disable(const char *node, const char *name, bdrv_disable_dirty_bitmap(bitmap); } -static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(const char *node, - const char *target, - strList *bitmaps, - HBitmap **backup, - Error **errp) +static BdrvDirtyBitmap *do_block_dirty_bitmap_merge( + const char *node, const char *target, + BlockDirtyBitmapMergeSourceList *bitmaps, + HBitmap **backup, Error **errp) { BlockDriverState *bs; BdrvDirtyBitmap *dst, *src, *anon; - strList *lst; + BlockDirtyBitmapMergeSourceList *lst; Error *local_err = NULL; dst = block_dirty_bitmap_lookup(node, target, &bs, errp); @@ -2988,11 +2986,28 @@ static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(const char *node, } for (lst = bitmaps; lst; lst = lst->next) { - src = bdrv_find_dirty_bitmap(bs, lst->value); - if (!src) { - error_setg(errp, "Dirty bitmap '%s' not found", lst->value); - dst = NULL; - goto out; + switch (lst->value->type) { + const char *name, *node; + case QTYPE_QSTRING: + name = lst->value->u.local; + src = bdrv_find_dirty_bitmap(bs, name); + if (!src) { + error_setg(errp, "Dirty bitmap '%s' not found", name); + dst = NULL; + goto out; + } + break; + case QTYPE_QDICT: + node = lst->value->u.external.node; + name = lst->value->u.external.name; + src = block_dirty_bitmap_lookup(node, name, NULL, errp); + if (!src) { + dst = NULL; + goto out; + } + break; + default: + abort(); } bdrv_merge_dirty_bitmap(anon, src, NULL, &local_err); @@ -3012,7 +3027,8 @@ static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(const char *node, } void qmp_block_dirty_bitmap_merge(const char *node, const char *target, - strList *bitmaps, Error **errp) + BlockDirtyBitmapMergeSourceList *bitmaps, + Error **errp) { do_block_dirty_bitmap_merge(node, target, bitmaps, NULL, errp); }
Add new optional parameter making possible to merge bitmaps from different nodes. It is needed to maintain external snapshots during incremental backup chain history. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- qapi/block-core.json | 22 ++++++++++++++++--- block/dirty-bitmap.c | 9 +++++--- blockdev.c | 50 +++++++++++++++++++++++++++++--------------- 3 files changed, 58 insertions(+), 23 deletions(-)