11.11.2019 19:02, Max Reitz wrote:
> There is no guarantee that we can still replace the node we want to
> replace at the end of the mirror job. Double-check by calling
> bdrv_recurse_can_replace().
>
> Signed-off-by: Max Reitz <mre...@redhat.com>
> ---
> block/mirror.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/block/mirror.c b/block/mirror.c
> index f0f2d9dff1..68a4404666 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -695,7 +695,19 @@ static int mirror_exit_common(Job *job)
> * drain potential other users of the BDS before changing the
> graph. */
> assert(s->in_drain);
> bdrv_drained_begin(target_bs);
> - bdrv_replace_node(to_replace, target_bs, &local_err);
> + /*
> + * Cannot use check_to_replace_node() here, because that would
> + * check for an op blocker on @to_replace, and we have our own
> + * there.
> + */
interesting, that check_to_replace_node would acquire aio context of src..
Here we acquire aio context only if s->to_replace set (above this hunk).. Isn't
it a bug?
If it is, it's preexisting, and not directly related to the patch, so here:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
> + if (bdrv_recurse_can_replace(src, to_replace)) {
> + bdrv_replace_node(to_replace, target_bs, &local_err);
> + } else {
> + error_setg(&local_err, "Can no longer replace '%s' by '%s', "
> + "because it can no longer be guaranteed that doing so
> "
> + "would not lead to an abrupt change of visible data",
> + to_replace->node_name, target_bs->node_name);
> + }
> bdrv_drained_end(target_bs);
> if (local_err) {
> error_report_err(local_err);
>
--
Best regards,
Vladimir
