diff mbox

[RFC,02/14] quorom: add a new read pattern

Message ID 1423710438-14377-3-git-send-email-wency@cn.fujitsu.com
State New
Headers show

Commit Message

Wen Congyang Feb. 12, 2015, 3:07 a.m. UTC
To block replication, we only need to read from the first child.

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Cc: Luiz Capitulino <lcapitulino@redhat.com>
Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 block/quorum.c       | 5 +++--
 qapi/block-core.json | 4 +++-
 2 files changed, 6 insertions(+), 3 deletions(-)

Comments

Gonglei (Arei) Feb. 12, 2015, 6:42 a.m. UTC | #1
On 2015/2/12 11:07, Wen Congyang wrote:
> To block replication, we only need to read from the first child.
> 
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> Cc: Luiz Capitulino <lcapitulino@redhat.com>
> Cc: Michael Roth <mdroth@linux.vnet.ibm.com>

A little typo in title of patch 2 and patch 5:
s/quorom/quorum/

Regards,
-Gonglei
Max Reitz Feb. 23, 2015, 8:36 p.m. UTC | #2
On 2015-02-11 at 22:07, Wen Congyang wrote:
> To block replication, we only need to read from the first child.
>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> Cc: Luiz Capitulino <lcapitulino@redhat.com>
> Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>   block/quorum.c       | 5 +++--
>   qapi/block-core.json | 4 +++-
>   2 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/block/quorum.c b/block/quorum.c
> index 437b122..5ed1ff8 100644
> --- a/block/quorum.c
> +++ b/block/quorum.c
> @@ -286,9 +286,10 @@ static void quorum_aio_cb(void *opaque, int ret)
>       BDRVQuorumState *s = acb->common.bs->opaque;
>       bool rewrite = false;
>   
> -    if (acb->is_read && s->read_pattern == QUORUM_READ_PATTERN_FIFO) {
> +    if (acb->is_read && s->read_pattern != QUORUM_READ_PATTERN_QUORUM) {

Maybe I'd prefer "&& (s->read_pattern == QUORUM_READ_PATTERN_FIFO || 
s->read_pattern == QUORUM_READ_PATTERN_FIRST)"; but it does fit with 
what quorum_aio_readv() does, so I'm fine with it.

>           /* We try to read next child in FIFO order if we fail to read */

However, I think this comment should be modified, because in fact we do 
not try to read the next child if s->read_pattern == 
QUORUM_READ_PATTERN_FIRST.

> -        if (ret < 0 && ++acb->child_iter < s->num_children) {
> +        if (s->read_pattern == QUORUM_READ_PATTERN_FIFO &&
> +            ret < 0 && ++acb->child_iter < s->num_children) {
>               read_fifo_child(acb);
>               return;
>           }
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index a3fdaf0..d6382e9 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1618,9 +1618,11 @@
>   #
>   # @fifo: read only from the first child that has not failed
>   #
> +# @first: read only from the first child

There should be version info here (like "(Since 2.3)").

Max

> +#
>   # Since: 2.2
>   ##
> -{ 'enum': 'QuorumReadPattern', 'data': [ 'quorum', 'fifo' ] }
> +{ 'enum': 'QuorumReadPattern', 'data': [ 'quorum', 'fifo', 'first' ] }
>   
>   ##
>   # @BlockdevOptionsQuorum
Eric Blake Feb. 23, 2015, 9:56 p.m. UTC | #3
On 02/11/2015 08:07 PM, Wen Congyang wrote:
> To block replication, we only need to read from the first child.

s/quorom/quorum/ in the subject line

s/To block/For block/

> 
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> Cc: Luiz Capitulino <lcapitulino@redhat.com>
> Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>  block/quorum.c       | 5 +++--
>  qapi/block-core.json | 4 +++-
>  2 files changed, 6 insertions(+), 3 deletions(-)

> +++ b/qapi/block-core.json
> @@ -1618,9 +1618,11 @@
>  #
>  # @fifo: read only from the first child that has not failed
>  #
> +# @first: read only from the first child
> +

Missing a 'since 2.3' designation.  Otherwise looks okay.
diff mbox

Patch

diff --git a/block/quorum.c b/block/quorum.c
index 437b122..5ed1ff8 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -286,9 +286,10 @@  static void quorum_aio_cb(void *opaque, int ret)
     BDRVQuorumState *s = acb->common.bs->opaque;
     bool rewrite = false;
 
-    if (acb->is_read && s->read_pattern == QUORUM_READ_PATTERN_FIFO) {
+    if (acb->is_read && s->read_pattern != QUORUM_READ_PATTERN_QUORUM) {
         /* We try to read next child in FIFO order if we fail to read */
-        if (ret < 0 && ++acb->child_iter < s->num_children) {
+        if (s->read_pattern == QUORUM_READ_PATTERN_FIFO &&
+            ret < 0 && ++acb->child_iter < s->num_children) {
             read_fifo_child(acb);
             return;
         }
diff --git a/qapi/block-core.json b/qapi/block-core.json
index a3fdaf0..d6382e9 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1618,9 +1618,11 @@ 
 #
 # @fifo: read only from the first child that has not failed
 #
+# @first: read only from the first child
+#
 # Since: 2.2
 ##
-{ 'enum': 'QuorumReadPattern', 'data': [ 'quorum', 'fifo' ] }
+{ 'enum': 'QuorumReadPattern', 'data': [ 'quorum', 'fifo', 'first' ] }
 
 ##
 # @BlockdevOptionsQuorum