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

[Alberto Garcia <berto@igalia.com>]

On Thu 02 Jul 2015 04:30:50 PM CEST, Wen Congyang wrote:

>> 2) Did you think of any API to update vote_threshold? Currently it
>>     cannot be higher than num_children, so it's effectively limited by
>>     the number of children that are attached when the quorum device is
>>     created.
>
> The things I think about it is: if vote_threshold-- when deleting a
> child, vote_threshold will be less than 0. If we don't do
> vote_threshold-- when it is 1, the vote_threshold will be changed when
> all children are added again. Which behavior is better?

Adding/removing a child and changing vote_threshold are in principle two
unrelated operations. One user could want to modify one but not the
other, so linking them together does not seem like a good idea. A
specific API to change vote_threshold could be added when the use case
appears (currently no one seems to have missed it).

So I think I would keep these things separate, I would also remove the
"TODO" comments that mention it.

With the current patches (that do not touch vote_threshold), you can
remove as many children as you want as long as there's at least one
left. However if you end up with num_chilren < vote_threshold then you
will get read errors. I see two alternatives:

  - Allow that and expect that the user will add the necessary children
    afterwards.

  - Forbid that completely and return an error.

I think I prefer the second option.

>> 3) I don't think it's necessary to set to NULL the pointers in
>> s->bs[i] when i >= num_children. There's no way to access those
>> pointers anyway. Same for the ' s->bs[s->num_children] = NULL; ' bit
>> in quorum_del_child(). I also think that using memset() for setting
>> NULL pointers is not portable, although QEMU is already doing this in
>> a few places.
>
> OK, will remove it in the next version. Just a question: why is using
> memset() for setting NULL pointers is not prtable?

The standard allows for null pointers to be internally represented by
nonzero bit patterns. However I'm not aware of any system that we
support that does that.

   http://c-faq.com/null/confusion4.html
   http://c-faq.com/null/machexamp.html

Berto