[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