On Thu, Feb 02, 2017 at 02:43:18PM +0300, Denis V. Lunev wrote:
> 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?
No, don't try to distinguish different types of errors. As mentioned
elsewhere just use the same logic for all errors, except when seeing
QIO_ERR_WOULD_BLOCK
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/ :|
