Re: [Qemu-devel] [PATCH v6] block/raw-posix.c: Fixes raw_getlength() on Mac OS X so that it reports the correct length of a real CD

[Programmingkid]

On Jan 14, 2015, at 12:02 PM, Peter Maydell wrote:

> On 13 January 2015 at 20:07, Programmingkid <address@hidden> wrote:
>> Allows QEMU on Mac OS X to use a real cdrom again.
>> 
>> Signed-off-by: John Arbuckle <address@hidden>
>> 
>> ---
>> Added fallback code - uses lseek() if ioctl() fails.
>> 
>> block/raw-posix.c |   25 ++++++++++++++++++++++++-
>> 1 files changed, 24 insertions(+), 1 deletions(-)
>> 
>> diff --git a/block/raw-posix.c b/block/raw-posix.c
>> index e51293a..5815707 100644
>> --- a/block/raw-posix.c
>> +++ b/block/raw-posix.c
>> @@ -1312,7 +1312,30 @@ again:
>>         if (size == 0)
>> #endif
>> #if defined(__APPLE__) && defined(__MACH__)
>> -        size = LLONG_MAX;
>> +    {
>> +        uint64_t sectors = 0;
>> +        uint32_t sector_size = 0;
>> +        bool ioctl_problem = false;
>> +        ret = 0;
>> +
>> +        /* Query the number of sectors on the disk */
>> +        ret = ioctl(fd, DKIOCGETBLOCKCOUNT, &sectors);
>> +        if (ret != 0)
>> +            ioctl_problem = true;
>> +
>> +        /* Query the size of each sector */
>> +        ret = ioctl(fd, DKIOCGETBLOCKSIZE, &sector_size);
>> +        if (ret != 0)
>> +            ioctl_problem = true;
>> +
>> +        /* If everything is ok */
>> +        if (ioctl_problem == false)
>> +            size = sectors * sector_size;
>> +
>> +        /* If a problem occurred with ioctl(), fallback to lseek() */
>> +        else
>> +            size = lseek(fd, 0LL, SEEK_END);
>> +    }
>> #else
> 
> This fixes the "make check" problem, but you're not catching
> the possibility of lseek failing. (Also the if() statements
> are all missing braces our coding style requires.) You can
> avoid that bool flag like this:
> 
>    {
>        uint64_t sectors = 0;
>        uint32_t sector_size = 0;
> 
>        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;
>            }
>        }
>    }

Yeah, your code looks great. I will make a patch for it if you need me to.