Re: [Qemu-devel] [PATCH 2/2] raw-posix: raw_co_get_block_status() return value

[Max Reitz <mreitz@redhat.com>]

On 22.09.2014 17:23, Max Reitz wrote:
> Instead of generating the full return value thrice in try_fiemap(),
> try_seek_hole() and as a fall-back in raw_co_get_block_status() itself,
> generate the value only in raw_co_get_block_status().
>
> Suggested-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>   block/raw-posix.c | 20 ++++++++++----------
>   1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index de4e3f3..7e1cc02 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -1455,12 +1455,12 @@ out:
>       return result;
>   }
>   
> -static int64_t try_fiemap(BlockDriverState *bs, off_t start, off_t *data,
> +static int try_fiemap(BlockDriverState *bs, off_t start, off_t *data,
>                             off_t *hole, int nb_sectors, int *pnum)

Self-NACK, I always love to see alignment issues only after I already 
sent the series...

Max

>   {
>   #ifdef CONFIG_FIEMAP
>       BDRVRawState *s = bs->opaque;
> -    int64_t ret = BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | start;
> +    int ret = 0;
>       struct {
>           struct fiemap fm;
>           struct fiemap_extent fe;
> @@ -1501,7 +1501,7 @@ static int64_t try_fiemap(BlockDriverState *bs, off_t start, off_t *data,
>   #endif
>   }
>   
> -static int64_t try_seek_hole(BlockDriverState *bs, off_t start, off_t *data,
> +static int try_seek_hole(BlockDriverState *bs, off_t start, off_t *data,
>                                off_t *hole, int *pnum)
>   {
>   #if defined SEEK_HOLE && defined SEEK_DATA
> @@ -1526,7 +1526,7 @@ static int64_t try_seek_hole(BlockDriverState *bs, off_t start, off_t *data,
>           }
>       }
>   
> -    return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | start;
> +    return 0;
>   #else
>       return -ENOTSUP;
>   #endif
> @@ -1552,7 +1552,7 @@ static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs,
>                                                       int nb_sectors, int *pnum)
>   {
>       off_t start, data = 0, hole = 0;
> -    int64_t ret;
> +    int ret;
>   
>       ret = fd_open(bs);
>       if (ret < 0) {
> @@ -1572,21 +1572,21 @@ static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs,
>               /* Assume everything is allocated. */
>               data = 0;
>               hole = start + nb_sectors * BDRV_SECTOR_SIZE;
> -            ret = BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | start;
> +            ret = 0;
>           }
>       }
>   
> +    assert(ret >= 0);
> +
>       if (data <= start) {
>           /* On a data extent, compute sectors to the end of the extent.  */
>           *pnum = MIN(nb_sectors, (hole - start) / BDRV_SECTOR_SIZE);
> +        return ret | BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | start;
>       } else {
>           /* On a hole, compute sectors to the beginning of the next extent.  */
>           *pnum = MIN(nb_sectors, (data - start) / BDRV_SECTOR_SIZE);
> -        ret &= ~BDRV_BLOCK_DATA;
> -        ret |= BDRV_BLOCK_ZERO;
> +        return ret | BDRV_BLOCK_ZERO | BDRV_BLOCK_OFFSET_VALID | start;
>       }
> -
> -    return ret;
>   }
>   
>   static coroutine_fn BlockDriverAIOCB *raw_aio_discard(BlockDriverState *bs,