Re: [Qemu-devel] [PULL v2 07/49] util: check the return value of fcntl in qemu_set_{block, nonblock}

[Kamil Rytarowski]

On 25.01.2019 19:53, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> On 1/15/19 9:04 PM, Michael S. Tsirkin wrote:
>> From: Li Qiang <address@hidden>
>>
>> Assert that the return value is not an error. This is like commit
>> 7e6478e7d4f for qemu_set_cloexec.
>>
>> Signed-off-by: Li Qiang <address@hidden>
>> Reviewed-by: Thomas Huth <address@hidden>
>> Reviewed-by: Michael S. Tsirkin <address@hidden>
>> Signed-off-by: Michael S. Tsirkin <address@hidden>
>> ---
>>  util/oslib-posix.c | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
>> index c1bee2a581..4ce1ba9ca4 100644
>> --- a/util/oslib-posix.c
>> +++ b/util/oslib-posix.c
>> @@ -233,14 +233,18 @@ void qemu_set_block(int fd)
>>  {
>>      int f;
>>      f = fcntl(fd, F_GETFL);
>> -    fcntl(fd, F_SETFL, f & ~O_NONBLOCK);
>> +    assert(f != -1);
>> +    f = fcntl(fd, F_SETFL, f & ~O_NONBLOCK);
>> +    assert(f != -1);
>>  }
>>  
>>  void qemu_set_nonblock(int fd)
>>  {
>>      int f;
>>      f = fcntl(fd, F_GETFL);
>> -    fcntl(fd, F_SETFL, f | O_NONBLOCK);
>> +    assert(f != -1);
>> +    f = fcntl(fd, F_SETFL, f | O_NONBLOCK);
>> +    assert(f != -1);
> 
> This commit breaks OpenBSD, when trying to start QEMU I get:
> assertion "f != -1" failed: file "util/oslib-posix.c", line 247,
> function "qemu_set_nonblock"
> 
> Having a quick look at gdb, the last device opened is /dev/null, and
> when fcntl() fails we have errno = ENODEV.
> 
>     19 ENODEV Operation not supported by device.
>     An attempt was made to apply an inappropriate function to a device,
>     for example, trying to read a write-only device such as a printer.
> 
> Digging further I found a recent commit which could fix this problem:
> https://github.com/openbsd/src/commit/c2a35b387f9d3c
> "fcntl(F_SETFL) invokes the FIONBIO and FIOASYNC ioctls internally, so
> the memory devices (/dev/null, /dev/zero, etc) need to permit them."
> 
> Brad: Do you think this might be the fix? If so, any idea what is the
> first release to contain this fix? I don't know OpenBSD and can't figure
> this out... Also, what would be the cleaner QEMU fix?
> 
> Thanks,
> 

I cannot speak for OpenBSD (never installed it myself), but if there is
a critical patch to test on NetBSD - please let me know.

> Phil.
> 
>>  }
>>  
>>  int socket_set_fast_reuse(int fd)
>>
>

signature.asc
Description: OpenPGP digital signature