[v5,3/5] block: Add driver methods to probe blocksizes and geometry

Introduce driver methods of defining disk blocksizes (physical and
logical) and hard drive geometry.
Methods are only implemented for "host_device". For "raw" devices
driver calls child's method.

For now geometry detection will only work for DASD devices. To check
that a local check_for_dasd function was introduced. It calls BIODASDINFO2
ioctl and returns its rc.

Blocksizes detection function will probe sizes for DASD devices and
set default for other devices.

Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 block/raw-posix.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 block/raw_bsd.c   | 14 ++++++++
 2 files changed, 111 insertions(+)

On Thu, 18 Dec 2014 12:18:02 +0100
Ekaterina Tumanova <tumanova@linux.vnet.ibm.com> wrote:

> Introduce driver methods of defining disk blocksizes (physical and
> logical) and hard drive geometry.
> Methods are only implemented for "host_device". For "raw" devices
> driver calls child's method.
> 
> For now geometry detection will only work for DASD devices. To check
> that a local check_for_dasd function was introduced. It calls BIODASDINFO2
> ioctl and returns its rc.
> 
> Blocksizes detection function will probe sizes for DASD devices and
> set default for other devices.
> 
> Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> ---
>  block/raw-posix.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  block/raw_bsd.c   | 14 ++++++++
>  2 files changed, 111 insertions(+)
> 
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index 38172ca..1783acf 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -56,6 +56,7 @@
>  #include <linux/cdrom.h>
>  #include <linux/fd.h>
>  #include <linux/fs.h>
> +#include <linux/hdreg.h>
>  #ifndef FS_NOCOW_FL
>  #define FS_NOCOW_FL                     0x00800000 /* Do not cow file */
>  #endif
> @@ -90,6 +91,10 @@
>  #include <xfs/xfs.h>
>  #endif
> 
> +#ifdef __s390__
> +#include <asm/dasd.h>
> +#endif
> +
>  //#define DEBUG_FLOPPY
> 
>  //#define DEBUG_BLOCK
> @@ -238,6 +243,23 @@ static int probe_logical_blocksize(int fd, unsigned int *sector_size)
>  #undef SECTOR_SIZE
>  }
> 
> +/**
> + * Get physical block size of @fd.
> + * On success, store it in @blk_size and return 0.
> + * On failure, return -errno.
> + */
> +static int probe_physical_blocksize(int fd, unsigned int *blk_size)
> +{
> +#ifdef BLKPBSZGET
> +    if (ioctl(fd, BLKPBSZGET, blk_size) < 0) {
> +        return -errno;
> +    }
> +    return 0;
> +#else
> +    return -ENOTSUP;
> +#endif
> +}
> +
>  static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
>  {
>      BDRVRawState *s = bs->opaque;
> @@ -660,6 +682,79 @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
>      bs->bl.opt_mem_alignment = s->buf_align;
>  }
> 
> +static int check_for_dasd(int fd)
> +{
> +#ifdef BIODASDINFO2
> +    struct dasd_information2_t info = {0};
> +
> +    return ioctl(fd, BIODASDINFO2, &info);
> +#else
> +    return -1;
> +#endif
> +}
> +
> +/**
> + * Try to get @bs's logical and physical block size.
> + * On success, store them in @bsz and return zero.
> + * On failure, return negative errno.
> + */
> +static int hdev_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
> +{
> +    BDRVRawState *s = bs->opaque;
> +    int ret;
> +
> +    /* If DASD, get blocksizes */
> +    if (check_for_dasd(s->fd) < 0) {
> +        return -ENOTSUP;
> +    }
> +    ret = probe_logical_blocksize(s->fd, &bsz->log);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +    return probe_physical_blocksize(s->fd, &bsz->phys);
> +}
> +
> +/**
> + * Try to get @bs's geometry (cyls, heads, sectos)
> + * On success, store them in @geo and return 0.
> + * On failure return -errno.
> + * (as for 12/2014 only DASDs are supported)
> + */
> +static int hdev_probe_geometry(BlockDriverState *bs, HDGeometry *geo)
> +{
> +    BDRVRawState *s = bs->opaque;
> +    struct hd_geometry ioctl_geo = {0};
> +    uint32_t blksize;
> +
> +    /* If DASD, get it's geometry */

s/it's/its/

> +    if (check_for_dasd(s->fd) < 0) {
> +        return -ENOTSUP;
> +    }
> +    if (ioctl(s->fd, HDIO_GETGEO, &ioctl_geo) < 0) {
> +        return -errno;
> +    }
> +    /* HDIO_GETGEO may return success even though geo contains zeros
> +       (e.g. certain multipath setups) */
> +    if (!ioctl_geo.heads || !ioctl_geo.sectors || !ioctl_geo.cylinders) {
> +        return -ENOTSUP;
> +    }
> +    /* Do not return a geometry for partition */
> +    if (ioctl_geo.start != 0) {
> +        return -ENOTSUP;
> +    }
> +    geo->heads = ioctl_geo.heads;
> +    geo->sectors = ioctl_geo.sectors;
> +    if (!probe_physical_blocksize(s->fd, &blksize)) {
> +        /* overwrite cyls: HDIO_GETGEO result is incorrect for big drives */
> +        geo->cylinders = bs->total_sectors / (blksize / BDRV_SECTOR_SIZE)
> +                                           / (geo->heads * geo->sectors);
> +        return 0;
> +    }
> +    geo->cylinders = ioctl_geo.cylinders;
> +
> +    return 0;
> +}
> +
>  static ssize_t handle_aiocb_ioctl(RawPosixAIOData *aiocb)
>  {
>      int ret;
> @@ -2125,6 +2220,8 @@ static BlockDriver bdrv_host_device = {
>      .bdrv_get_info = raw_get_info,
>      .bdrv_get_allocated_file_size
>                          = raw_get_allocated_file_size,
> +    .bdrv_probe_blocksizes = hdev_probe_blocksizes,
> +    .bdrv_probe_geometry = hdev_probe_geometry,
> 
>      .bdrv_detach_aio_context = raw_detach_aio_context,
>      .bdrv_attach_aio_context = raw_attach_aio_context,
> diff --git a/block/raw_bsd.c b/block/raw_bsd.c
> index 05b02c7..f3d532b 100644
> --- a/block/raw_bsd.c
> +++ b/block/raw_bsd.c
> @@ -235,6 +235,18 @@ static int raw_probe(const uint8_t *buf, int buf_size, const char *filename)
>      return 1;
>  }
> 
> +static int raw_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
> +{
> +    bdrv_probe_blocksizes(bs->file, bsz);
> +
> +    return 0;
> +}
> +
> +static int raw_probe_geometry(BlockDriverState *bs, HDGeometry *geo)
> +{
> +    return bdrv_probe_geometry(bs->file, geo);
> +}
> +
>  BlockDriver bdrv_raw = {
>      .format_name          = "raw",
>      .bdrv_probe           = &raw_probe,
> @@ -252,6 +264,8 @@ BlockDriver bdrv_raw = {
>      .has_variable_length  = true,
>      .bdrv_get_info        = &raw_get_info,
>      .bdrv_refresh_limits  = &raw_refresh_limits,
> +    .bdrv_probe_blocksizes = &raw_probe_blocksizes,
> +    .bdrv_probe_geometry  = &raw_probe_geometry,
>      .bdrv_is_inserted     = &raw_is_inserted,
>      .bdrv_media_changed   = &raw_media_changed,
>      .bdrv_eject           = &raw_eject,

Looks good to me now (apart from the very minor typo).

Reviewed-by: Thomas Huth <thuth@linux.vnet.ibm.com>

On Thu, Dec 18, 2014 at 12:18:02PM +0100, Ekaterina Tumanova wrote:
> @@ -90,6 +91,10 @@
>  #include <xfs/xfs.h>
>  #endif
>  
> +#ifdef __s390__
> +#include <asm/dasd.h>
> +#endif

#if defined(__linux__) && defined(__s390__)

Or you could move it inside #ifdef __linux__ earlier in the file.

> +/**
> + * Try to get @bs's geometry (cyls, heads, sectos)
> + * On success, store them in @geo and return 0.
> + * On failure return -errno.
> + * (as for 12/2014 only DASDs are supported)

This comment isn't useful because it's apparent from the code and it
could get outdated.  It might be better to remove it and instead
document that this function allows the block driver to assign default
geometry values that the guest sees.

> + */
> +static int hdev_probe_geometry(BlockDriverState *bs, HDGeometry *geo)
> +{
> +    BDRVRawState *s = bs->opaque;
> +    struct hd_geometry ioctl_geo = {0};
> +    uint32_t blksize;
> +
> +    /* If DASD, get it's geometry */
> +    if (check_for_dasd(s->fd) < 0) {
> +        return -ENOTSUP;
> +    }
> +    if (ioctl(s->fd, HDIO_GETGEO, &ioctl_geo) < 0) {

This is a Linux ioctl, won't this break compilation on non-Linux hosts?

> +        return -errno;
> +    }
> +    /* HDIO_GETGEO may return success even though geo contains zeros
> +       (e.g. certain multipath setups) */
> +    if (!ioctl_geo.heads || !ioctl_geo.sectors || !ioctl_geo.cylinders) {
> +        return -ENOTSUP;
> +    }
> +    /* Do not return a geometry for partition */
> +    if (ioctl_geo.start != 0) {
> +        return -ENOTSUP;
> +    }
> +    geo->heads = ioctl_geo.heads;
> +    geo->sectors = ioctl_geo.sectors;
> +    if (!probe_physical_blocksize(s->fd, &blksize)) {
> +        /* overwrite cyls: HDIO_GETGEO result is incorrect for big drives */
> +        geo->cylinders = bs->total_sectors / (blksize / BDRV_SECTOR_SIZE)
> +                                           / (geo->heads * geo->sectors);

Please use bdrv_nb_sectors(bs) instead of bs->total_sectors.
bs->total_sectors is a cached value that is basically private to
block.c and should not be accessed directly.