Re: [Qemu-devel] [PATCH v6 2/4] quorum: implement bdrv_add_child() and bdrv_del_child()

[Alberto Garcia]

On Fri 16 Oct 2015 10:57:44 AM CEST, Wen Congyang wrote:
> +static void quorum_add_child(BlockDriverState *bs, BlockDriverState 
> *child_bs,
> +                             Error **errp)
> +{
> +    BDRVQuorumState *s = bs->opaque;
> +    BdrvChild *child;
> +
> +    bdrv_drain(bs);
> +
> +    assert(s->num_children <= INT_MAX / sizeof(BdrvChild *));
> +    if (s->num_children == INT_MAX / sizeof(BdrvChild *)) {
> +        error_setg(errp, "Too many children");
> +        return;
> +    }

This limit guarantees that s->num_children <= INT_MAX and that
s->num_children * sizeof(BdrvChild *) <= SIZE_MAX, right?

> +    s->children = g_renew(BdrvChild *, s->children, s->num_children + 1);
> +
> +    bdrv_ref(child_bs);
> +    child = bdrv_attach_child(bs, child_bs, &child_format);
> +    s->children[s->num_children++] = child;
> +}

Maybe you want to use ++s->num_children in the g_renew() call to make it
symmetric to the one in quorum_del_child()...

> +    /* We can safely remove this child now */
> +    memmove(&s->children[i], &s->children[i + 1],
> +            (s->num_children - i - 1) * sizeof(void *));
> +    s->children = g_renew(BdrvChild *, s->children, --s->num_children);
> +    bdrv_unref_child(bs, child);
> +}

But it's not really important, so you can leave it as it is now if you
prefer.

Reviewed-by: Alberto Garcia <address@hidden>

Berto