On Wed, Oct 02, 2013 at 04:20:36PM +0200, Peter Lieven wrote:
> if a raw device like an iscsi target or host device is used
> the current implementation makes a second call out to get
> the block status of bs->file. however, the raw driver already
> has called bdrv_get_block_status on bs->file.
>
> v4: use a flag to detect the raw driver instead of the strncmp
> hack.
>
> Signed-off-by: Peter Lieven <p...@kamp.de>
> ---
> block.c | 4 ++--
> block/raw_bsd.c | 6 +++++-
> include/block/block.h | 3 +++
> 3 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/block.c b/block.c
> index 93e113a..7fa2e43 100644
> --- a/block.c
> +++ b/block.c
> @@ -3161,7 +3161,7 @@ static int64_t coroutine_fn
> bdrv_co_get_block_status(BlockDriverState *bs,
>
> if (bs->file &&
> (ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO) &&
> - (ret & BDRV_BLOCK_OFFSET_VALID)) {
> + (ret & BDRV_BLOCK_OFFSET_VALID) && !(ret & BDRV_BLOCK_RAW)) {
> ret2 = bdrv_co_get_block_status(bs->file, ret >> BDRV_SECTOR_BITS,
> *pnum, pnum);
> if (ret2 >= 0) {
> @@ -3172,7 +3172,7 @@ static int64_t coroutine_fn
> bdrv_co_get_block_status(BlockDriverState *bs,
> }
> }
>
> - return ret;
> + return ret & ~BDRV_BLOCK_RAW;
> }
>
> /* Coroutine wrapper for bdrv_get_block_status() */
> diff --git a/block/raw_bsd.c b/block/raw_bsd.c
> index d4ace60..a9e0209 100644
> --- a/block/raw_bsd.c
> +++ b/block/raw_bsd.c
> @@ -62,7 +62,11 @@ static int64_t coroutine_fn
> raw_co_get_block_status(BlockDriverState *bs,
> int64_t sector_num,
> int nb_sectors, int *pnum)
> {
> - return bdrv_get_block_status(bs->file, sector_num, nb_sectors, pnum);
> + int64_t ret = bdrv_get_block_status(bs->file, sector_num, nb_sectors,
> pnum);
> + if (ret < 0) {
> + return ret;
> + }
> + return ret | BDRV_BLOCK_RAW;
> }
>
> static int coroutine_fn raw_co_write_zeroes(BlockDriverState *bs,
> diff --git a/include/block/block.h b/include/block/block.h
> index f808550..cb7019b 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -84,6 +84,8 @@ typedef struct BlockDevOps {
> /* BDRV_BLOCK_DATA: data is read from bs->file or another file
> * BDRV_BLOCK_ZERO: sectors read as zero
> * BDRV_BLOCK_OFFSET_VALID: sector stored in bs->file as raw data
> + * BDRV_BLOCK_RAW: used internally to indicate that the request
> + * was piped through the raw driver
Sorry I didn't review this earlier but this flag looks hacky and I'm not
confident about merging the patch yet.
The patch makes me wonder if the raw_bsd driver should avoid calling
bs->file itself:
return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID |
(sector_num << BDRV_SECTOR_BITS);
Let block.c:bdrv_co_get_block_status() call down into bs->file.
The problem is then the protocol cannot report unallocated sectors with
this approach.
I think we want to preserve bs' offset while taking the other flags from
bs->file (DATA, ZERO).
Peter, Paolo: What do you think of this approach?
Stefan
