[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH for-5.0 v2 13/23] mirror: Double-check immediately before rep
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [PATCH for-5.0 v2 13/23] mirror: Double-check immediately before replacing |
Date: |
Fri, 29 Nov 2019 11:18:39 +0000 |
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 <address@hidden>
> ---
> 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 <address@hidden>
> + 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
- Re: [PATCH for-5.0 v2 08/23] quorum: Store children in own structure, (continued)
- [PATCH for-5.0 v2 09/23] quorum: Add QuorumChild.to_be_replaced, Max Reitz, 2019/11/11
- [PATCH for-5.0 v2 10/23] quorum: Implement .bdrv_recurse_can_replace(), Max Reitz, 2019/11/11
- [PATCH for-5.0 v2 11/23] block: Use bdrv_recurse_can_replace(), Max Reitz, 2019/11/11
- [PATCH for-5.0 v2 12/23] block: Remove bdrv_recurse_is_first_non_filter(), Max Reitz, 2019/11/11
- [PATCH for-5.0 v2 13/23] mirror: Double-check immediately before replacing, Max Reitz, 2019/11/11
- Re: [PATCH for-5.0 v2 13/23] mirror: Double-check immediately before replacing,
Vladimir Sementsov-Ogievskiy <=
- [PATCH for-5.0 v2 14/23] quorum: Stop marking it as a filter, Max Reitz, 2019/11/11
- [PATCH for-5.0 v2 16/23] iotests: Use complete_and_wait() in 155, Max Reitz, 2019/11/11
- [PATCH for-5.0 v2 15/23] mirror: Prevent loops, Max Reitz, 2019/11/11
- Re: [PATCH for-5.0 v2 15/23] mirror: Prevent loops, Vladimir Sementsov-Ogievskiy, 2019/11/29
- Re: [PATCH for-5.0 v2 15/23] mirror: Prevent loops, Max Reitz, 2019/11/29
- Re: [PATCH for-5.0 v2 15/23] mirror: Prevent loops, Vladimir Sementsov-Ogievskiy, 2019/11/29
- Re: [PATCH for-5.0 v2 15/23] mirror: Prevent loops, Max Reitz, 2019/11/29
- Re: [PATCH for-5.0 v2 15/23] mirror: Prevent loops, Vladimir Sementsov-Ogievskiy, 2019/11/29
- Re: [PATCH for-5.0 v2 15/23] mirror: Prevent loops, Max Reitz, 2019/11/29
[PATCH for-5.0 v2 19/23] iotests: Resolve TODOs in 041, Max Reitz, 2019/11/11