On Thu, 13 Dec 2018 at 10:19, Daniel P. Berrangé <berra...@redhat.com> wrote: > > On Wed, Dec 12, 2018 at 06:09:37PM -0800, Li Qiang wrote: > > Assert that the return value is not an error. This is like commit > > 7e6478e7d4f for qemu_set_cloexec. > > > > Signed-off-by: Li Qiang <liq...@gmail.com> > > --- > > 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); > > This leads to *awful* diagnostics. We need to print something > useful when it fails so we stand a chance of understanding what > is wrong.
It's the same thing we do in qemu_set_cloexec(), though, and nobody's complained about that that I know of. I think we need to understand whether we're getting asserts in vhost_user_test because of something silly like passing -1 as the fd, or because the fcntl() can legitimately fail. If the former, the assert isn't a big deal because when we hit it in newly developed code the problem is going to be obvious when run under a debugger. If the latter, we need to actually pass out the error status and fix all the callers to check it... thanks -- PMM