[Ocfs2-tools-devel] [PATCH 1/1] O2image: A potential bug in o2image.

[Srinivas Eeda]

Yes, your understanding is correct and the fix looks good to me.

Signed-off-by: Srinivas Eeda <srinivas.eeda at oracle.com>

Tristan Ye wrote:
> Currently, we're using a scaling-down strategy to alloc memory chunk
> for o2image's bitmap blocks, the unerdyling alloc function which we
> were using(ocfs2_malloc_blocks) treat channel_io_blksz as basic unit.
> which may be smaller than our OCFS2_IMAGE_BITMAP_BLOCKSIZE(512 vs 4096).
>
> As a result, the problem is we're probably going to miss some blocks to allocate
> if the actual allocsize is not a multiple of OCFS2_IMAGE_BITMAP_BLOCKSIZE.
>
> Say we have 4096 * 17(bitmap blocks) to allocate, and failed at the first attemp,
> while did succeed after trying 'allocsize >>1', actually '2048 * 17' only fills up
> 8 image bitmap blocks, therefore, we get 16 image bitmap blocks allocated eventually.
> Now one block was missed.
>
> I guess it's very likely to hit if the number of bitmap blocks is odd, am I understanding
> incorrectly?
>
> Signed-off-by: Tristan Ye <tristan.ye at oracle.com>
> ---
>  libocfs2/image.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/libocfs2/image.c b/libocfs2/image.c
> index 0a1eeb8..05716c5 100644
> --- a/libocfs2/image.c
> +++ b/libocfs2/image.c
> @@ -122,6 +122,10 @@ errcode_t ocfs2_image_alloc_bitmap(ocfs2_filesys *ofs)
>  			if (allocsize == OCFS2_IMAGE_BITMAP_BLOCKSIZE)
>  				goto out;
>  			allocsize >>= 1;
> +			if (allocsize % OCFS2_IMAGE_BITMAP_BLOCKSIZE) {
> +				allocsize /= OCFS2_IMAGE_BITMAP_BLOCKSIZE;
> +				allocsize *= OCFS2_IMAGE_BITMAP_BLOCKSIZE
> +			}
>  			continue;
>  		}
>  
> @@ -138,6 +142,8 @@ errcode_t ocfs2_image_alloc_bitmap(ocfs2_filesys *ofs)
>  			indx++;
>  		}
>  		leftsize -= allocsize;
> +		if (leftsize <= allocsize);
> +			allocsize = leftsize;
>  	}
>  out:
>  	/* If allocation failed free and return error */
>