[sheepdog] [PATCH v3] dog/vdi: bug fix for voting majority, add test case to test it
Hitoshi Mitake
mitake.hitoshi at gmail.com
Sun Apr 6 19:08:11 CEST 2014
Hi Ruoyu,
Thanks a lot for your fix. The content of this v3 patch looks good to
me. But there are some style problems as described in below.
At Fri, 4 Apr 2014 13:55:14 +0800,
Ruoyu wrote:
>
> There is a critical error in the function vote_majority_object.
> When all the three copies are different, the original logic still
> choose the last one as majority.
You must add your "Signed-off-by" signature here. It would be like
this:
Signed-off-by: Ruoyu <liangry at ucweb.com>
> ---
> dog/vdi.c | 21 ++++++++++++--
> tests/functional/091 | 72 ++++++++++++++++++++++++++++++++++++++++++++++++
> tests/functional/091.out | 57 ++++++++++++++++++++++++++++++++++++++
> tests/functional/group | 1 +
> 4 files changed, 148 insertions(+), 3 deletions(-)
> create mode 100755 tests/functional/091
> create mode 100644 tests/functional/091.out
We already have a test case for majority voting: 077. Could you add
the content of 091 to the existing 077?
>
> diff --git a/dog/vdi.c b/dog/vdi.c
> index 999d4be..a9b3c79 100644
> --- a/dog/vdi.c
> +++ b/dog/vdi.c
> @@ -1578,6 +1578,7 @@ static void vote_majority_object(struct vdi_check_info *info)
> int count = 0, nr_live_copies = 0;
> struct vdi_check_work *majority = NULL;
>
> + /* step 1 */
> for (int i = 0; i < info->nr_copies; i++) {
> struct vdi_check_work *vcw = &info->vcw[i];
>
> @@ -1594,14 +1595,28 @@ static void vote_majority_object(struct vdi_check_info *info)
> count--;
> }
>
> + /* step 2 */
> + if (count > 0 && count <= nr_live_copies / 2) {
> + count = 0;
> + for (int i = 0; i < info->nr_copies; i++) {
> + struct vdi_check_work *vcw = &info->vcw[i];
> +
> + if (!vcw->object_found)
> + continue;
> + if (!memcmp(majority->hash, vcw->hash,
> sizeof(vcw->hash)))
The above line is over 80 characters. It violates the coding rule of
sheepdog. You can check style of your patch with the
script/checkpatch.pl in the sheepdog source tree.
Thanks,
Hitoshi
More information about the sheepdog
mailing list