On Thu, Feb 02, 2017 at 11:59:22AM +0300, Denis V. Lunev wrote:
> On 02/01/2017 04:53 PM, Daniel P. Berrange wrote:
> > On Wed, Feb 01, 2017 at 04:15:28PM +0300, Denis V. Lunev wrote:
> >> From: Anton Nefedov <anton.nefe...@virtuozzo.com>
> >>
> >> Socket backend read handler should normally perform a disconnect, however
> >> the read handler may not get a chance to run if the frontend is not ready
> >> (qemu_chr_be_can_write == 0).
> >>
> >> This means that in virtio-serial frontend case if
> >> - the host has disconnected (giving EPIPE on socket write)
> >> - and the guest has disconnected (-> frontend not ready -> backend
> >> will not read)
> >> - and there is still data (frontend->backend) to flush (has to be a really
> >> tricky timing but nevertheless, we have observed the case in production)
> >>
> >> This results in virtio-serial trying to flush this data continiously
> >> forming
> >> a busy loop.
> >>
> >> Solution: react on EPIPE in the socket write handler. We must not
> >> disconnect
> >> right away though, there still may be data to read (see 4bf1cb0).
> >>
> >> Signed-off-by: Anton Nefedov <anton.nefe...@virtuozzo.com>
> >> Signed-off-by: Denis V. Lunev <d...@openvz.org>
> >> CC: Paolo Bonzini <pbonz...@redhat.com>
> >> CC: Daniel P. Berrange <berra...@redhat.com>
> >> ---
> >> qemu-char.c | 12 +++++++++++-
> >> 1 file changed, 11 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/qemu-char.c b/qemu-char.c
> >> index 6b4a299..f1f7a07 100644
> >> --- a/qemu-char.c
> >> +++ b/qemu-char.c
> >> @@ -1202,7 +1202,7 @@ static int io_channel_send_full(QIOChannel *ioc,
> >> errno = EAGAIN;
> >> return -1;
> >> } else if (ret < 0) {
> >> - errno = EINVAL;
> >> + errno = errno == EPIPE ? EPIPE : EINVAL;
> > You can't rely on 'errno' being valid after qio_channel_writev_full().
> >
> > The 'Error **errp' arg to that function is the only error reporting
> > mechanism that's supported. In particular since the TCP channel can
> > be wrapped in TLS, the errno can easily have been clobbered by time
> > you get here.
> can we return errno directly from this call and pass exit code to the upper
> layer? This will be fair and much more straightforward.
No, the qio APIs explicitly do *not* use errno because their implementations
may be calling APIs which in turn do not provide errnos. errno is only a
useful concept if your always calling into system calls. As soon as you need
to deal with higher level libraries, errno is woefully inadequate as a
concept, hence using Error ** instead.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
