[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL v2 07/49] util: check the return value of fcntl i
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PULL v2 07/49] util: check the return value of fcntl in qemu_set_{block, nonblock} |
Date: |
Fri, 25 Jan 2019 14:36:35 -0500 |
On Fri, Jan 25, 2019 at 02:04:15PM -0500, Brad Smith wrote:
> On 1/25/2019 1:53 PM, 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?
>
> I don't know. But that commit was included with 6.3 or newer.
I'm quite prepared to revert that - the value of spreading
asserts around is marginal and if we don't set fd
even on a real file to non-blocking qemu doesn't explode -
it just hangs which isn't much worse than an assert.
Let me know.
--
MST
- [Qemu-devel] [PULL v2 00/49] pci, pc, virtio: fixes, features, Michael S. Tsirkin, 2019/01/15
- [Qemu-devel] [PULL v2 14/49] tests: acpi: squash sanitize_fadt_ptrs() into test_acpi_fadt_table(), Michael S. Tsirkin, 2019/01/15
- [Qemu-devel] [PULL v2 06/49] vhost-user: fix ioeventfd_enabled, Michael S. Tsirkin, 2019/01/15
- [Qemu-devel] [PULL v2 05/49] tests: vhost-user-test: initialize 'fd' in chr_read, Michael S. Tsirkin, 2019/01/15
- [Qemu-devel] [PULL v2 13/49] tests: smbios: fetch whole table in one step instead of reading it step by step, Michael S. Tsirkin, 2019/01/15
- [Qemu-devel] [PULL v2 08/49] tests: acpi: use AcpiSdtTable::aml in consistent way, Michael S. Tsirkin, 2019/01/15
- [Qemu-devel] [PULL v2 09/49] tests: acpi: make sure FADT is fetched only once, Michael S. Tsirkin, 2019/01/15
- [Qemu-devel] [PULL v2 15/49] tests: acpi: use AcpiSdtTable::aml instead of AcpiSdtTable::header::signature, Michael S. Tsirkin, 2019/01/15
- [Qemu-devel] [PULL v2 12/49] tests: acpi: reuse fetch_table() in vmgenid-test, Michael S. Tsirkin, 2019/01/15
- [Qemu-devel] [PULL v2 11/49] tests: acpi: reuse fetch_table() for fetching FACS and DSDT, Michael S. Tsirkin, 2019/01/15
- [Qemu-devel] [PULL v2 10/49] tests: acpi: simplify rsdt handling, Michael S. Tsirkin, 2019/01/15