[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 38/38] contrib/ivshmem-server: Print "not for pr
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 38/38] contrib/ivshmem-server: Print "not for production" warning |
Date: |
Mon, 07 Mar 2016 19:42:15 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Marc-André Lureau <address@hidden> writes:
> Hi
>
> On Mon, Feb 29, 2016 at 7:40 PM, Markus Armbruster <address@hidden> wrote:
>> The code is okay for illustrating how things work and for testing, but
>> its error handling make it unfit for production use. Print a warning
>> to protect the innocent.
>>
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>
> I guess David would do something about the problems you report. Tbh, I
That would be nice.
> don't think ivshmem-server is so bad wrt error handling.
ivshmem_server_send_one_msg() returns -1 on error with errno set. Okay.
ivshmem_server_send_initial_info() fails in turn.
ivshmem_server_handle_new_conn() handles this by closing the connection.
Okay, except for EAGAIN and EINTR.
All other callers ignore ivshmem_server_send_one_msg() failures. Not
okay.
Here's an example of how things could go haywire:
* The server handles connections one after the other. It makes the file
descriptors non-blocking.
* When a client connects, ivshmem-server sends 3 + N*V messages to the
new client, and V messages to each existing client, where N is the
number of existing clients, and V is the number of vectors. Of these,
only the 3 to the new client are checked for errors. The unchecked
messages transmit eventfds for interrupts in groups of V messages.
* With a sufficiently large N*V and a sluggish client, the server can
conceivably hit EAGAIN. When it happens, the server drops messages
silently.
* InterVM interrupts corresponding to dropped eventfds will be silently
dropped.
* If out a group of V messages any non-trailing messages get dropped,
the trailing ones get silently miswired to the wrong vector.
Good luck debugging this in the field!
A thorough review of error handling is called for. Since I can't do
that now, I'm adding the warning.
> Meanwhile:
> Reviewed-by: Marc-André Lureau <address@hidden>
Thanks!