From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44376) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XI9UB-00020g-Lv for qemu-devel@nongnu.org; Fri, 15 Aug 2014 00:51:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XI9U3-00005D-2U for qemu-devel@nongnu.org; Fri, 15 Aug 2014 00:51:31 -0400 Received: from mail-pa0-x22f.google.com ([2607:f8b0:400e:c03::22f]:48475) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XI9U2-00004r-NI for qemu-devel@nongnu.org; Fri, 15 Aug 2014 00:51:22 -0400 Received: by mail-pa0-f47.google.com with SMTP id kx10so2885182pab.34 for ; Thu, 14 Aug 2014 21:51:21 -0700 (PDT) Date: Fri, 15 Aug 2014 12:51:11 +0800 From: Liu Yuan Message-ID: <20140815045111.GA12897@ubuntu-trusty> References: <1405574336-3794-1-git-send-email-namei.unix@gmail.com> <1405574336-3794-3-git-send-email-namei.unix@gmail.com> <20140814110931.GB2009@irqsave.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20140814110931.GB2009@irqsave.net> Subject: Re: [Qemu-devel] [PATCH v4 2/2] block/quorum: add simple read pattern support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?iso-8859-1?Q?Beno=EEt?= Canet Cc: Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi On Thu, Aug 14, 2014 at 01:09:32PM +0200, Benoît Canet wrote: > The Thursday 17 Jul 2014 à 13:18:56 (+0800), Liu Yuan wrote : > > This patch adds single read pattern to quorum driver and quorum vote is default > > pattern. > > > > For now we do a quorum vote on all the reads, it is designed for unreliable > > underlying storage such as non-redundant NFS to make sure data integrity at the > > cost of the read performance. > > > > For some use cases as following: > > > > VM > > -------------- > > | | > > v v > > A B > > > > Both A and B has hardware raid storage to justify the data integrity on its own. > > So it would help performance if we do a single read instead of on all the nodes. > > Further, if we run VM on either of the storage node, we can make a local read > > request for better performance. > > > > This patch generalize the above 2 nodes case in the N nodes. That is, > > > > vm -> write to all the N nodes, read just one of them. If single read fails, we > > try to read next node in FIFO order specified by the startup command. > > > > The 2 nodes case is very similar to DRBD[1] though lack of auto-sync > > functionality in the single device/node failure for now. But compared with DRBD > > we still have some advantages over it: > > > > - Suppose we have 20 VMs running on one(assume A) of 2 nodes' DRBD backed > > storage. And if A crashes, we need to restart all the VMs on node B. But for > > practice case, we can't because B might not have enough resources to setup 20 VMs > > at once. So if we run our 20 VMs with quorum driver, and scatter the replicated > > images over the data center, we can very likely restart 20 VMs without any > > resource problem. > > > > After all, I think we can build a more powerful replicated image functionality > > on quorum and block jobs(block mirror) to meet various High Availibility needs. > > > > E.g, Enable single read pattern on 2 children, > > > > -drive driver=quorum,children.0.file.filename=0.qcow2,\ > > children.1.file.filename=1.qcow2,read-pattern=fifo,vote-threshold=1 > > > > [1] http://en.wikipedia.org/wiki/Distributed_Replicated_Block_Device > > > > Cc: Benoit Canet > > Cc: Eric Blake > > Cc: Kevin Wolf > > Cc: Stefan Hajnoczi > > Signed-off-by: Liu Yuan > > --- > > block/quorum.c | 179 +++++++++++++++++++++++++++++++++++++++++---------------- > > 1 file changed, 131 insertions(+), 48 deletions(-) > > > > diff --git a/block/quorum.c b/block/quorum.c > > index d5ee9c0..ebf5c71 100644 > > --- a/block/quorum.c > > +++ b/block/quorum.c > > @@ -24,6 +24,7 @@ > > #define QUORUM_OPT_VOTE_THRESHOLD "vote-threshold" > > #define QUORUM_OPT_BLKVERIFY "blkverify" > > #define QUORUM_OPT_REWRITE "rewrite-corrupted" > > +#define QUORUM_OPT_READ_PATTERN "read-pattern" > > > > /* This union holds a vote hash value */ > > typedef union QuorumVoteValue { > > @@ -74,6 +75,8 @@ typedef struct BDRVQuorumState { > > bool rewrite_corrupted;/* true if the driver must rewrite-on-read corrupted > > * block if Quorum is reached. > > */ > > + > > + QuorumReadPattern read_pattern; > > } BDRVQuorumState; > > > > typedef struct QuorumAIOCB QuorumAIOCB; > > @@ -117,6 +120,7 @@ struct QuorumAIOCB { > > > > bool is_read; > > int vote_ret; > > + int child_iter; /* which child to read in fifo pattern */ > > }; > > > > static bool quorum_vote(QuorumAIOCB *acb); > > @@ -154,8 +158,10 @@ static void quorum_aio_finalize(QuorumAIOCB *acb) > > > > if (acb->is_read) { > > for (i = 0; i < s->num_children; i++) { > > - qemu_vfree(acb->qcrs[i].buf); > > - qemu_iovec_destroy(&acb->qcrs[i].qiov); > > + if (i <= acb->child_iter) { > > + qemu_vfree(acb->qcrs[i].buf); > > + qemu_iovec_destroy(&acb->qcrs[i].qiov); > > + } > > This seems convoluted. > > What about ? > /* on the quorum case acb->child_iter == s->num_children - 1 */ > for (i = 0; i <= acb->child_iter; i++) { > qemu_vfree(acb->qcrs[i].buf); > qemu_iovec_destroy(&acb->qcrs[i].qiov); > } > > > } Sounds good. I'll update the patch v5. Thanks Yuan