OpenE2K/qemu-e2k
13
4
Fork 0
Code Issues 4 Pull Requests Projects 2 Releases Activity

bitmaps patches for 2021-02-12

Browse Source 
- add 'transform' member to manipulate bitmaps across migration
 - work towards better error handling during bdrv_open
 -----BEGIN PGP SIGNATURE-----
 
 iQEzBAABCAAdFiEEccLMIrHEYCkn0vOqp6FrSiUnQ2oFAmAnDQsACgkQp6FrSiUn
 Q2qc5Qf/SKVdpX4j7OnHF6sBuf/8LVWz4KazSqEU0ohazBJmafgJpH2EA5pXMXR4
 frZDWeanGmhj1MjMkta/++uvEBU/TMpW2z98mZvjErteXdnRQAlII/hOCI+QZJvg
 viQ5t1EyrkyXzUePOjs+AwqA5KHWbCKt6QqyItQ78HvI23sw/fuvHj0G67KbVzXZ
 VcSrVr0J7PXnZV/hWfg+C+Nn9Ro9tsVdn79awLYVQ7/SDro3hzylpcHMQaHMK2oe
 mX4D2kNq7s21E27Zb6vlknUhQPkMdETk0gfEbpn7sTVMEc58GRLC7Tqfx7l0JIFK
 5izVyA5vndKVxDGYPkbDK6VL2uDg4A==
 =+Epy
 -----END PGP SIGNATURE-----

Merge remote-tracking branch 'remotes/ericb/tags/pull-bitmaps-2021-02-12' into staging

bitmaps patches for 2021-02-12

- add 'transform' member to manipulate bitmaps across migration
- work towards better error handling during bdrv_open

# gpg: Signature made Fri 12 Feb 2021 23:19:39 GMT
# gpg:                using RSA key 71C2CC22B1C4602927D2F3AAA7A16B4A2527436A
# gpg: Good signature from "Eric Blake <eblake@redhat.com>" [full]
# gpg:                 aka "Eric Blake (Free Software Programmer) <ebb9@byu.net>" [full]
# gpg:                 aka "[jpeg image of size 6874]" [full]
# Primary key fingerprint: 71C2 CC22 B1C4 6029 27D2  F3AA A7A1 6B4A 2527 436A

* remotes/ericb/tags/pull-bitmaps-2021-02-12:
  block: use return status of bdrv_append()
  block: return status from bdrv_append and friends
  qemu-iotests: 300: Add test case for modifying persistence of bitmap
  migration: dirty-bitmap: Allow control of bitmap persistence
  migration: dirty-bitmap: Use struct for alias map inner members

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
This commit is contained in:
Peter Maydell 2021-02-13 21:26:00 +00:00
commit 392b9a74b9
11 changed files with 226 additions and 75 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

54
block.c
View File

@ -2827,14 +2827,15 @@ static BdrvChildRole bdrv_backing_role(BlockDriverState *bs)
* Sets the bs->backing link of a BDS. A new reference is created; callers
* which don't need their own reference any more must call bdrv_unref().
*/
void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
int bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
Error **errp)
{
int ret = 0;
bool update_inherits_from = bdrv_chain_contains(bs, backing_hd) &&
bdrv_inherits_from_recursive(backing_hd, bs);
if (bdrv_is_backing_chain_frozen(bs, child_bs(bs->backing), errp)) {
return;
return -EPERM;
}
if (backing_hd) {
@ -2853,15 +2854,22 @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
bs->backing = bdrv_attach_child(bs, backing_hd, "backing", &child_of_bds,
bdrv_backing_role(bs), errp);
if (!bs->backing) {
ret = -EPERM;
goto out;
}
/* If backing_hd was already part of bs's backing chain, and
* inherits_from pointed recursively to bs then let's update it to
* point directly to bs (else it will become NULL). */
if (bs->backing && update_inherits_from) {
if (update_inherits_from) {
backing_hd->inherits_from = bs;
}
out:
bdrv_refresh_limits(bs, NULL);
return ret;
}
/*
@ -3110,7 +3118,6 @@ static BlockDriverState *bdrv_append_temp_snapshot(BlockDriverState *bs,
int64_t total_size;
QemuOpts *opts = NULL;
BlockDriverState *bs_snapshot = NULL;
Error *local_err = NULL;
int ret;
/* if snapshot, we create a temporary backing file and open it
@ -3157,9 +3164,8 @@ static BlockDriverState *bdrv_append_temp_snapshot(BlockDriverState *bs,
* order to be able to return one, we have to increase
* bs_snapshot's refcount here */
bdrv_ref(bs_snapshot);
bdrv_append(bs_snapshot, bs, &local_err);
if (local_err) {
error_propagate(errp, local_err);
ret = bdrv_append(bs_snapshot, bs, errp);
if (ret < 0) {
bs_snapshot = NULL;
goto out;
}
@ -4532,7 +4538,7 @@ static bool should_update_child(BdrvChild *c, BlockDriverState *to)
* With auto_skip=false the error is returned if from has a parent which should
* not be updated.
*/
static void bdrv_replace_node_common(BlockDriverState *from,
static int bdrv_replace_node_common(BlockDriverState *from,
BlockDriverState *to,
bool auto_skip, Error **errp)
{
@ -4556,11 +4562,13 @@ static void bdrv_replace_node_common(BlockDriverState *from,
if (auto_skip) {
continue;
}
ret = -EINVAL;
error_setg(errp, "Should not change '%s' link to '%s'",
c->name, from->node_name);
goto out;
}
if (c->frozen) {
ret = -EPERM;
error_setg(errp, "Cannot change '%s' link to '%s'",
c->name, from->node_name);
goto out;
@ -4591,13 +4599,17 @@ static void bdrv_replace_node_common(BlockDriverState *from,
bdrv_set_perm(to);
ret = 0;
out:
g_slist_free(list);
bdrv_drained_end(from);
bdrv_unref(from);
return ret;
}
void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
int bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
Error **errp)
{
return bdrv_replace_node_common(from, to, true, errp);
@ -4619,28 +4631,30 @@ void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
* parents of bs_top after bdrv_append() returns. If the caller needs to keep a
* reference of its own, it must call bdrv_ref().
*/
void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top,
int bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top,
Error **errp)
{
Error *local_err = NULL;
bdrv_set_backing_hd(bs_new, bs_top, &local_err);
if (local_err) {
error_propagate(errp, local_err);
int ret = bdrv_set_backing_hd(bs_new, bs_top, errp);
if (ret < 0) {
goto out;
}
bdrv_replace_node(bs_top, bs_new, &local_err);
if (local_err) {
error_propagate(errp, local_err);
ret = bdrv_replace_node(bs_top, bs_new, errp);
if (ret < 0) {
bdrv_set_backing_hd(bs_new, NULL, &error_abort);
goto out;
}
/* bs_new is now referenced by its new parents, we don't need the
* additional reference any more. */
ret = 0;
out:
/*
* bs_new is now referenced by its new parents, we don't need the
* additional reference any more.
*/
bdrv_unref(bs_new);
return ret;
}
static void bdrv_delete(BlockDriverState *bs)

23
block/backup-top.c
View File

@ -191,7 +191,8 @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
BlockCopyState **bcs,
Error **errp)
{
Error *local_err = NULL;
ERRP_GUARD();
int ret;
BDRVBackupTopState *state;
BlockDriverState *top;
bool appended = false;
@ -224,9 +225,9 @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
bdrv_drained_begin(source);
bdrv_ref(top);
bdrv_append(top, source, &local_err);
if (local_err) {
error_prepend(&local_err, "Cannot append backup-top filter: ");
ret = bdrv_append(top, source, errp);
if (ret < 0) {
error_prepend(errp, "Cannot append backup-top filter: ");
goto fail;
}
appended = true;
@ -236,19 +237,18 @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
* we want.
*/
state->active = true;
bdrv_child_refresh_perms(top, top->backing, &local_err);
if (local_err) {
error_prepend(&local_err,
"Cannot set permissions for backup-top filter: ");
ret = bdrv_child_refresh_perms(top, top->backing, errp);
if (ret < 0) {
error_prepend(errp, "Cannot set permissions for backup-top filter: ");
goto fail;
}
state->cluster_size = cluster_size;
state->bcs = block_copy_state_new(top->backing, state->target,
cluster_size, perf->use_copy_range,
write_flags, &local_err);
if (local_err) {
error_prepend(&local_err, "Cannot create block-copy-state: ");
write_flags, errp);
if (!state->bcs) {
error_prepend(errp, "Cannot create block-copy-state: ");
goto fail;
}
*bcs = state->bcs;
@ -266,7 +266,6 @@ fail:
}
bdrv_drained_end(source);
error_propagate(errp, local_err);
return NULL;
}

6
block/commit.c
View File

@ -254,7 +254,6 @@ void commit_start(const char *job_id, BlockDriverState *bs,
BlockDriverState *iter;
BlockDriverState *commit_top_bs = NULL;
BlockDriverState *filtered_base;
Error *local_err = NULL;
int64_t base_size, top_size;
uint64_t base_perms, iter_shared_perms;
int ret;
@ -312,10 +311,9 @@ void commit_start(const char *job_id, BlockDriverState *bs,
commit_top_bs->total_sectors = top->total_sectors;
bdrv_append(commit_top_bs, top, &local_err);
if (local_err) {
ret = bdrv_append(commit_top_bs, top, errp);
if (ret < 0) {
commit_top_bs = NULL;
error_propagate(errp, local_err);
goto fail;
}

6
block/mirror.c
View File

@ -1569,7 +1569,6 @@ static BlockJob *mirror_start_job(
BlockDriverState *mirror_top_bs;
bool target_is_backing;
uint64_t target_perms, target_shared_perms;
Error *local_err = NULL;
int ret;
if (granularity == 0) {
@ -1618,12 +1617,11 @@ static BlockJob *mirror_start_job(
* it alive until block_job_create() succeeds even if bs has no parent. */
bdrv_ref(mirror_top_bs);
bdrv_drained_begin(bs);
bdrv_append(mirror_top_bs, bs, &local_err);
ret = bdrv_append(mirror_top_bs, bs, errp);
bdrv_drained_end(bs);
if (local_err) {
if (ret < 0) {
bdrv_unref(mirror_top_bs);
error_propagate(errp, local_err);
return NULL;
}

6
blockdev.c
View File

@ -1432,6 +1432,7 @@ typedef struct ExternalSnapshotState {
static void external_snapshot_prepare(BlkActionState *common,
Error **errp)
{
int ret;
int flags = 0;
QDict *options = NULL;
Error *local_err = NULL;
@ -1591,9 +1592,8 @@ static void external_snapshot_prepare(BlkActionState *common,
* can fail, so we need to do it in .prepare; undoing it for abort is
* always possible. */
bdrv_ref(state->new_bs);
bdrv_append(state->new_bs, state->old_bs, &local_err);
if (local_err) {
error_propagate(errp, local_err);
ret = bdrv_append(state->new_bs, state->old_bs, errp);
if (ret < 0) {
goto out;
}
state->overlay_appended = true;

6
include/block/block.h
View File

@ -356,9 +356,9 @@ int bdrv_create(BlockDriver *drv, const char* filename,
int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp);
BlockDriverState *bdrv_new(void);
void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top,
int bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top,
Error **errp);
void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
int bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
Error **errp);
BlockDriverState *bdrv_insert_node(BlockDriverState *bs, QDict *node_options,
int flags, Error **errp);
@ -373,7 +373,7 @@ BdrvChild *bdrv_open_child(const char *filename,
BdrvChildRole child_role,
bool allow_none, Error **errp);
BlockDriverState *bdrv_open_blockdev_ref(BlockdevRef *ref, Error **errp);
void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
int bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
Error **errp);
int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
const char *bdref_key, Error **errp);

58
migration/block-dirty-bitmap.c
View File

@ -75,6 +75,8 @@
#include "qemu/id.h"
#include "qapi/error.h"
#include "qapi/qapi-commands-migration.h"
#include "qapi/qapi-visit-migration.h"
#include "qapi/clone-visitor.h"
#include "trace.h"
#define CHUNK_SIZE (1 << 10)
@ -148,6 +150,7 @@ typedef struct DBMLoadState {
BdrvDirtyBitmap *bitmap;
bool before_vm_start_handled; /* set in dirty_bitmap_mig_before_vm_start */
BitmapMigrationBitmapAlias *bmap_inner;
/*
* cancelled
@ -224,6 +227,7 @@ static GHashTable *construct_alias_map(const BitmapMigrationNodeAliasList *bbm,
AliasMapInnerNode *amin;
GHashTable *bitmaps_map;
const char *node_map_from, *node_map_to;
GDestroyNotify gdn;
if (!id_wellformed(bmna->alias)) {
error_setg(errp, "The node alias '%s' is not well-formed",
@ -263,8 +267,9 @@ static GHashTable *construct_alias_map(const BitmapMigrationNodeAliasList *bbm,
node_map_to = bmna->node_name;
}
bitmaps_map = g_hash_table_new_full(g_str_hash, g_str_equal,
g_free, g_free);
gdn = (GDestroyNotify) qapi_free_BitmapMigrationBitmapAlias;
bitmaps_map = g_hash_table_new_full(g_str_hash, g_str_equal, g_free,
gdn);
amin = g_new(AliasMapInnerNode, 1);
*amin = (AliasMapInnerNode){
@ -276,7 +281,7 @@ static GHashTable *construct_alias_map(const BitmapMigrationNodeAliasList *bbm,
for (bmbal = bmna->bitmaps; bmbal; bmbal = bmbal->next) {
const BitmapMigrationBitmapAlias *bmba = bmbal->value;
const char *bmap_map_from, *bmap_map_to;
const char *bmap_map_from;
if (strlen(bmba->alias) > UINT8_MAX) {
error_setg(errp,
@ -293,7 +298,6 @@ static GHashTable *construct_alias_map(const BitmapMigrationNodeAliasList *bbm,
if (name_to_alias) {
bmap_map_from = bmba->name;
bmap_map_to = bmba->alias;
if (g_hash_table_contains(bitmaps_map, bmba->name)) {
error_setg(errp, "The bitmap '%s'/'%s' is mapped twice",
@ -302,7 +306,6 @@ static GHashTable *construct_alias_map(const BitmapMigrationNodeAliasList *bbm,
}
} else {
bmap_map_from = bmba->alias;
bmap_map_to = bmba->name;
if (g_hash_table_contains(bitmaps_map, bmba->alias)) {
error_setg(errp, "The bitmap alias '%s'/'%s' is used twice",
@ -311,8 +314,8 @@ static GHashTable *construct_alias_map(const BitmapMigrationNodeAliasList *bbm,
}
}
g_hash_table_insert(bitmaps_map,
g_strdup(bmap_map_from), g_strdup(bmap_map_to));
g_hash_table_insert(bitmaps_map, g_strdup(bmap_map_from),
QAPI_CLONE(BitmapMigrationBitmapAlias, bmba));
}
}
@ -527,6 +530,7 @@ static int add_bitmaps_to_list(DBMSaveState *s, BlockDriverState *bs,
}
FOR_EACH_DIRTY_BITMAP(bs, bitmap) {
BitmapMigrationBitmapAliasTransform *bitmap_transform = NULL;
bitmap_name = bdrv_dirty_bitmap_name(bitmap);
if (!bitmap_name) {
continue;
@ -538,11 +542,18 @@ static int add_bitmaps_to_list(DBMSaveState *s, BlockDriverState *bs,
}
if (bitmap_aliases) {
bitmap_alias = g_hash_table_lookup(bitmap_aliases, bitmap_name);
if (!bitmap_alias) {
BitmapMigrationBitmapAlias *bmap_inner;
bmap_inner = g_hash_table_lookup(bitmap_aliases, bitmap_name);
if (!bmap_inner) {
/* Skip bitmaps with no alias */
continue;
}
bitmap_alias = bmap_inner->alias;
if (bmap_inner->has_transform) {
bitmap_transform = bmap_inner->transform;
}
} else {
if (strlen(bitmap_name) > UINT8_MAX) {
error_report("Cannot migrate bitmap '%s' on node '%s': "
@ -568,9 +579,16 @@ static int add_bitmaps_to_list(DBMSaveState *s, BlockDriverState *bs,
if (bdrv_dirty_bitmap_enabled(bitmap)) {
dbms->flags |= DIRTY_BITMAP_MIG_START_FLAG_ENABLED;
}
if (bitmap_transform &&
bitmap_transform->has_persistent) {
if (bitmap_transform->persistent) {
dbms->flags |= DIRTY_BITMAP_MIG_START_FLAG_PERSISTENT;
}
} else {
if (bdrv_dirty_bitmap_get_persistence(bitmap)) {
dbms->flags |= DIRTY_BITMAP_MIG_START_FLAG_PERSISTENT;
}
}
QSIMPLEQ_INSERT_TAIL(&s->dbms_list, dbms, entry);
}
@ -777,6 +795,7 @@ static int dirty_bitmap_load_start(QEMUFile *f, DBMLoadState *s)
uint32_t granularity = qemu_get_be32(f);
uint8_t flags = qemu_get_byte(f);
LoadBitmapState *b;
bool persistent;
if (s->cancelled) {
return 0;
@ -801,7 +820,15 @@ static int dirty_bitmap_load_start(QEMUFile *f, DBMLoadState *s)
return -EINVAL;
}
if (flags & DIRTY_BITMAP_MIG_START_FLAG_PERSISTENT) {
if (s->bmap_inner &&
s->bmap_inner->has_transform &&
s->bmap_inner->transform->has_persistent) {
persistent = s->bmap_inner->transform->persistent;
} else {
persistent = flags & DIRTY_BITMAP_MIG_START_FLAG_PERSISTENT;
}
if (persistent) {
bdrv_dirty_bitmap_set_persistence(s->bitmap, true);
}
@ -1074,14 +1101,19 @@ static int dirty_bitmap_load_header(QEMUFile *f, DBMLoadState *s,
bitmap_name = s->bitmap_alias;
if (!s->cancelled && bitmap_alias_map) {
bitmap_name = g_hash_table_lookup(bitmap_alias_map,
s->bitmap_alias);
if (!bitmap_name) {
BitmapMigrationBitmapAlias *bmap_inner;
bmap_inner = g_hash_table_lookup(bitmap_alias_map, s->bitmap_alias);
if (!bmap_inner) {
error_report("Error: Unknown bitmap alias '%s' on node "
"'%s' (alias '%s')", s->bitmap_alias,
s->bs->node_name, s->node_alias);
cancel_incoming_locked(s);
} else {
bitmap_name = bmap_inner->name;
}
s->bmap_inner = bmap_inner;
}
if (!s->cancelled) {

19
qapi/migration.json
View File

@ -536,6 +536,19 @@
'data': [ 'none', 'zlib',
{ 'name': 'zstd', 'if': 'defined(CONFIG_ZSTD)' } ] }
##
# @BitmapMigrationBitmapAliasTransform:
#
# @persistent: If present, the bitmap will be made persistent
# or transient depending on this parameter.
#
# Since: 6.0
##
{ 'struct': 'BitmapMigrationBitmapAliasTransform',
'data': {
'*persistent': 'bool'
} }
##
# @BitmapMigrationBitmapAlias:
#
@ -544,12 +557,16 @@
# @alias: An alias name for migration (for example the bitmap name on
# the opposite site).
#
# @transform: Allows the modification of the migrated bitmap.
# (since 6.0)
#
# Since: 5.2
##
{ 'struct': 'BitmapMigrationBitmapAlias',
'data': {
'name': 'str',
'alias': 'str'
'alias': 'str',
'*transform': 'BitmapMigrationBitmapAliasTransform'
} }
##

93
tests/qemu-iotests/300
View File

@ -600,6 +600,99 @@ class TestCrossAliasMigration(TestDirtyBitmapMigration):
self.verify_dest_has_all_bitmaps()
self.verify_dest_error(None)
class TestAliasTransformMigration(TestDirtyBitmapMigration):
"""
Tests the 'transform' option which modifies bitmap persistence on migration.
"""
src_node_name = 'node-a'
dst_node_name = 'node-b'
src_bmap_name = 'bmap-a'
dst_bmap_name = 'bmap-b'
def setUp(self) -> None:
TestDirtyBitmapMigration.setUp(self)
# Now create another block device and let both have two bitmaps each
result = self.vm_a.qmp('blockdev-add',
node_name='node-b', driver='null-co',
read_zeroes=False)
self.assert_qmp(result, 'return', {})
result = self.vm_b.qmp('blockdev-add',
node_name='node-a', driver='null-co',
read_zeroes=False)
self.assert_qmp(result, 'return', {})
bmaps_to_add = (('node-a', 'bmap-b'),
('node-b', 'bmap-a'),
('node-b', 'bmap-b'))
for (node, bmap) in bmaps_to_add:
result = self.vm_a.qmp('block-dirty-bitmap-add',
node=node, name=bmap)
self.assert_qmp(result, 'return', {})
@staticmethod
def transform_mapping() -> BlockBitmapMapping:
return [
{
'node-name': 'node-a',
'alias': 'node-a',
'bitmaps': [
{
'name': 'bmap-a',
'alias': 'bmap-a',
'transform':
{
'persistent': True
}
},
{
'name': 'bmap-b',
'alias': 'bmap-b'
}
]
},
{
'node-name': 'node-b',
'alias': 'node-b',
'bitmaps': [
{
'name': 'bmap-a',
'alias': 'bmap-a'
},
{
'name': 'bmap-b',
'alias': 'bmap-b'
}
]
}
]
def verify_dest_bitmap_state(self) -> None:
bitmaps = self.vm_b.query_bitmaps()
for node in bitmaps:
bitmaps[node] = sorted(((bmap['name'], bmap['persistent']) for bmap in bitmaps[node]))
self.assertEqual(bitmaps,
{'node-a': [('bmap-a', True), ('bmap-b', False)],
'node-b': [('bmap-a', False), ('bmap-b', False)]})
def test_transform_on_src(self) -> None:
self.set_mapping(self.vm_a, self.transform_mapping())
self.migrate()
self.verify_dest_bitmap_state()
self.verify_dest_error(None)
def test_transform_on_dst(self) -> None:
self.set_mapping(self.vm_b, self.transform_mapping())
self.migrate()
self.verify_dest_bitmap_state()
self.verify_dest_error(None)
if __name__ == '__main__':
iotests.main(supported_protocols=['file'])

4
tests/qemu-iotests/300.out
View File

@ -1,5 +1,5 @@
.....................................
.......................................
----------------------------------------------------------------------
Ran 37 tests
Ran 39 tests
OK

6
tests/test-bdrv-graph-mod.c
View File

@ -101,7 +101,7 @@ static BlockDriverState *pass_through_node(const char *name)
*/
static void test_update_perm_tree(void)
{
Error *local_err = NULL;
int ret;
BlockBackend *root = blk_new(qemu_get_aio_context(),
BLK_PERM_WRITE | BLK_PERM_CONSISTENT_READ,
@ -114,8 +114,8 @@ static void test_update_perm_tree(void)
bdrv_attach_child(filter, bs, "child", &child_of_bds,
BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY, &error_abort);
bdrv_append(filter, bs, &local_err);
error_free_or_abort(&local_err);
ret = bdrv_append(filter, bs, NULL);
g_assert_cmpint(ret, <, 0);
blk_unref(root);
}