Re: [Qemu-devel] [PATCH COLO-BLOCK v8 02/18] quorum: implement block driver interfaces add/delete a BDS's child

[Alberto Garcia]

On Tue 07 Jul 2015 10:43:00 AM CEST, Wen Congyang wrote:
> Signed-off-by: Wen Congyang <address@hidden>
> Signed-off-by: zhanghailiang <address@hidden>
> Signed-off-by: Gonglei <address@hidden>
> Cc: Alberto Garcia <address@hidden>

Sorry for being so late with this, I was on holidays during July.

> +static void quorum_del_child(BlockDriverState *bs, BlockDriverState 
> *child_bs,
> +                             Error **errp)
> +{
> +    BDRVQuorumState *s = bs->opaque;
> +    int i;
> +
> +    for (i = 0; i < s->num_children; i++) {
> +        if (s->bs[i] == child_bs) {
> +            break;
> +        }
> +    }
> +
> +    if (i == s->num_children) {
> +        error_setg(errp, "Invalid child");
> +        return;
> +    }
> +
> +    if (s->num_children <= s->threshold) {
> +        error_setg(errp, "Cannot remove any more child");
> +        return;
> +    }

I think that should be "Cannot remove any more children".

You could also make it a bit more explanatory by saying "The number of
children cannot be lower than the vote threshold" or something like
that.

> +    bdrv_drain(bs);
> +    /* We can safe remove this child now */
> +    memmove(&s->bs[i], &s->bs[i+1], (s->num_children - i - 1) * sizeof(void 
> *));

s/safe/safely/

Apart from that, the code looks good to me, so

Reviewed-by: Alberto Garcia <address@hidden>

Berto