Re: [Qemu-devel] [PATCH v3 2/5] raw-posix: Factor block size detection out of raw_probe_alignment()

[Markus Armbruster]

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

> Put it in new probe_logical_blocksize().
>
> Signed-off-by: Ekaterina Tumanova <tuman...@linux.vnet.ibm.com>
> ---
>  block/raw-posix.c | 44 +++++++++++++++++++++++++++-----------------
>  1 file changed, 27 insertions(+), 17 deletions(-)
>
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index b1af77e..633d5bc 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -217,11 +217,35 @@ static int raw_normalize_devicepath(const char 
> **filename)
>  }
>  #endif
>  
> +/*
> + * Set logical block size via ioctl. On success return 0. Otherwise -errno.

Set?  I suspect you mean get.

> + */
> +static int probe_logical_blocksize(int fd, unsigned int *sector_size)
> +{
> +    /* Try a few ioctls to get the right size */
> +#ifdef BLKSSZGET
> +    if (ioctl(fd, BLKSSZGET, sector_size) < 0) {
> +        return -errno;
> +    }
> +#endif
> +#ifdef DKIOCGETBLOCKSIZE
> +    if (ioctl(fd, DKIOCGETBLOCKSIZE, sector_size) < 0) {
> +        return -errno;
> +    }
> +#endif
> +#ifdef DIOCGSECTORSIZE
> +    if (ioctl(fd, DIOCGSECTORSIZE, sector_size) < 0) {
> +        return -errno;
> +    }
> +#endif
> +
> +    return 0;
> +}
> +
>  static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
>  {
>      BDRVRawState *s = bs->opaque;
>      char *buf;
> -    unsigned int sector_size;
>  
>      /* For /dev/sg devices the alignment is not really used.
>         With buffered I/O, we don't have any restrictions. */
> @@ -231,25 +255,11 @@ static void raw_probe_alignment(BlockDriverState *bs, 
> int fd, Error **errp)
>          return;
>      }
>  
> -    /* Try a few ioctls to get the right size */
>      bs->request_alignment = 0;
>      s->buf_align = 0;
> +    /* Let's try to use the logical blocksize for the alignment. */
> +    probe_logical_blocksize(fd, &bs->request_alignment);

I figure this works, but perhaps the following would be clearer:

       if (probe_logical_blocksize(fd, &bs->request_alignment) < 0) {
           bs->request_alignment = 0;
       }

Your call.

>  
> -#ifdef BLKSSZGET
> -    if (ioctl(fd, BLKSSZGET, &sector_size) >= 0) {
> -        bs->request_alignment = sector_size;
> -    }
> -#endif
> -#ifdef DKIOCGETBLOCKSIZE
> -    if (ioctl(fd, DKIOCGETBLOCKSIZE, &sector_size) >= 0) {
> -        bs->request_alignment = sector_size;
> -    }
> -#endif
> -#ifdef DIOCGSECTORSIZE
> -    if (ioctl(fd, DIOCGSECTORSIZE, &sector_size) >= 0) {
> -        bs->request_alignment = sector_size;
> -    }
> -#endif
>  #ifdef CONFIG_XFS
>      if (s->is_xfs) {
>          struct dioattr da;