From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52028) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agMcv-0002wY-Bm for qemu-devel@nongnu.org; Wed, 16 Mar 2016 21:21:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1agMcu-0007AJ-8e for qemu-devel@nongnu.org; Wed, 16 Mar 2016 21:21:25 -0400 References: <1457578181-27111-1-git-send-email-xiecl.fnst@cn.fujitsu.com> <1457578181-27111-3-git-send-email-xiecl.fnst@cn.fujitsu.com> <56E653E0.9030808@cn.fujitsu.com> From: Wen Congyang Message-ID: <56EA06E0.7000409@cn.fujitsu.com> Date: Thu, 17 Mar 2016 09:22:40 +0800 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v12 2/3] quorum: implement bdrv_add_child() and bdrv_del_child() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alberto Garcia , Changlong Xie , qemu devel , Eric Blake , Kevin Wolf , Max Reitz , Stefan Hajnoczi Cc: zhanghailiang , qemu block , Jiang Yunhong , Dong Eddie , Markus Armbruster , "Dr. David Alan Gilbert" , Gonglei On 03/16/2016 08:38 PM, Alberto Garcia wrote: > On Mon 14 Mar 2016 07:02:08 AM CET, Changlong Xie wrote: > >>>> @@ -81,6 +82,8 @@ typedef struct BDRVQuorumState { >>>> bool rewrite_corrupted;/* true if the driver must rewrite-on-read corrupted >>>> * block if Quorum is reached. >>>> */ >>>> + unsigned long *index_bitmap; >> >> Hi Berto >> >> *NOTE*, In the old version, we just used "bs->node_name", but in the >> lastest one, as Kevin suggested we introduce >> "child->child_name"(formart as "children.xxx"), this is the key cause >> why we need this two functions here. > > I'm sorry I missed this discussion earlier. Your code seems technically > correct but I have several questions: > > - I read that one of the reasons for this change is that "In theory, the > same node could be attached twice to the same parent in different > roles.". Is there any example of that? What's the use case? Kevin may know the case. > > - How do you obtain the child name? IIRC, the answer is no now. I think we can improve 'info block' output > > - I see that if you have children.0 and children.1 (let's say hd0.qcow2 > and hd1.qcow2), then you remove children.0 and add it again, it will > keep the 'children.0' name (that's what the bitmap is for if I'm > understanding it correctly). However the position in the s->children > array will change because you do memmove() when you remove children.0 > and then add it again to the end of the array. > > Initial status: > > s->children[0] <--> "children.0" (hd0.qcow2) > s->children[1] <--> "children.1" (hd1.qcow2) > > children.0 (hd0.qcow2) is removed: > > s->children[0] <--> "children.1" (hd1.qcow2) > > children.0 (hd0.qcow2) is added again: > > s->children[0] <--> "children.1" (hd1.qcow2) > s->children[1] <--> "children.0" (hd0.qcow2) Yes, it is correct. > > Is this correct? Is this the indented behavior? Since you are reading > in FIFO mode, now hd1.qcow2 will always be read first, so if > children.1 was the secondary disk, it has just become the primary. Yes. Thanks Wen Congyang > > I also think that it would be great to have tests for this > functionality, but they can be added later. > > Thanks, > > Berto > > > . >