[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH v4 18/36] block: add bdrv_attach_child_common() transaction actio
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
[PATCH v4 18/36] block: add bdrv_attach_child_common() transaction action |
Date: |
Wed, 28 Apr 2021 18:17:46 +0300 |
Split out no-perm part of bdrv_root_attach_child() into separate
transaction action. bdrv_root_attach_child() now moves to new
permission update paradigm: first update graph relations then update
permissions.
qsd-jobs test output updated. Seems now permission update goes in
another order. Still, the test comment say that we only want to check
that command doesn't crash, and it's still so.
Error message is a bit misleading as it looks like job was added first.
But actually in new paradigm of graph update we can't distinguish such
things. We should update the error message, but let's not do it now.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block.c | 190 ++++++++++++++++++--------
tests/qemu-iotests/tests/qsd-jobs.out | 2 +-
2 files changed, 137 insertions(+), 55 deletions(-)
diff --git a/block.c b/block.c
index 6040b9dea0..4d5c60f2ae 100644
--- a/block.c
+++ b/block.c
@@ -2955,37 +2955,74 @@ static void bdrv_replace_child(BdrvChild *child,
BlockDriverState *new_bs)
}
}
-/*
- * This function steals the reference to child_bs from the caller.
- * That reference is later dropped by bdrv_root_unref_child().
- *
- * On failure NULL is returned, errp is set and the reference to
- * child_bs is also dropped.
- *
- * The caller must hold the AioContext lock @child_bs, but not that of @ctx
- * (unless @child_bs is already in @ctx).
- */
-BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
- const char *child_name,
- const BdrvChildClass *child_class,
- BdrvChildRole child_role,
- uint64_t perm, uint64_t shared_perm,
- void *opaque, Error **errp)
+static void bdrv_remove_empty_child(BdrvChild *child)
{
- BdrvChild *child;
- Error *local_err = NULL;
- int ret;
- AioContext *ctx;
+ assert(!child->bs);
+ QLIST_SAFE_REMOVE(child, next);
+ g_free(child->name);
+ g_free(child);
+}
- ret = bdrv_check_update_perm(child_bs, NULL, perm, shared_perm, NULL,
errp);
- if (ret < 0) {
- bdrv_abort_perm_update(child_bs);
- bdrv_unref(child_bs);
- return NULL;
+typedef struct BdrvAttachChildCommonState {
+ BdrvChild **child;
+ AioContext *old_parent_ctx;
+ AioContext *old_child_ctx;
+} BdrvAttachChildCommonState;
+
+static void bdrv_attach_child_common_abort(void *opaque)
+{
+ BdrvAttachChildCommonState *s = opaque;
+ BdrvChild *child = *s->child;
+ BlockDriverState *bs = child->bs;
+
+ bdrv_replace_child_noperm(child, NULL);
+
+ if (bdrv_get_aio_context(bs) != s->old_child_ctx) {
+ bdrv_try_set_aio_context(bs, s->old_child_ctx, &error_abort);
}
- child = g_new(BdrvChild, 1);
- *child = (BdrvChild) {
+ if (bdrv_child_get_parent_aio_context(child) != s->old_parent_ctx) {
+ GSList *ignore = g_slist_prepend(NULL, child);
+
+ child->klass->can_set_aio_ctx(child, s->old_parent_ctx, &ignore,
+ &error_abort);
+ g_slist_free(ignore);
+ ignore = g_slist_prepend(NULL, child);
+ child->klass->set_aio_ctx(child, s->old_parent_ctx, &ignore);
+
+ g_slist_free(ignore);
+ }
+
+ bdrv_unref(bs);
+ bdrv_remove_empty_child(child);
+ *s->child = NULL;
+}
+
+static TransactionActionDrv bdrv_attach_child_common_drv = {
+ .abort = bdrv_attach_child_common_abort,
+ .clean = g_free,
+};
+
+/*
+ * Common part of attaching bdrv child to bs or to blk or to job
+ */
+static int bdrv_attach_child_common(BlockDriverState *child_bs,
+ const char *child_name,
+ const BdrvChildClass *child_class,
+ BdrvChildRole child_role,
+ uint64_t perm, uint64_t shared_perm,
+ void *opaque, BdrvChild **child,
+ Transaction *tran, Error **errp)
+{
+ BdrvChild *new_child;
+ AioContext *parent_ctx;
+ AioContext *child_ctx = bdrv_get_aio_context(child_bs);
+
+ assert(child);
+ assert(*child == NULL);
+
+ new_child = g_new(BdrvChild, 1);
+ *new_child = (BdrvChild) {
.bs = NULL,
.name = g_strdup(child_name),
.klass = child_class,
@@ -2995,37 +3032,92 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState
*child_bs,
.opaque = opaque,
};
- ctx = bdrv_child_get_parent_aio_context(child);
-
- /* If the AioContexts don't match, first try to move the subtree of
+ /*
+ * If the AioContexts don't match, first try to move the subtree of
* child_bs into the AioContext of the new parent. If this doesn't work,
- * try moving the parent into the AioContext of child_bs instead. */
- if (bdrv_get_aio_context(child_bs) != ctx) {
- ret = bdrv_try_set_aio_context(child_bs, ctx, &local_err);
+ * try moving the parent into the AioContext of child_bs instead.
+ */
+ parent_ctx = bdrv_child_get_parent_aio_context(new_child);
+ if (child_ctx != parent_ctx) {
+ Error *local_err = NULL;
+ int ret = bdrv_try_set_aio_context(child_bs, parent_ctx, &local_err);
+
if (ret < 0 && child_class->can_set_aio_ctx) {
- GSList *ignore = g_slist_prepend(NULL, child);
- ctx = bdrv_get_aio_context(child_bs);
- if (child_class->can_set_aio_ctx(child, ctx, &ignore, NULL)) {
+ GSList *ignore = g_slist_prepend(NULL, new_child);
+ if (child_class->can_set_aio_ctx(new_child, child_ctx, &ignore,
+ NULL))
+ {
error_free(local_err);
ret = 0;
g_slist_free(ignore);
- ignore = g_slist_prepend(NULL, child);
- child_class->set_aio_ctx(child, ctx, &ignore);
+ ignore = g_slist_prepend(NULL, new_child);
+ child_class->set_aio_ctx(new_child, child_ctx, &ignore);
}
g_slist_free(ignore);
}
+
if (ret < 0) {
error_propagate(errp, local_err);
- g_free(child);
- bdrv_abort_perm_update(child_bs);
- bdrv_unref(child_bs);
- return NULL;
+ bdrv_remove_empty_child(new_child);
+ return ret;
}
}
- /* This performs the matching bdrv_set_perm() for the above check. */
- bdrv_replace_child(child, child_bs);
+ bdrv_ref(child_bs);
+ bdrv_replace_child_noperm(new_child, child_bs);
+ *child = new_child;
+
+ BdrvAttachChildCommonState *s = g_new(BdrvAttachChildCommonState, 1);
+ *s = (BdrvAttachChildCommonState) {
+ .child = child,
+ .old_parent_ctx = parent_ctx,
+ .old_child_ctx = child_ctx,
+ };
+ tran_add(tran, &bdrv_attach_child_common_drv, s);
+
+ return 0;
+}
+
+static void bdrv_detach_child(BdrvChild *child)
+{
+ bdrv_replace_child(child, NULL);
+ bdrv_remove_empty_child(child);
+}
+
+/*
+ * This function steals the reference to child_bs from the caller.
+ * That reference is later dropped by bdrv_root_unref_child().
+ *
+ * On failure NULL is returned, errp is set and the reference to
+ * child_bs is also dropped.
+ *
+ * The caller must hold the AioContext lock @child_bs, but not that of @ctx
+ * (unless @child_bs is already in @ctx).
+ */
+BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
+ const char *child_name,
+ const BdrvChildClass *child_class,
+ BdrvChildRole child_role,
+ uint64_t perm, uint64_t shared_perm,
+ void *opaque, Error **errp)
+{
+ int ret;
+ BdrvChild *child = NULL;
+ Transaction *tran = tran_new();
+
+ ret = bdrv_attach_child_common(child_bs, child_name, child_class,
+ child_role, perm, shared_perm, opaque,
+ &child, tran, errp);
+ if (ret < 0) {
+ bdrv_unref(child_bs);
+ return NULL;
+ }
+
+ ret = bdrv_refresh_perms(child_bs, errp);
+ tran_finalize(tran, ret);
+
+ bdrv_unref(child_bs);
return child;
}
@@ -3067,16 +3159,6 @@ BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,
return child;
}
-static void bdrv_detach_child(BdrvChild *child)
-{
- QLIST_SAFE_REMOVE(child, next);
-
- bdrv_replace_child(child, NULL);
-
- g_free(child->name);
- g_free(child);
-}
-
/* Callers must ensure that child->frozen is false. */
void bdrv_root_unref_child(BdrvChild *child)
{
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
--
2.29.2
- [PATCH v4 12/36] block: inline bdrv_child_*() permission functions calls, (continued)
- [PATCH v4 12/36] block: inline bdrv_child_*() permission functions calls, Vladimir Sementsov-Ogievskiy, 2021/04/28
- [PATCH v4 03/36] tests/test-bdrv-graph-mod: add test_append_greedy_filter, Vladimir Sementsov-Ogievskiy, 2021/04/28
- [PATCH v4 02/36] tests/test-bdrv-graph-mod: add test_parallel_perm_update, Vladimir Sementsov-Ogievskiy, 2021/04/28
- [PATCH v4 05/36] block: BdrvChildClass: add .get_parent_aio_context handler, Vladimir Sementsov-Ogievskiy, 2021/04/28
- [PATCH v4 14/36] block: add bdrv_drv_set_perm transaction action, Vladimir Sementsov-Ogievskiy, 2021/04/28
- [PATCH v4 06/36] block: drop ctx argument from bdrv_root_attach_child, Vladimir Sementsov-Ogievskiy, 2021/04/28
- [PATCH v4 16/36] block: add bdrv_replace_child_safe() transaction action, Vladimir Sementsov-Ogievskiy, 2021/04/28
- [PATCH v4 15/36] block: add bdrv_list_* permission update functions, Vladimir Sementsov-Ogievskiy, 2021/04/28
- [PATCH v4 08/36] util: add transactions.c, Vladimir Sementsov-Ogievskiy, 2021/04/28
- [PATCH v4 17/36] block: fix bdrv_replace_node_common, Vladimir Sementsov-Ogievskiy, 2021/04/28
- [PATCH v4 18/36] block: add bdrv_attach_child_common() transaction action,
Vladimir Sementsov-Ogievskiy <=
- [PATCH v4 04/36] block: bdrv_append(): don't consume reference, Vladimir Sementsov-Ogievskiy, 2021/04/28
- [PATCH v4 09/36] block: bdrv_refresh_perms: check for parents permissions conflict, Vladimir Sementsov-Ogievskiy, 2021/04/28
- [PATCH v4 07/36] block: make bdrv_reopen_{prepare, commit, abort} private, Vladimir Sementsov-Ogievskiy, 2021/04/28
- [PATCH v4 10/36] block: refactor bdrv_child* permission functions, Vladimir Sementsov-Ogievskiy, 2021/04/28
- [PATCH v4 11/36] block: rewrite bdrv_child_try_set_perm() using bdrv_refresh_perms(), Vladimir Sementsov-Ogievskiy, 2021/04/28
- [PATCH v4 21/36] block: adapt bdrv_append() for inserting filters, Vladimir Sementsov-Ogievskiy, 2021/04/28
- [PATCH v4 22/36] block: add bdrv_remove_filter_or_cow transaction action, Vladimir Sementsov-Ogievskiy, 2021/04/28
- [PATCH v4 19/36] block: add bdrv_attach_child_noperm() transaction action, Vladimir Sementsov-Ogievskiy, 2021/04/28
- [PATCH v4 31/36] block: drop unused permission update functions, Vladimir Sementsov-Ogievskiy, 2021/04/28
- [PATCH v4 24/36] block/backup-top: drop .active, Vladimir Sementsov-Ogievskiy, 2021/04/28