Re: [Qemu-devel] [PATCH 1/1] qemu_char: socket backend: disconnect on EPIPE

[Denis V. Lunev]

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 <address@hidden>
>>
>> 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 <address@hidden>
>> Signed-off-by: Denis V. Lunev <address@hidden>
>> CC: Paolo Bonzini <address@hidden>
>> CC: Daniel P. Berrange <address@hidden>
>> ---
>>  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.

In this case we will have to check ret directly below.

Den