Am 14.06.2016 um 23:30 hat Eric Blake geschrieben:
> The raw block driver was blindly copying all limits from bs->file,
> even though: 1. the main bdrv_refresh_limits() already does this
> for many of gthe limits, and 2. blindly copying from the children
> can weaken any stricter limits that were already inherited from
> the backing dhain during the main bdrv_refresh_limits(). Also,
> the next patch is about to move .request_alignment into
> BlockLimits, and that is a limit that should NOT be copied from
> other layers in the BDS chain.
>
> Solve the issue by factoring out a new bdrv_merge_limits(),
> and using that function to properly merge limits when comparing
> two BlockDriverState objects.
>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
>
> ---
> v2: new patch
> ---
> include/block/block.h | 1 +
> include/block/block_int.h | 4 ++--
> include/qemu/typedefs.h | 1 +
> block/io.c | 31 +++++++++++++------------------
> block/raw_bsd.c | 4 ++--
> 5 files changed, 19 insertions(+), 22 deletions(-)
>
> diff --git a/include/block/block.h b/include/block/block.h
> index 733a8ec..c1d4648 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -262,6 +262,7 @@ int64_t bdrv_nb_sectors(BlockDriverState *bs);
> int64_t bdrv_getlength(BlockDriverState *bs);
> int64_t bdrv_get_allocated_file_size(BlockDriverState *bs);
> void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);
> +void bdrv_merge_limits(BlockLimits *dst, const BlockLimits *src);
> void bdrv_refresh_limits(BlockDriverState *bs, Error **errp);
> int bdrv_commit(BlockDriverState *bs);
> int bdrv_change_backing_file(BlockDriverState *bs,
Why does this need to be an external block layer interface? (block.h
instead of block_int.h)
Or in fact...
> diff --git a/block/raw_bsd.c b/block/raw_bsd.c
> index b1d5237..379ce8d 100644
> --- a/block/raw_bsd.c
> +++ b/block/raw_bsd.c
> @@ -152,7 +152,7 @@ static int raw_get_info(BlockDriverState *bs,
> BlockDriverInfo *bdi)
>
> static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
> {
> - bs->bl = bs->file->bs->bl;
> + bdrv_merge_limits(&bs->bl, &bs->file->bs->bl);
> }
...isn't this completely redundant because bdrv_refresh_limits() already
called bdrv_merge_limits(&bs->bl, &bs->file->bs->bl)? We could simply
remove the .bdrv_refresh_limits implementation from raw_bsd.
And then bdrv_merge_limits() could even be static (I think factoring it
out is a good idea anyway because it removes some duplication).
Kevin
