diff options
Diffstat (limited to 'tests')
| -rwxr-xr-x | tests/qemu-iotests/245 | 2 | ||||
| -rw-r--r-- | tests/qemu-iotests/283.out | 2 | ||||
| -rw-r--r-- | tests/qemu-iotests/tests/qsd-jobs.out | 2 | ||||
| -rw-r--r-- | tests/unit/test-bdrv-drain.c | 2 | ||||
| -rw-r--r-- | tests/unit/test-bdrv-graph-mod.c | 209 |
5 files changed, 212 insertions, 5 deletions
diff --git a/tests/qemu-iotests/245 b/tests/qemu-iotests/245 index 11104b9208..fc5297e268 100755 --- a/tests/qemu-iotests/245 +++ b/tests/qemu-iotests/245 @@ -905,7 +905,7 @@ class TestBlockdevReopen(iotests.QMPTestCase): # We can't reopen hd1 to read-only, as block-stream requires it to be # read-write self.reopen(opts['backing'], {'read-only': True}, - "Cannot make block node read-only, there is a writer on it") + "Read-only block node 'hd1' cannot support read-write users") # We can't remove hd2 while the stream job is ongoing opts['backing']['backing'] = None diff --git a/tests/qemu-iotests/283.out b/tests/qemu-iotests/283.out index 37c35058ae..97e62a4c94 100644 --- a/tests/qemu-iotests/283.out +++ b/tests/qemu-iotests/283.out @@ -5,7 +5,7 @@ {"execute": "blockdev-add", "arguments": {"driver": "blkdebug", "image": "base", "node-name": "other", "take-child-perms": ["write"]}} {"return": {}} {"execute": "blockdev-backup", "arguments": {"device": "source", "sync": "full", "target": "target"}} -{"error": {"class": "GenericError", "desc": "Cannot set permissions for backup-top filter: Conflicts with use by other as 'image', which uses 'write' on base"}} +{"error": {"class": "GenericError", "desc": "Cannot append backup-top filter: Conflicts with use by source as 'image', which does not allow 'write' on base"}} === backup-top should be gone after job-finalize === diff --git a/tests/qemu-iotests/tests/qsd-jobs.out b/tests/qemu-iotests/tests/qsd-jobs.out index 5f41491e05..9f52255da8 100644 --- a/tests/qemu-iotests/tests/qsd-jobs.out +++ b/tests/qemu-iotests/tests/qsd-jobs.out @@ -16,7 +16,7 @@ QMP_VERSION {"return": {}} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "job0"}} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "null", "id": "job0"}} -{"error": {"class": "GenericError", "desc": "Conflicts with use by a block device as 'root', which uses 'write' on fmt_base"}} +{"error": {"class": "GenericError", "desc": "Conflicts with use by stream job 'job0' as 'intermediate node', which does not allow 'write' on fmt_base"}} {"return": {}} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_EXPORT_DELETED", "data": {"id": "export1"}} *** done diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c index 8a29e33e00..892f7f47d8 100644 --- a/tests/unit/test-bdrv-drain.c +++ b/tests/unit/test-bdrv-drain.c @@ -1478,7 +1478,6 @@ static void test_append_to_drained(void) g_assert_cmpint(base_s->drain_count, ==, 1); g_assert_cmpint(base->in_flight, ==, 0); - /* Takes ownership of overlay, so we don't have to unref it later */ bdrv_append(overlay, base, &error_abort); g_assert_cmpint(base->in_flight, ==, 0); g_assert_cmpint(overlay->in_flight, ==, 0); @@ -1495,6 +1494,7 @@ static void test_append_to_drained(void) g_assert_cmpint(overlay->quiesce_counter, ==, 0); g_assert_cmpint(overlay_s->drain_count, ==, 0); + bdrv_unref(overlay); bdrv_unref(base); blk_unref(blk); } diff --git a/tests/unit/test-bdrv-graph-mod.c b/tests/unit/test-bdrv-graph-mod.c index c4f7d16039..88f25c0cdb 100644 --- a/tests/unit/test-bdrv-graph-mod.c +++ b/tests/unit/test-bdrv-graph-mod.c @@ -1,7 +1,7 @@ /* * Block node graph modifications tests * - * Copyright (c) 2019 Virtuozzo International GmbH. All rights reserved. + * Copyright (c) 2019-2021 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 @@ -44,6 +44,21 @@ static BlockDriver bdrv_no_perm = { .bdrv_child_perm = no_perm_default_perms, }; +static void exclusive_write_perms(BlockDriverState *bs, BdrvChild *c, + BdrvChildRole role, + BlockReopenQueue *reopen_queue, + uint64_t perm, uint64_t shared, + uint64_t *nperm, uint64_t *nshared) +{ + *nperm = BLK_PERM_WRITE; + *nshared = BLK_PERM_ALL & ~BLK_PERM_WRITE; +} + +static BlockDriver bdrv_exclusive_writer = { + .format_name = "exclusive-writer", + .bdrv_child_perm = exclusive_write_perms, +}; + static BlockDriverState *no_perm_node(const char *name) { return bdrv_new_open_driver(&bdrv_no_perm, name, BDRV_O_RDWR, &error_abort); @@ -55,6 +70,12 @@ static BlockDriverState *pass_through_node(const char *name) BDRV_O_RDWR, &error_abort); } +static BlockDriverState *exclusive_writer_node(const char *name) +{ + return bdrv_new_open_driver(&bdrv_exclusive_writer, name, + BDRV_O_RDWR, &error_abort); +} + /* * test_update_perm_tree * @@ -117,6 +138,7 @@ static void test_update_perm_tree(void) ret = bdrv_append(filter, bs, NULL); g_assert_cmpint(ret, <, 0); + bdrv_unref(filter); blk_unref(root); } @@ -181,10 +203,189 @@ static void test_should_update_child(void) bdrv_append(filter, bs, &error_abort); g_assert(target->backing->bs == bs); + bdrv_unref(filter); bdrv_unref(bs); blk_unref(root); } +/* + * test_parallel_exclusive_write + * + * Check that when we replace node, old permissions of the node being removed + * doesn't break the replacement. + */ +static void test_parallel_exclusive_write(void) +{ + BlockDriverState *top = exclusive_writer_node("top"); + BlockDriverState *base = no_perm_node("base"); + BlockDriverState *fl1 = pass_through_node("fl1"); + BlockDriverState *fl2 = pass_through_node("fl2"); + + /* + * bdrv_attach_child() eats child bs reference, so we need two @base + * references for two filters: + */ + bdrv_ref(base); + + bdrv_attach_child(top, fl1, "backing", &child_of_bds, BDRV_CHILD_DATA, + &error_abort); + bdrv_attach_child(fl1, base, "backing", &child_of_bds, BDRV_CHILD_FILTERED, + &error_abort); + bdrv_attach_child(fl2, base, "backing", &child_of_bds, BDRV_CHILD_FILTERED, + &error_abort); + + bdrv_replace_node(fl1, fl2, &error_abort); + + bdrv_unref(fl2); + bdrv_unref(top); +} + +static void write_to_file_perms(BlockDriverState *bs, BdrvChild *c, + BdrvChildRole role, + BlockReopenQueue *reopen_queue, + uint64_t perm, uint64_t shared, + uint64_t *nperm, uint64_t *nshared) +{ + if (bs->file && c == bs->file) { + *nperm = BLK_PERM_WRITE; + *nshared = BLK_PERM_ALL & ~BLK_PERM_WRITE; + } else { + *nperm = 0; + *nshared = BLK_PERM_ALL; + } +} + +static BlockDriver bdrv_write_to_file = { + .format_name = "tricky-perm", + .bdrv_child_perm = write_to_file_perms, +}; + + +/* + * The following test shows that topological-sort order is required for + * permission update, simple DFS is not enough. + * + * Consider the block driver which has two filter children: one active + * with exclusive write access and one inactive with no specific + * permissions. + * + * And, these two children has a common base child, like this: + * + * ┌─────┐ ┌──────┐ + * │ fl2 │ ◀── │ top │ + * └─────┘ └──────┘ + * │ │ + * │ │ w + * │ ▼ + * │ ┌──────┐ + * │ │ fl1 │ + * │ └──────┘ + * │ │ + * │ │ w + * │ ▼ + * │ ┌──────┐ + * └───────▶ │ base │ + * └──────┘ + * + * So, exclusive write is propagated. + * + * Assume, we want to make fl2 active instead of fl1. + * So, we set some option for top driver and do permission update. + * + * With simple DFS, if permission update goes first through + * top->fl1->base branch it will succeed: it firstly drop exclusive write + * permissions and than apply them for another BdrvChildren. + * But if permission update goes first through top->fl2->base branch it + * will fail, as when we try to update fl2->base child, old not yet + * updated fl1->base child will be in conflict. + * + * With topological-sort order we always update parents before children, so fl1 + * and fl2 are both updated when we update base and there is no conflict. + */ +static void test_parallel_perm_update(void) +{ + BlockDriverState *top = no_perm_node("top"); + BlockDriverState *tricky = + bdrv_new_open_driver(&bdrv_write_to_file, "tricky", BDRV_O_RDWR, + &error_abort); + BlockDriverState *base = no_perm_node("base"); + BlockDriverState *fl1 = pass_through_node("fl1"); + BlockDriverState *fl2 = pass_through_node("fl2"); + BdrvChild *c_fl1, *c_fl2; + + /* + * bdrv_attach_child() eats child bs reference, so we need two @base + * references for two filters: + */ + bdrv_ref(base); + + bdrv_attach_child(top, tricky, "file", &child_of_bds, BDRV_CHILD_DATA, + &error_abort); + c_fl1 = bdrv_attach_child(tricky, fl1, "first", &child_of_bds, + BDRV_CHILD_FILTERED, &error_abort); + c_fl2 = bdrv_attach_child(tricky, fl2, "second", &child_of_bds, + BDRV_CHILD_FILTERED, &error_abort); + bdrv_attach_child(fl1, base, "backing", &child_of_bds, BDRV_CHILD_FILTERED, + &error_abort); + bdrv_attach_child(fl2, base, "backing", &child_of_bds, BDRV_CHILD_FILTERED, + &error_abort); + + /* Select fl1 as first child to be active */ + tricky->file = c_fl1; + bdrv_child_refresh_perms(top, top->children.lh_first, &error_abort); + + assert(c_fl1->perm & BLK_PERM_WRITE); + assert(!(c_fl2->perm & BLK_PERM_WRITE)); + + /* Now, try to switch active child and update permissions */ + tricky->file = c_fl2; + bdrv_child_refresh_perms(top, top->children.lh_first, &error_abort); + + assert(c_fl2->perm & BLK_PERM_WRITE); + assert(!(c_fl1->perm & BLK_PERM_WRITE)); + + /* Switch once more, to not care about real child order in the list */ + tricky->file = c_fl1; + bdrv_child_refresh_perms(top, top->children.lh_first, &error_abort); + + assert(c_fl1->perm & BLK_PERM_WRITE); + assert(!(c_fl2->perm & BLK_PERM_WRITE)); + + bdrv_unref(top); +} + +/* + * It's possible that filter required permissions allows to insert it to backing + * chain, like: + * + * 1. [top] -> [filter] -> [base] + * + * but doesn't allow to add it as a branch: + * + * 2. [filter] --\ + * v + * [top] -> [base] + * + * So, inserting such filter should do all graph modifications and only then + * update permissions. If we try to go through intermediate state [2] and update + * permissions on it we'll fail. + * + * Let's check that bdrv_append() can append such a filter. + */ +static void test_append_greedy_filter(void) +{ + BlockDriverState *top = exclusive_writer_node("top"); + BlockDriverState *base = no_perm_node("base"); + BlockDriverState *fl = exclusive_writer_node("fl1"); + + bdrv_attach_child(top, base, "backing", &child_of_bds, BDRV_CHILD_COW, + &error_abort); + + bdrv_append(fl, base, &error_abort); + bdrv_unref(fl); + bdrv_unref(top); +} + int main(int argc, char *argv[]) { bdrv_init(); @@ -195,6 +396,12 @@ int main(int argc, char *argv[]) g_test_add_func("/bdrv-graph-mod/update-perm-tree", test_update_perm_tree); g_test_add_func("/bdrv-graph-mod/should-update-child", test_should_update_child); + g_test_add_func("/bdrv-graph-mod/parallel-perm-update", + test_parallel_perm_update); + g_test_add_func("/bdrv-graph-mod/parallel-exclusive-write", + test_parallel_exclusive_write); + g_test_add_func("/bdrv-graph-mod/append-greedy-filter", + test_append_greedy_filter); return g_test_run(); } |