From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38428) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZApRm-0001TY-TO for qemu-devel@nongnu.org; Thu, 02 Jul 2015 21:07:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZApRa-0000om-9S for qemu-devel@nongnu.org; Thu, 02 Jul 2015 21:07:18 -0400 Message-ID: <5595E118.1090106@cn.fujitsu.com> Date: Fri, 3 Jul 2015 09:10:48 +0800 From: Wen Congyang MIME-Version: 1.0 References: <1435635285-5804-1-git-send-email-wency@cn.fujitsu.com> <1435635285-5804-3-git-send-email-wency@cn.fujitsu.com> <55954B1A.9030302@gmail.com> In-Reply-To: Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH COLO-BLOCK v7 02/17] quorum: implement block driver interfaces add/delete a BDS's child List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alberto Garcia , Wen Congyang , qemu devel , Fam Zheng , Max Reitz , Paolo Bonzini Cc: Kevin Wolf , Lai Jiangshan , qemu block , Jiang Yunhong , Dong Eddie , "Dr. David Alan Gilbert" , Gonglei , Stefan Hajnoczi , Yang Hongyang , zhanghailiang 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 > . >