Re: [Qemu-devel] [PATCH v4 1/5] block: add bdrv functions for geometry and blocksize

[Markus Armbruster <armbru@redhat.com>]

Ekaterina Tumanova <tumanova@linux.vnet.ibm.com> writes:

> Add driver functions for geometry and blocksize detection
>
> Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
> Reviewed-by: Thomas Huth <thuth@linux.vnet.ibm.com>
> ---
>  block.c                   | 34 ++++++++++++++++++++++++++++++++++
>  include/block/block.h     | 13 +++++++++++++
>  include/block/block_int.h | 15 +++++++++++++++
>  3 files changed, 62 insertions(+)
>
> diff --git a/block.c b/block.c
> index 4165d42..93409f5 100644
> --- a/block.c
> +++ b/block.c
> @@ -548,6 +548,40 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error **errp)
>      }
>  }
>  
> +/**
> + * Get @bs's logical and physical block size, store them in @bsz.
> + * @bs must not be empty.
> + */
> +void bdrv_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
> +{
> +    BlockDriver *drv = bs->drv;
> +
> +    assert(drv != NULL);
> +    if (drv->bdrv_probe_blocksizes &&
> +        !drv->bdrv_probe_blocksizes(bs, bsz)) {
> +            return;
> +    }
> +    bsz->log = BDRV_SECTOR_SIZE;
> +    bsz->phys = BDRV_SECTOR_SIZE;
> +}
> +
> +/**
> + * Try to get @bs's geometry (cyls, heads, sectos)

s/sectos/sectors/

Since we're changing this comment anyway: I like my function contracts
to state restrictions prominently, and therefore prefer "@bs must not be
empty" on its own line, right here.

> + * On success, store them in @geo struct and return 0.
> + * On failure return -errno. @bs must not be empty.
> + */
> +int bdrv_probe_geometry(BlockDriverState *bs, HDGeometry *geo)
> +{
> +    BlockDriver *drv = bs->drv;
> +
> +    assert(drv != NULL);
> +    if (drv->bdrv_probe_geometry) {
> +        return drv->bdrv_probe_geometry(bs, geo);
> +    }
> +
> +    return -ENOTSUP;
> +}
> +
>  /*
>   * Create a uniquely-named empty temporary file.
>   * Return 0 upon success, otherwise a negative errno value.
> diff --git a/include/block/block.h b/include/block/block.h
> index 6e7275d..14ac3b1 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -60,6 +60,17 @@ typedef enum {
>      BDRV_REQ_MAY_UNMAP    = 0x4,
>  } BdrvRequestFlags;
>  
> +typedef struct BlockSizes {
> +    uint32_t phys;
> +    uint32_t log;
> +} BlockSizes;
> +
> +typedef struct HDGeometry {
> +    uint32_t heads;
> +    uint32_t sectors;
> +    uint32_t cylinders;
> +} HDGeometry;
> +
>  #define BDRV_O_RDWR        0x0002
>  #define BDRV_O_SNAPSHOT    0x0008 /* open the file read only and save writes in a snapshot */
>  #define BDRV_O_TEMPORARY   0x0010 /* delete the file after use */
> @@ -539,6 +550,8 @@ AioContext *bdrv_get_aio_context(BlockDriverState *bs);
>   * the old #AioContext is not executing.
>   */
>  void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context);
> +void bdrv_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz);
> +int bdrv_probe_geometry(BlockDriverState *bs, HDGeometry *geo);
>  
>  void bdrv_io_plug(BlockDriverState *bs);
>  void bdrv_io_unplug(BlockDriverState *bs);
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 06a21dd..c2c5f0e 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -273,6 +273,21 @@ struct BlockDriver {
>      void (*bdrv_io_unplug)(BlockDriverState *bs);
>      void (*bdrv_flush_io_queue)(BlockDriverState *bs);
>  
> +    /**
> +     * Try to get @bs's logical and physical block size.
> +     * On success, store them in @bsz and return zero.
> +     * On failure, return negative errno.
> +     */
> +    int (*bdrv_probe_blocksizes)(BlockDriverState *bs, BlockSizes *bsz);
> +    /**
> +     * Try to get @bs's geometry (cyls, heads, sectos)

s/sectos/sectors/

> +     * On success, store them in @geo and return 0.
> +     * On failure return -errno.
> +     * Only drivers that want to override guest geometry implement this
> +     * callback; see hd_geometry_guess().
> +     */
> +    int (*bdrv_probe_geometry)(BlockDriverState *bs, HDGeometry *geo);
> +
>      QLIST_ENTRY(BlockDriver) list;
>  };