[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH COLO-BLOCK v8 02/18] quorum: implement block dri
From: |
Wen Congyang |
Subject: |
Re: [Qemu-devel] [PATCH COLO-BLOCK v8 02/18] quorum: implement block driver interfaces add/delete a BDS's child |
Date: |
Thu, 6 Aug 2015 11:15:48 +0800 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 |
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 <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>
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
> .
>