[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 08/13] stream: Replace subtree drain with a single node drain
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [PATCH 08/13] stream: Replace subtree drain with a single node drain |
Date: |
Wed, 9 Nov 2022 19:52:48 +0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.2 |
On 11/8/22 15:37, Kevin Wolf wrote:
The subtree drain was introduced in commit b1e1af394d9 as a way to avoid
graph changes between finding the base node and changing the block graph
as necessary on completion of the image streaming job.
The block graph could change between these two points because
bdrv_set_backing_hd() first drains the parent node, which involved
polling and can do anything.
Subtree draining was an imperfect way to make this less likely (because
with it, fewer callbacks are called during this window). Everyone agreed
that it's not really the right solution, and it was only committed as a
stopgap solution.
This replaces the subtree drain with a solution that simply drains the
parent node before we try to find the base node, and then call a version
of bdrv_set_backing_hd() that doesn't drain, but just asserts that the
parent node is already drained.
This way, any graph changes caused by draining happen before we start
looking at the graph and things stay consistent between finding the base
node and changing the graph.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
[..]
base = bdrv_filter_or_cow_bs(s->above_base);
- if (base) {
- bdrv_ref(base);
- }
-
unfiltered_base = bdrv_skip_filters(base);
if (bdrv_cow_child(unfiltered_bs)) {
@@ -82,7 +85,7 @@ static int stream_prepare(Job *job)
}
}
- bdrv_set_backing_hd(unfiltered_bs, base, &local_err);
+ bdrv_set_backing_hd_drained(unfiltered_bs, base, &local_err);
ret = bdrv_change_backing_file(unfiltered_bs, base_id, base_fmt,
false);
If we have yield points / polls during bdrv_set_backing_hd_drained() and
bdrv_change_backing_file(), it's still bad and another graph-modifying
operation may interleave. But b1e1af394d9 reports only polling in
bdrv_set_backing_hd(), so I think it's OK to not care about other cases.
if (local_err) {
error_report_err(local_err);
@@ -92,10 +95,7 @@ static int stream_prepare(Job *job)
}
out:
- if (base) {
- bdrv_unref(base);
- }
- bdrv_subtree_drained_end(s->above_base);
+ bdrv_drained_end(unfiltered_bs);
return ret;
}
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
--
Best regards,
Vladimir
[PATCH 09/13] block: Remove subtree drains, Kevin Wolf, 2022/11/08
[PATCH 11/13] block: Remove ignore_bds_parents parameter from drain functions, Kevin Wolf, 2022/11/08