[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 2/2] block/quorum: add simple read pattern su
From: |
Liu Yuan |
Subject: |
Re: [Qemu-devel] [PATCH v4 2/2] block/quorum: add simple read pattern support |
Date: |
Fri, 15 Aug 2014 12:51:11 +0800 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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 <address@hidden>
> > Cc: Eric Blake <address@hidden>
> > Cc: Kevin Wolf <address@hidden>
> > Cc: Stefan Hajnoczi <address@hidden>
> > Signed-off-by: Liu Yuan <address@hidden>
> > ---
> > 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