On 07/02/2015 11:21 PM, Alberto Garcia wrote: > 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.
OK, I will do it. > >>> 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 Thanks for your explantion. Wen Congyang > > Berto > . >
