Marc-André Lureau <marcandre.lur...@gmail.com> writes: > Hi > > On Mon, Feb 29, 2016 at 7:40 PM, Markus Armbruster <arm...@redhat.com> 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 <arm...@redhat.com> >> --- > > 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 <marcandre.lur...@redhat.com> Thanks!
