On 08/05/2015 08:19 PM, Alberto Garcia wrote:
> On Tue 07 Jul 2015 10:43:00 AM CEST, Wen Congyang wrote:
>> Signed-off-by: Wen Congyang <we...@cn.fujitsu.com>
>> Signed-off-by: zhanghailiang <zhang.zhanghaili...@huawei.com>
>> Signed-off-by: Gonglei <arei.gong...@huawei.com>
>> Cc: Alberto Garcia <be...@igalia.com>
>
> 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 <be...@igalia.com>
Thanks, I have send add/delete a quorum child separately. This patch is changed
one line:
unref child_bs when it is removed.
I will update that patch and add your reviewed-by in that patchset.
Thanks
Wen Congyang
>
> Berto
> .
>
