[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 28/36] block: add bdrv_set_backing_noperm() transaction ac

From: Vladimir Sementsov-Ogievskiy
Subject: Re: [PATCH v2 28/36] block: add bdrv_set_backing_noperm() transaction action
Date: Mon, 8 Feb 2021 12:34:19 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.0

05.02.2021 19:26, Kevin Wolf wrote:
Am 27.11.2020 um 15:45 hat Vladimir Sementsov-Ogievskiy geschrieben:
Split out no-perm part of bdrv_set_backing_hd() as a separate
transaction action. Note the in case of existing BdrvChild we reuse it,
not recreate, just to do less actions.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

   * 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,
-                         Error **errp)
+static int bdrv_set_backing_noperm(BlockDriverState *bs,
+                                   BlockDriverState *backing_bs,
+                                   GSList **tran, Error **errp)
-    bool update_inherits_from = bdrv_chain_contains(bs, backing_hd) &&
-        bdrv_inherits_from_recursive(backing_hd, bs);
+    int ret = 0;
+    bool update_inherits_from = bdrv_chain_contains(bs, backing_bs) &&
+        bdrv_inherits_from_recursive(backing_bs, bs);
+    GSList *attach_tran = NULL;
+    BdrvSetBackingNoPermState *s;
if (bdrv_is_backing_chain_frozen(bs, child_bs(bs->backing), errp)) {
-        return;
+        return -EPERM;
- if (backing_hd) {
-        bdrv_ref(backing_hd);
+    if (bs->backing && backing_bs) {
+        bdrv_replace_child_safe(bs->backing, backing_bs, tran);

The old code with separate bdrv_unref_child() and then
bdrv_attach_child() tried to make the AioContests of bs and backing_bs
compatible by moving one of the nodes if necessary.

bdrv_replace_child_safe() doesn't seem to do that, but it only asserts
that both nodes are already in the same context.

I see that iotest 245 doesn't crash, which I think it should if this
were broken, but where does the switch happen now?

Hmm. Seems on path "if (bs->backing && backing_bs) {" we really miss aio 
context handling. Probably 245 doesn't check this branch? Or if leaves different aio contexts in one 

Best regards,

reply via email to

[Prev in Thread] Current Thread [Next in Thread]