Re: [Qemu-devel] [PATCH 1/2] glusterfs: fix max_discard

[Peter Lieven]

Am 02.02.2015 um 19:29 schrieb Denis V. Lunev:
> qemu_gluster_co_discard calculates size to discard as follows
>     size_t size = nb_sectors * BDRV_SECTOR_SIZE;
>     ret = glfs_discard_async(s->fd, offset, size, &gluster_finish_aiocb, acb);
>
> glfs_discard_async is declared as follows:
>   int glfs_discard_async (glfs_fd_t *fd, off_t length, size_t lent,
>                           glfs_io_cbk fn, void *data) __THROW
> This is problematic on i686 as sizeof(size_t) == 4.
>
> Set bl_max_discard to SIZE_MAX >> BDRV_SECTOR_BITS to avoid overflow
> on i386.
>
> Signed-off-by: Denis V. Lunev <address@hidden>
> CC: Kevin Wolf <address@hidden>
> CC: Peter Lieven <address@hidden>
> ---
>  block/gluster.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/block/gluster.c b/block/gluster.c
> index 1eb3a8c..47bf92d 100644
> --- a/block/gluster.c
> +++ b/block/gluster.c
> @@ -622,6 +622,13 @@ out:
>      return ret;
>  }
>  
> +static void qemu_gluster_refresh_limits(BlockDriverState *bs, Error **errp)
> +{
> +#if SIZE_MAX == UINT_MAX
> +    bs->bl.max_discard = SIZE_MAX >> BDRV_SECTOR_BITS;
> +#endif

I would write:

bs->bl.max_discard = MIN(SIZE_MAX >> BDRV_SECTOR_BITS, INT_MAX);

without the condition.


Peter



> +}
> +
>  #ifdef CONFIG_GLUSTERFS_DISCARD
>  static coroutine_fn int qemu_gluster_co_discard(BlockDriverState *bs,
>          int64_t sector_num, int nb_sectors)
> @@ -735,6 +742,7 @@ static BlockDriver bdrv_gluster = {
>  #ifdef CONFIG_GLUSTERFS_DISCARD
>      .bdrv_co_discard              = qemu_gluster_co_discard,
>  #endif
> +    .bdrv_refresh_limits          = qemu_gluster_refresh_limits,
>  #ifdef CONFIG_GLUSTERFS_ZEROFILL
>      .bdrv_co_write_zeroes         = qemu_gluster_co_write_zeroes,
>  #endif
> @@ -762,6 +770,7 @@ static BlockDriver bdrv_gluster_tcp = {
>  #ifdef CONFIG_GLUSTERFS_DISCARD
>      .bdrv_co_discard              = qemu_gluster_co_discard,
>  #endif
> +    .bdrv_refresh_limits          = qemu_gluster_refresh_limits,
>  #ifdef CONFIG_GLUSTERFS_ZEROFILL
>      .bdrv_co_write_zeroes         = qemu_gluster_co_write_zeroes,
>  #endif
> @@ -789,6 +798,7 @@ static BlockDriver bdrv_gluster_unix = {
>  #ifdef CONFIG_GLUSTERFS_DISCARD
>      .bdrv_co_discard              = qemu_gluster_co_discard,
>  #endif
> +    .bdrv_refresh_limits          = qemu_gluster_refresh_limits,
>  #ifdef CONFIG_GLUSTERFS_ZEROFILL
>      .bdrv_co_write_zeroes         = qemu_gluster_co_write_zeroes,
>  #endif
> @@ -816,6 +826,7 @@ static BlockDriver bdrv_gluster_rdma = {
>  #ifdef CONFIG_GLUSTERFS_DISCARD
>      .bdrv_co_discard              = qemu_gluster_co_discard,
>  #endif
> +    .bdrv_refresh_limits          = qemu_gluster_refresh_limits,
>  #ifdef CONFIG_GLUSTERFS_ZEROFILL
>      .bdrv_co_write_zeroes         = qemu_gluster_co_write_zeroes,
>  #endif