Re: [Qemu-devel] [PATCH v5 0/3] Quorum maintaince operations

2014-05-31 Thread Benoît Canet
The Friday 30 May 2014 à 19:53:12 (+0200), Max Reitz wrote :
 On 30.05.2014 13:18, Benoît Canet wrote:
 These are the last bits required to make quorum usable in production.
 
 v5: rebase on latest Stefan's block branch [Kevin]
 
 v4:
  update patchset to stefan's block branch
  drop Max reviewed by because the series changes
 
 Benoît Canet (3):
quorum: Add the rewrite-corrupted parameter to quorum.
block: Add drive-mirror-replace command
qemu-iotests: Add 096 new test for drive-mirror-replace.
 
 Independently of this series, while looking at patch 1 again
 (although I had reviewed it before already), I noticed that
 quorum_get_winner() does not select the definite winner, but only a
 winner. So if you have a quorum instance with four children and a
 threshold of two, it may happen that there are basically two winners
 which both fulfill the threshold condition. In this case,
 quorum_get_winner() will just return the first winner. However, I
 think it should then return that there is no winner (i.e., NULL).
 
 On the other hand, the user should be aware that it may be a bad
 idea to choose a threshold which does not exceed half of the
 children count; thus, I'm just asking what you think about this. :-)

I think Quorum n/(2 *n) is a bad idea. (n + 1) / (2 *n) is a least required.

I wonder where we could put some documentation about this for the user.

Best regards

Benoît

 
 Max
 



Re: [Qemu-devel] [PATCH v5 0/3] Quorum maintaince operations

2014-05-31 Thread Max Reitz

On 31.05.2014 20:45, Benoît Canet wrote:

The Friday 30 May 2014 à 19:53:12 (+0200), Max Reitz wrote :

On 30.05.2014 13:18, Benoît Canet wrote:

These are the last bits required to make quorum usable in production.

v5: rebase on latest Stefan's block branch [Kevin]

v4:
 update patchset to stefan's block branch
 drop Max reviewed by because the series changes

Benoît Canet (3):
   quorum: Add the rewrite-corrupted parameter to quorum.
   block: Add drive-mirror-replace command
   qemu-iotests: Add 096 new test for drive-mirror-replace.

Independently of this series, while looking at patch 1 again
(although I had reviewed it before already), I noticed that
quorum_get_winner() does not select the definite winner, but only a
winner. So if you have a quorum instance with four children and a
threshold of two, it may happen that there are basically two winners
which both fulfill the threshold condition. In this case,
quorum_get_winner() will just return the first winner. However, I
think it should then return that there is no winner (i.e., NULL).

On the other hand, the user should be aware that it may be a bad
idea to choose a threshold which does not exceed half of the
children count; thus, I'm just asking what you think about this. :-)

I think Quorum n/(2 *n) is a bad idea. (n + 1) / (2 *n) is a least required.

I wonder where we could put some documentation about this for the user.


Well, quorum_valid_threshold() exists and could check this… Maybe it 
could at least emit a warning?


Max


Best regards

Benoît


Max






Re: [Qemu-devel] [PATCH v5 0/3] Quorum maintaince operations

2014-05-31 Thread Benoît Canet
The Saturday 31 May 2014 à 20:47:47 (+0200), Max Reitz wrote :
 On 31.05.2014 20:45, Benoît Canet wrote:
 The Friday 30 May 2014 à 19:53:12 (+0200), Max Reitz wrote :
 On 30.05.2014 13:18, Benoît Canet wrote:
 These are the last bits required to make quorum usable in production.
 
 v5: rebase on latest Stefan's block branch [Kevin]
 
 v4:
  update patchset to stefan's block branch
  drop Max reviewed by because the series changes
 
 Benoît Canet (3):
quorum: Add the rewrite-corrupted parameter to quorum.
block: Add drive-mirror-replace command
qemu-iotests: Add 096 new test for drive-mirror-replace.
 Independently of this series, while looking at patch 1 again
 (although I had reviewed it before already), I noticed that
 quorum_get_winner() does not select the definite winner, but only a
 winner. So if you have a quorum instance with four children and a
 threshold of two, it may happen that there are basically two winners
 which both fulfill the threshold condition. In this case,
 quorum_get_winner() will just return the first winner. However, I
 think it should then return that there is no winner (i.e., NULL).
 
 On the other hand, the user should be aware that it may be a bad
 idea to choose a threshold which does not exceed half of the
 children count; thus, I'm just asking what you think about this. :-)
 I think Quorum n/(2 *n) is a bad idea. (n + 1) / (2 *n) is a least required.
 
 I wonder where we could put some documentation about this for the user.
 
 Well, quorum_valid_threshold() exists and could check this… Maybe it
 could at least emit a warning?
 
 Max

Thanks,

Good idea I will write a patch doing this.

Best regards

Benoît
 
 Best regards
 
 Benoît
 
 Max
 
 
 



Re: [Qemu-devel] [PATCH v5 0/3] Quorum maintaince operations

2014-05-30 Thread Max Reitz

On 30.05.2014 13:18, Benoît Canet wrote:

These are the last bits required to make quorum usable in production.

v5: rebase on latest Stefan's block branch [Kevin]

v4:
 update patchset to stefan's block branch
 drop Max reviewed by because the series changes

Benoît Canet (3):
   quorum: Add the rewrite-corrupted parameter to quorum.
   block: Add drive-mirror-replace command
   qemu-iotests: Add 096 new test for drive-mirror-replace.


Independently of this series, while looking at patch 1 again (although I 
had reviewed it before already), I noticed that quorum_get_winner() does 
not select the definite winner, but only a winner. So if you have a 
quorum instance with four children and a threshold of two, it may happen 
that there are basically two winners which both fulfill the threshold 
condition. In this case, quorum_get_winner() will just return the first 
winner. However, I think it should then return that there is no winner 
(i.e., NULL).


On the other hand, the user should be aware that it may be a bad idea to 
choose a threshold which does not exceed half of the children count; 
thus, I'm just asking what you think about this. :-)


Max