On 02/02/2017 12:47 PM, Daniel P. Berrange wrote: > 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 But the problem is that Error does not have error code field inside. Should we play with ErrorClass enum?
Den