Re: [Qemu-devel] [PATCH] util: check the return value of fcntl in qemu_set_{block, noblock}

From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Li Qiang <liq3ea@gmail.com>
Cc: Peter Maydell <peter.maydell@linaro.org>,
	fam@euphon.net, marcandre lureau <marcandre.lureau@redhat.com>,
	Qemu Developers <qemu-devel@nongnu.org>,
	Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] util: check the return value of fcntl in qemu_set_{block, noblock}
Date: Thu, 13 Dec 2018 10:17:21 +0000	[thread overview]
Message-ID: <20181213101721.GB5171@redhat.com> (raw)
In-Reply-To: <CAKXe6SJdkTuBGD1x3Q3xa411KM=9MPB+-SB_yis3jD9u4nHbbg@mail.gmail.com>

On Thu, Dec 13, 2018 at 05:56:24PM +0800, Li Qiang wrote:
> Peter Maydell <peter.maydell@linaro.org> 于2018年12月13日周四 下午5:31写道：
> 
> > On Thu, 13 Dec 2018 at 06:58, <no-reply@patchew.org> wrote:
> > >
> > > Patchew URL:
> > https://patchew.org/QEMU/1544666977-4816-1-git-send-email-liq3ea@gmail.com/
> > >
> > >
> > >
> > > Hi,
> > >
> > > This series failed the docker-quick@centos7 build test. Please find the
> > testing commands and
> > > their output below. If you have Docker installed, you can probably
> > reproduce it
> > > locally.
> > >
> > > === TEST SCRIPT BEGIN ===
> > > #!/bin/bash
> > > time make docker-test-quick@centos7 SHOW_ENV=1 J=8
> > > === TEST SCRIPT END ===
> > >
> > > libpmem support   no
> > > libudev           no
> > >
> > > WARNING: Use of SDL 1.2 is deprecated and will be removed in
> > > WARNING: future releases. Please switch to using SDL 2.0
> > >
> > > NOTE: cross-compilers enabled:  'cc'
> > >   GEN     x86_64-softmmu/config-devices.mak.tmp
> > >
> > >
> > > The full log is available at
> > >
> > http://patchew.org/logs/1544666977-4816-1-git-send-email-liq3ea@gmail.com/testing.docker-quick@centos7/?type=message
> > .
> >
> > Patchew's attempt to limit the log to only the section with
> > the errors/warnings seems to have misfired here -- it looks
> > like it's picked the first bit of the log with a warning/error
> > rather than extracting all of them, which in this case happens
> > to be the harmless complaint that this build setup doesn't
> > have SDL2 installed.
> >
> > The actual cause of the failure is much lower down:
> >
> >
> Indeed.
> 
> 
> >   GTESTER check-qtest-aarch64
> > vhost-user-test: /tmp/qemu-test/src/util/oslib-posix.c:245:
> > qemu_set_nonblock: Assertion `f != -1' failed.
> >
> 
> So here means the fcntl call returns '-1'.
> Seems this test have some bugs?

Not neccessarily. It means that some code is caller qemu_set_nonblock
with a file descriptor for this which is not valid. You'll have to
debug what caller is triggering this to understand why

It might be as simple as something passing in an FD == -1, and indeed
I fear this is quite likely as we've been ignoring errors from
qemu_set_nonblock forever.   IOW, your change may well break existing
code that is in fact working just fine today.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|