[Qemu-devel] quorum: validate vote threshold against num_children even if read-pattern is fifo

[Qemu-devel] quorum: validate vote threshold against num_children even if read-pattern is fifo

[Wen Congyang]

We need to use threshold to check if too many write operation fails.
If threshold is larger than num children, we always get write error
event even if all write operations success.

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 block/quorum.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/block/quorum.c b/block/quorum.c
index a7df17c..b0eead0 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -894,6 +894,12 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags,
     }
 
     s->threshold = qemu_opt_get_number(opts, QUORUM_OPT_VOTE_THRESHOLD, 0);
+    /* and validate it against s->num_children */
+    ret = quorum_valid_threshold(s->threshold, s->num_children, &local_err);
+    if (ret < 0) {
+        goto exit;
+    }
+
     ret = parse_read_pattern(qemu_opt_get(opts, QUORUM_OPT_READ_PATTERN));
     if (ret < 0) {
         error_setg(&local_err, "Please set read-pattern as fifo or quorum");
@@ -902,12 +908,6 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags,
     s->read_pattern = ret;
 
     if (s->read_pattern == QUORUM_READ_PATTERN_QUORUM) {
-        /* and validate it against s->num_children */
-        ret = quorum_valid_threshold(s->threshold, s->num_children, &local_err);
-        if (ret < 0) {
-            goto exit;
-        }
-
         /* is the driver in blkverify mode */
         if (qemu_opt_get_bool(opts, QUORUM_OPT_BLKVERIFY, false) &&
             s->num_children == 2 && s->threshold == 2) {
-- 
2.4.3

Re: [Qemu-devel] quorum: validate vote threshold against num_children even if read-pattern is fifo

[Alberto Garcia]

On Fri 03 Jul 2015 08:45:06 AM CEST, Wen Congyang wrote:

> We need to use threshold to check if too many write operation fails.
> If threshold is larger than num children, we always get write error
> event even if all write operations success.
>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>  block/quorum.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/block/quorum.c b/block/quorum.c
> index a7df17c..b0eead0 100644
> --- a/block/quorum.c
> +++ b/block/quorum.c
> @@ -894,6 +894,12 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags,
>      }
>  
>      s->threshold = qemu_opt_get_number(opts, QUORUM_OPT_VOTE_THRESHOLD, 0);
> +    /* and validate it against s->num_children */
> +    ret = quorum_valid_threshold(s->threshold, s->num_children, &local_err);
> +    if (ret < 0) {
> +        goto exit;
> +    }
> +

I wonder why this check was moved inside the (s->read_pattern ==
QUORUM_READ_PATTERN_QUORUM) block when the fifo mode was introduced
(adding Liu Yuan to Cc).

I assume that you are not going to allow removing children under the
vote_threshold limit as we discussed, right?

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

Re: [Qemu-devel] quorum: validate vote threshold against num_children even if read-pattern is fifo

[Wen Congyang]

On 07/03/2015 04:41 PM, Alberto Garcia wrote:
> On Fri 03 Jul 2015 08:45:06 AM CEST, Wen Congyang wrote:
> 
>> We need to use threshold to check if too many write operation fails.
>> If threshold is larger than num children, we always get write error
>> event even if all write operations success.
>>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> ---
>>  block/quorum.c | 12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/block/quorum.c b/block/quorum.c
>> index a7df17c..b0eead0 100644
>> --- a/block/quorum.c
>> +++ b/block/quorum.c
>> @@ -894,6 +894,12 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags,
>>      }
>>  
>>      s->threshold = qemu_opt_get_number(opts, QUORUM_OPT_VOTE_THRESHOLD, 0);
>> +    /* and validate it against s->num_children */
>> +    ret = quorum_valid_threshold(s->threshold, s->num_children, &local_err);
>> +    if (ret < 0) {
>> +        goto exit;
>> +    }
>> +
> 
> I wonder why this check was moved inside the (s->read_pattern ==
> QUORUM_READ_PATTERN_QUORUM) block when the fifo mode was introduced
> (adding Liu Yuan to Cc).
> 
> I assume that you are not going to allow removing children under the
> vote_threshold limit as we discussed, right?

Yes.

Thanks
Wen Congyang

> 
> Reviewed-by: Alberto Garcia <berto@igalia.com>
> 
> Berto
> .
> 

Re: [Qemu-devel] quorum: validate vote threshold against num_children even if read-pattern is fifo

[Wen Congyang]

Ping...

On 07/03/2015 02:45 PM, Wen Congyang wrote:
> We need to use threshold to check if too many write operation fails.
> If threshold is larger than num children, we always get write error
> event even if all write operations success.
> 
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>  block/quorum.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/block/quorum.c b/block/quorum.c
> index a7df17c..b0eead0 100644
> --- a/block/quorum.c
> +++ b/block/quorum.c
> @@ -894,6 +894,12 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags,
>      }
>  
>      s->threshold = qemu_opt_get_number(opts, QUORUM_OPT_VOTE_THRESHOLD, 0);
> +    /* and validate it against s->num_children */
> +    ret = quorum_valid_threshold(s->threshold, s->num_children, &local_err);
> +    if (ret < 0) {
> +        goto exit;
> +    }
> +
>      ret = parse_read_pattern(qemu_opt_get(opts, QUORUM_OPT_READ_PATTERN));
>      if (ret < 0) {
>          error_setg(&local_err, "Please set read-pattern as fifo or quorum");
> @@ -902,12 +908,6 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags,
>      s->read_pattern = ret;
>  
>      if (s->read_pattern == QUORUM_READ_PATTERN_QUORUM) {
> -        /* and validate it against s->num_children */
> -        ret = quorum_valid_threshold(s->threshold, s->num_children, &local_err);
> -        if (ret < 0) {
> -            goto exit;
> -        }
> -
>          /* is the driver in blkverify mode */
>          if (qemu_opt_get_bool(opts, QUORUM_OPT_BLKVERIFY, false) &&
>              s->num_children == 2 && s->threshold == 2) {
> 

Re: [Qemu-devel] quorum: validate vote threshold against num_children even if read-pattern is fifo

[Kevin Wolf]

Am 19.08.2015 um 04:53 hat Wen Congyang geschrieben:
> Ping...

It might have helped to...

a) CC the qemu-block mailing list
b) CC the subsystem maintainer that should apply the patch (according to
   scripts/get_maintainer.pl that's me with Berto's Acked-by)
c) include the "PATCH" keyword in the subject line.

I'm copying qemu-block now and will consider the patch once I'm back
from KVM Forum.

Kevin


> On 07/03/2015 02:45 PM, Wen Congyang wrote:
> > We need to use threshold to check if too many write operation fails.
> > If threshold is larger than num children, we always get write error
> > event even if all write operations success.
> > 
> > Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> > ---
> >  block/quorum.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/block/quorum.c b/block/quorum.c
> > index a7df17c..b0eead0 100644
> > --- a/block/quorum.c
> > +++ b/block/quorum.c
> > @@ -894,6 +894,12 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags,
> >      }
> >  
> >      s->threshold = qemu_opt_get_number(opts, QUORUM_OPT_VOTE_THRESHOLD, 0);
> > +    /* and validate it against s->num_children */
> > +    ret = quorum_valid_threshold(s->threshold, s->num_children, &local_err);
> > +    if (ret < 0) {
> > +        goto exit;
> > +    }
> > +
> >      ret = parse_read_pattern(qemu_opt_get(opts, QUORUM_OPT_READ_PATTERN));
> >      if (ret < 0) {
> >          error_setg(&local_err, "Please set read-pattern as fifo or quorum");
> > @@ -902,12 +908,6 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags,
> >      s->read_pattern = ret;
> >  
> >      if (s->read_pattern == QUORUM_READ_PATTERN_QUORUM) {
> > -        /* and validate it against s->num_children */
> > -        ret = quorum_valid_threshold(s->threshold, s->num_children, &local_err);
> > -        if (ret < 0) {
> > -            goto exit;
> > -        }
> > -
> >          /* is the driver in blkverify mode */
> >          if (qemu_opt_get_bool(opts, QUORUM_OPT_BLKVERIFY, false) &&
> >              s->num_children == 2 && s->threshold == 2) {
> > 
> 
> 

Re: [Qemu-devel] quorum: validate vote threshold against num_children even if read-pattern is fifo

[Max Reitz]

On 03.07.2015 08:45, Wen Congyang wrote:
> We need to use threshold to check if too many write operation fails.
> If threshold is larger than num children, we always get write error
> event even if all write operations success.
> 
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>  block/quorum.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>

Re: [Qemu-devel] quorum: validate vote threshold against num_children even if read-pattern is fifo

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 474 bytes --]

On 03.07.2015 08:45, Wen Congyang wrote:
> We need to use threshold to check if too many write operation fails.
> If threshold is larger than num children, we always get write error
> event even if all write operations success.
> 
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>  block/quorum.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)

Thanks, applied to my block tree:

https://github.com/XanClic/qemu/commits/block


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]