Re: [PATCH v4 7/7] block: detect DKIOCGETBLOCKCOUNT/SIZE before use

[Max Reitz <mreitz@redhat.com>]

On 08.06.21 15:16, Paolo Bonzini wrote:
> From: Joelle van Dyne <j@getutm.app>
>
> iOS hosts do not have these defined so we fallback to the
> default behaviour.
>
> Co-authored-by: Warner Losh <imp@bsdimp.com>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Joelle van Dyne <j@getutm.app>
> Message-Id: <20210315180341.31638-4-j@getutm.app>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   block/file-posix.c | 21 +++++++++------------
>   1 file changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 5821e1afed..4e2f7cf508 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -2322,8 +2322,11 @@ static int64_t raw_getlength(BlockDriverState *bs)
>   again:
>   #endif
>       if (!fstat(fd, &sb) && (S_IFCHR & sb.st_mode)) {
> +        size = 0;
>   #ifdef DIOCGMEDIASIZE
> -        if (ioctl(fd, DIOCGMEDIASIZE, (off_t *)&size))
> +        if (ioctl(fd, DIOCGMEDIASIZE, (off_t *)&size)) {
> +            size = 0;
> +        }
>   #elif defined(DIOCGPART)
>           {
>                   struct partinfo pi;
> @@ -2332,9 +2335,7 @@ again:
>                   else
>                           size = 0;
>           }
> -        if (size == 0)
> -#endif
> -#if defined(__APPLE__) && defined(__MACH__)
> +#elif defined(DKIOCGETBLOCKCOUNT) && defined(DKIOCGETBLOCKSIZE)

In v3, I was wondering whether it’s intentional that the following 
DKIOCGETBLOCKCOUNT/SIZE block would no longer be used as a fallback if 
the DIOCGMEDIASIZE ioctl failed (which it was before this patch).  I’m 
still wondering.

Max

>           {
>               uint64_t sectors = 0;
>               uint32_t sector_size = 0;
> @@ -2342,19 +2343,15 @@ again:
>               if (ioctl(fd, DKIOCGETBLOCKCOUNT, &sectors) == 0
>                  && ioctl(fd, DKIOCGETBLOCKSIZE, &sector_size) == 0) {
>                   size = sectors * sector_size;
> -            } else {
> -                size = lseek(fd, 0LL, SEEK_END);
> -                if (size < 0) {
> -                    return -errno;
> -                }
>               }
>           }
> -#else
> -        size = lseek(fd, 0LL, SEEK_END);
> +#endif
> +        if (size == 0) {
> +            size = lseek(fd, 0LL, SEEK_END);
> +        }
>           if (size < 0) {
>               return -errno;
>           }
> -#endif
>   #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
>           switch(s->type) {
>           case FTYPE_CD: