Re: [Qemu-devel] [PATCH v2] block/quorum: add simple read pattern support

[Liu Yuan]

On Mon, Jul 14, 2014 at 09:33:59PM -0600, Eric Blake wrote:
> On 07/14/2014 09:19 PM, Liu Yuan wrote:
> > This patch adds single read pattern to quorum driver and quorum vote is 
> > default
> > pattern.
> > 
> 
> > 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:
> > 
> 
> > +++ b/block.c
> > @@ -2212,7 +2212,7 @@ int bdrv_commit(BlockDriverState *bs)
> >  
> >      if (!drv)
> >          return -ENOMEDIUM;
> > -    
> > +
> >      if (!bs->backing_hd) {
> >          return -ENOTSUP;
> >      }
> 
> While this whitespace cleanup is nice, it doesn't belong in this patch,
> when there is no other change to this unrelated file.
> 
> 
> > +++ b/qapi/block-core.json
> > @@ -1398,12 +1398,17 @@
> >  # @rewrite-corrupted: #optional rewrite corrupted data when quorum is 
> > reached
> >  #                     (Since 2.1)
> >  #
> > +# @read-pattern: #optional choose quorum or fifo pattern for read
> > +#                set to quorum by default (Since 2.2)
> > +#
> >  # Since: 2.0
> >  ##
> >  { 'type': 'BlockdevOptionsQuorum',
> >    'data': { '*blkverify': 'bool',
> >              'children': [ 'BlockdevRef' ],
> > -            'vote-threshold': 'int', '*rewrite-corrupted': 'bool' } }
> > +            'vote-threshold': 'int',
> > +            '*rewrite-corrupted': 'bool',
> > +            '*read-pattern': 'str' } }
> 
> Raw strings that encode a finite set of values are bad for type-safety.
>  Please add an enum:
> 
> { 'enum': 'QuorumReadPattern', 'data': [ 'quorum', 'fifo' ] }
> 
> then use '*read-pattern': 'QuorumReadPattern'

For startup command line, did it imply a change too? Sorry I'm not familiar with
block-core.json.

With your suggestion, how we pass read-pattern via qmp?

 read-pattern: fifo
 or 
 read-pattern: quorum


> 
> Should we offer multiple modes in addition to 'quorum'?  For example, I
> could see a difference between 'fifo' (favor read from the first quorum
> member always, unless it fails, good when the first member is local and
> other member is remote) and 'round-robin' (evenly distribute reads; each
> read goes to the next available quorum member, good when all members are
> equally distant).

I guess so. 'round-robin' in your context would benefit use case seeking for
better read load balance.

Thanks
Yuan