From 592203e7cfbd1ad261178431fcf390adfe8b16df Mon Sep 17 00:00:00 2001 From: John Snow Date: Tue, 28 May 2019 19:33:31 -0400 Subject: migration/dirty-bitmaps: change bitmap enumeration method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Shift from looking at every root BDS to *every* BDS. This will migrate bitmaps that are attached to blockdev created nodes instead of just ones attached to emulated storage devices. Note that this will not migrate anonymous or internal-use bitmaps, as those are defined as having no name. This will also fix the Coverity issues Peter Maydell has been asking about for the past several releases, as well as fixing a real bug. Reported-by: Peter Maydell Reported-by: Coverity 😅 Reported-by: aihua liang Reviewed-by: Vladimir Sementsov-Ogievskiy Signed-off-by: John Snow Message-id: 20190514201926.10407-1-jsnow@redhat.com Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1652490 Fixes: Coverity CID 1390625 CC: Stefan Hajnoczi Signed-off-by: John Snow --- migration/block-dirty-bitmap.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c index d1bb863cb6..4a896a09eb 100644 --- a/migration/block-dirty-bitmap.c +++ b/migration/block-dirty-bitmap.c @@ -273,7 +273,6 @@ static int init_dirty_bitmap_migration(void) BlockDriverState *bs; BdrvDirtyBitmap *bitmap; DirtyBitmapMigBitmapState *dbms; - BdrvNextIterator it; Error *local_err = NULL; dirty_bitmap_mig_state.bulk_completed = false; @@ -281,13 +280,8 @@ static int init_dirty_bitmap_migration(void) dirty_bitmap_mig_state.prev_bitmap = NULL; dirty_bitmap_mig_state.no_bitmaps = false; - for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) { - const char *drive_name = bdrv_get_device_or_node_name(bs); - - /* skip automatically inserted nodes */ - while (bs && bs->drv && bs->implicit) { - bs = backing_bs(bs); - } + for (bs = bdrv_next_all_states(NULL); bs; bs = bdrv_next_all_states(bs)) { + const char *name = bdrv_get_device_or_node_name(bs); for (bitmap = bdrv_dirty_bitmap_next(bs, NULL); bitmap; bitmap = bdrv_dirty_bitmap_next(bs, bitmap)) @@ -296,7 +290,7 @@ static int init_dirty_bitmap_migration(void) continue; } - if (drive_name == NULL) { + if (!name || strcmp(name, "") == 0) { error_report("Found bitmap '%s' in unnamed node %p. It can't " "be migrated", bdrv_dirty_bitmap_name(bitmap), bs); goto fail; @@ -313,7 +307,7 @@ static int init_dirty_bitmap_migration(void) dbms = g_new0(DirtyBitmapMigBitmapState, 1); dbms->bs = bs; - dbms->node_name = drive_name; + dbms->node_name = name; dbms->bitmap = bitmap; dbms->total_sectors = bdrv_nb_sectors(bs); dbms->sectors_per_chunk = CHUNK_SIZE * 8 * -- cgit v1.2.3 From eff0829b0710d60c1846eb99baeb47b2478a414a Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Tue, 28 May 2019 19:33:31 -0400 Subject: qapi: support external bitmaps in block-dirty-bitmap-merge 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 Reviewed-by: John Snow Message-id: 20190517152111.206494-2-vsementsov@virtuozzo.com Signed-off-by: John Snow --- block/dirty-bitmap.c | 9 ++++++--- blockdev.c | 50 +++++++++++++++++++++++++++++++++----------------- qapi/block-core.json | 22 +++++++++++++++++++--- 3 files changed, 58 insertions(+), 23 deletions(-) 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); } 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: -- cgit v1.2.3 From 403bb8185ec18267fe47a0e304d26a17263572dc Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Tue, 28 May 2019 19:33:31 -0400 Subject: iotests: test external snapshot with bitmap copying This test shows that external snapshots and incremental backups are friends. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: John Snow Message-id: 20190517152111.206494-3-vsementsov@virtuozzo.com Signed-off-by: John Snow --- tests/qemu-iotests/254 | 52 ++++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/254.out | 52 ++++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/group | 1 + 3 files changed, 105 insertions(+) create mode 100755 tests/qemu-iotests/254 create mode 100644 tests/qemu-iotests/254.out diff --git a/tests/qemu-iotests/254 b/tests/qemu-iotests/254 new file mode 100755 index 0000000000..33cb80a512 --- /dev/null +++ b/tests/qemu-iotests/254 @@ -0,0 +1,52 @@ +#!/usr/bin/env python +# +# Test external snapshot with bitmap copying. +# +# Copyright (c) 2019 Virtuozzo International GmbH. All rights reserved. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +import iotests +from iotests import qemu_img_create, file_path, log + +disk, top = file_path('disk', 'top') +size = 1024 * 1024 + +qemu_img_create('-f', iotests.imgfmt, disk, str(size)) + +vm = iotests.VM().add_drive(disk, opts='node-name=base') +vm.launch() + +vm.qmp_log('block-dirty-bitmap-add', node='drive0', name='bitmap0') + +vm.hmp_qemu_io('drive0', 'write 0 512K') + +vm.qmp_log('transaction', indent=2, actions=[ + {'type': 'blockdev-snapshot-sync', + 'data': {'device': 'drive0', 'snapshot-file': top, + 'snapshot-node-name': 'snap'}}, + {'type': 'block-dirty-bitmap-add', + 'data': {'node': 'snap', 'name': 'bitmap0'}}, + {'type': 'block-dirty-bitmap-merge', + 'data': {'node': 'snap', 'target': 'bitmap0', + 'bitmaps': [{'node': 'base', 'name': 'bitmap0'}]}} +], filters=[iotests.filter_qmp_testfiles]) + +result = vm.qmp('query-block')['return'][0] +log("query-block: device = {}, node-name = {}, dirty-bitmaps:".format( + result['device'], result['inserted']['node-name'])) +log(result['dirty-bitmaps'], indent=2) + +vm.shutdown() diff --git a/tests/qemu-iotests/254.out b/tests/qemu-iotests/254.out new file mode 100644 index 0000000000..d7394cf002 --- /dev/null +++ b/tests/qemu-iotests/254.out @@ -0,0 +1,52 @@ +{"execute": "block-dirty-bitmap-add", "arguments": {"name": "bitmap0", "node": "drive0"}} +{"return": {}} +{ + "execute": "transaction", + "arguments": { + "actions": [ + { + "data": { + "device": "drive0", + "snapshot-file": "TEST_DIR/PID-top", + "snapshot-node-name": "snap" + }, + "type": "blockdev-snapshot-sync" + }, + { + "data": { + "name": "bitmap0", + "node": "snap" + }, + "type": "block-dirty-bitmap-add" + }, + { + "data": { + "bitmaps": [ + { + "name": "bitmap0", + "node": "base" + } + ], + "node": "snap", + "target": "bitmap0" + }, + "type": "block-dirty-bitmap-merge" + } + ] + } +} +{ + "return": {} +} +query-block: device = drive0, node-name = snap, dirty-bitmaps: +[ + { + "busy": false, + "count": 524288, + "granularity": 65536, + "name": "bitmap0", + "persistent": false, + "recording": true, + "status": "active" + } +] diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index 2c74deec00..859c4b5e9f 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -264,3 +264,4 @@ 249 rw auto quick 252 rw auto backing quick 253 rw auto quick +254 rw auto backing quick -- cgit v1.2.3