Re: [Qemu-devel] [PATCH v5 0/3] Quorum maintaince operations
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
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
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
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