[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 10/22] quorum: Implement .bdrv_recurse_can_replace()
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [PATCH 10/22] quorum: Implement .bdrv_recurse_can_replace() |
Date: |
Wed, 25 Sep 2019 13:39:44 +0000 |
20.09.2019 18:27, Max Reitz wrote:
> Signed-off-by: Max Reitz <address@hidden>
> ---
> block/quorum.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 62 insertions(+)
>
> diff --git a/block/quorum.c b/block/quorum.c
> index 207054a64e..81b57dbae2 100644
> --- a/block/quorum.c
> +++ b/block/quorum.c
> @@ -825,6 +825,67 @@ static bool
> quorum_recurse_is_first_non_filter(BlockDriverState *bs,
> return false;
> }
>
> +static bool quorum_recurse_can_replace(BlockDriverState *bs,
> + BlockDriverState *to_replace)
> +{
> + BDRVQuorumState *s = bs->opaque;
> + int i;
> +
> + for (i = 0; i < s->num_children; i++) {
> + /*
> + * We have no idea whether our children show the same data as
> + * this node (@bs). It is actually highly likely that
> + * @to_replace does not, because replacing a broken child is
> + * one of the main use cases here.
> + *
> + * We do know that the new BDS will match @bs, so replacing
> + * any of our children by it will be safe. It cannot change
> + * the data this quorum node presents to its parents.
> + *
> + * However, replacing @to_replace by @bs in any of our
I'm a bit misleaded: by @bs, or by node equal to @bs (accordingly to
bdrv_recurse_can_replace spec)?
> + * children's chains may change visible data somewhere in
> + * there. We therefore cannot recurse down those chains with
> + * bdrv_recurse_can_replace().
> + * (More formally, bdrv_recurse_can_replace() requires that
> + * @to_replace will be replaced by something matching the @bs
> + * passed to it. We cannot guarantee that.)
Aha, and you answered already :) OK.
> + *
> + * Thus, we can only check whether any of our immediate
> + * children matches @to_replace.
> + *
> + * (In the future, we might add a function to recurse down a
> + * chain that checks that nothing there cares about a change
> + * in data from the respective child in question. For
> + * example, most filters do not care when their child's data
> + * suddenly changes, as long as their parents do not care.)
> + */
> + if (s->children[i].child->bs == to_replace) {
Hmm, still, what is wrong if we just put
bdrv_recurse_can_replace(s->children[i].child->bs, to_replace) into this if
condition?
(or may be more correct to call it after taking permissions)
> + Error *local_err = NULL;
> +
> + /*
> + * We now have to ensure that there is no other parent
> + * that cares about replacing this child by a node with
> + * potentially different data.
> + */
> + s->children[i].to_be_replaced = true;
> + bdrv_child_refresh_perms(bs, s->children[i].child, &local_err);
> +
> + /* Revert permissions */
> + s->children[i].to_be_replaced = false;
> + bdrv_child_refresh_perms(bs, s->children[i].child, &error_abort);
> +
> + if (local_err) {
> + error_free(local_err);
> + return false;
> + }
> +
> + return true;
> + }
> + }
> +
> + return false;
> +}
> +
> static int quorum_valid_threshold(int threshold, int num_children, Error
> **errp)
> {
>
> @@ -1195,6 +1256,7 @@ static BlockDriver bdrv_quorum = {
>
> .is_filter = true,
> .bdrv_recurse_is_first_non_filter =
> quorum_recurse_is_first_non_filter,
> + .bdrv_recurse_can_replace = quorum_recurse_can_replace,
>
> .strong_runtime_opts = quorum_strong_runtime_opts,
> };
>
--
Best regards,
Vladimir
- Re: [PATCH 08/22] quorum: Store children in own structure, (continued)
- [PATCH 07/22] blkverify: Implement .bdrv_recurse_can_replace(), Max Reitz, 2019/09/20
- [PATCH 11/22] block: Use bdrv_recurse_can_replace(), Max Reitz, 2019/09/20
- [PATCH 13/22] mirror: Double-check immediately before replacing, Max Reitz, 2019/09/20
- [PATCH 17/22] iotests: Add VM.assert_block_path(), Max Reitz, 2019/09/20
- [PATCH 10/22] quorum: Implement .bdrv_recurse_can_replace(), Max Reitz, 2019/09/20
- [PATCH 12/22] block: Remove bdrv_recurse_is_first_non_filter(), Max Reitz, 2019/09/20
- [PATCH 14/22] quorum: Stop marking it as a filter, Max Reitz, 2019/09/20
- [PATCH 15/22] mirror: Prevent loops, Max Reitz, 2019/09/20