From: Markus Armbruster <armbru@redhat.com>
To: "Marc-André Lureau" <marcandre.lureau@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>, cam <cam@cs.ualberta.ca>,
Claudio Fontana <claudio.fontana@huawei.com>,
QEMU <qemu-devel@nongnu.org>,
David Marchand <david.marchand@6wind.com>
Subject: Re: [Qemu-devel] [PATCH 38/38] contrib/ivshmem-server: Print "not for production" warning
Date: Mon, 07 Mar 2016 19:42:15 +0100 [thread overview]
Message-ID: <87bn6qp1tk.fsf@blackfin.pond.sub.org> (raw)
In-Reply-To: <CAJ+F1C+6VaaTeL+h3n4yKr9CSF+tCdtzAL4wZ5q20Dgquz1_+w@mail.gmail.com> ("Marc-André Lureau"'s message of "Thu, 3 Mar 2016 15:15:21 +0100")
Marc-André Lureau <marcandre.lureau@gmail.com> writes:
> Hi
>
> On Mon, Feb 29, 2016 at 7:40 PM, Markus Armbruster <armbru@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 <armbru@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.lureau@redhat.com>
Thanks!
prev parent reply other threads:[~2016-03-07 18:42 UTC|newest]
Thread overview: 118+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-29 18:40 [Qemu-devel] [PATCH 00/38] ivshmem: Fixes, cleanups, device model split Markus Armbruster
2016-02-29 18:40 ` [Qemu-devel] [PATCH 01/38] exec: Fix memory allocation when memory path names new file Markus Armbruster
2016-03-01 11:35 ` Paolo Bonzini
2016-03-01 11:58 ` Markus Armbruster
2016-03-04 18:50 ` Markus Armbruster
2016-03-07 13:12 ` Paolo Bonzini
2016-02-29 18:40 ` [Qemu-devel] [PATCH 02/38] qemu-doc: Fix ivshmem huge page example Markus Armbruster
2016-03-01 10:51 ` Marc-André Lureau
2016-03-01 11:35 ` Paolo Bonzini
2016-02-29 18:40 ` [Qemu-devel] [PATCH 03/38] event_notifier: Make event_notifier_init_fd() #ifdef CONFIG_EVENTFD Markus Armbruster
2016-03-01 10:57 ` Marc-André Lureau
2016-03-01 12:00 ` Markus Armbruster
2016-03-01 12:05 ` Paolo Bonzini
2016-03-01 11:35 ` Paolo Bonzini
2016-02-29 18:40 ` [Qemu-devel] [PATCH 04/38] tests/libqos/pci-pc: Fix qpci_pc_iomap() to map BARs aligned Markus Armbruster
2016-03-01 11:05 ` Marc-André Lureau
2016-03-01 12:05 ` Markus Armbruster
2016-02-29 18:40 ` [Qemu-devel] [PATCH 05/38] ivshmem-test: Improve test case /ivshmem/single Markus Armbruster
2016-03-01 11:06 ` Marc-André Lureau
2016-02-29 18:40 ` [Qemu-devel] [PATCH 06/38] ivshmem-test: Clean up wait for devices to become operational Markus Armbruster
2016-03-01 11:10 ` Marc-André Lureau
2016-02-29 18:40 ` [Qemu-devel] [PATCH 07/38] ivshmem-test: Improve test cases /ivshmem/server-* Markus Armbruster
2016-03-01 11:13 ` Marc-André Lureau
2016-02-29 18:40 ` [Qemu-devel] [PATCH 08/38] ivshmem: Rewrite specification document Markus Armbruster
2016-03-01 11:25 ` Marc-André Lureau
2016-03-01 15:46 ` Eric Blake
2016-03-02 9:50 ` Markus Armbruster
2016-02-29 18:40 ` [Qemu-devel] [PATCH 09/38] ivshmem: Add missing newlines to debug printfs Markus Armbruster
2016-03-01 12:20 ` Marc-André Lureau
2016-02-29 18:40 ` [Qemu-devel] [PATCH 10/38] ivshmem: Compile debug prints unconditionally to prevent bit-rot Markus Armbruster
2016-03-01 12:22 ` Marc-André Lureau
2016-03-01 15:49 ` Eric Blake
2016-03-02 9:51 ` Markus Armbruster
2016-03-02 15:52 ` Eric Blake
2016-02-29 18:40 ` [Qemu-devel] [PATCH 11/38] ivshmem: Clean up after commit 9940c32 Markus Armbruster
2016-03-01 12:47 ` Marc-André Lureau
2016-02-29 18:40 ` [Qemu-devel] [PATCH 12/38] ivshmem: Drop ivshmem_event() stub Markus Armbruster
2016-03-01 12:48 ` Marc-André Lureau
2016-02-29 18:40 ` [Qemu-devel] [PATCH 13/38] ivshmem: Don't destroy the chardev on version mismatch Markus Armbruster
2016-03-01 15:39 ` Marc-André Lureau
2016-03-02 9:52 ` Markus Armbruster
2016-02-29 18:40 ` [Qemu-devel] [PATCH 14/38] ivshmem: Fix harmless misuse of Error Markus Armbruster
2016-03-01 15:47 ` Marc-André Lureau
2016-02-29 18:40 ` [Qemu-devel] [PATCH 15/38] ivshmem: Failed realize() can leave migration blocker behind Markus Armbruster
2016-03-01 15:59 ` Marc-André Lureau
2016-03-02 9:54 ` Markus Armbruster
2016-03-02 10:50 ` Marc-André Lureau
2016-02-29 18:40 ` [Qemu-devel] [PATCH 16/38] ivshmem: Clean up register callbacks Markus Armbruster
2016-03-01 16:04 ` Marc-André Lureau
2016-02-29 18:40 ` [Qemu-devel] [PATCH 17/38] ivshmem: Clean up MSI-X conditions Markus Armbruster
2016-03-01 16:57 ` Marc-André Lureau
2016-03-02 10:25 ` Markus Armbruster
2016-02-29 18:40 ` [Qemu-devel] [PATCH 18/38] ivshmem: Leave INTx alone when using MSI-X Markus Armbruster
2016-03-01 17:14 ` Marc-André Lureau
2016-03-01 17:30 ` Paolo Bonzini
2016-03-02 11:04 ` Markus Armbruster
2016-03-02 14:15 ` Paolo Bonzini
2016-03-02 15:50 ` Markus Armbruster
2016-02-29 18:40 ` [Qemu-devel] [PATCH 19/38] ivshmem: Assert interrupts are set up once Markus Armbruster
2016-03-02 12:02 ` Marc-André Lureau
2016-02-29 18:40 ` [Qemu-devel] [PATCH 20/38] ivshmem: Simplify rejection of invalid peer ID from server Markus Armbruster
2016-03-02 15:08 ` Marc-André Lureau
2016-02-29 18:40 ` [Qemu-devel] [PATCH 21/38] ivshmem: Disentangle ivshmem_read() Markus Armbruster
2016-03-02 15:28 ` Marc-André Lureau
2016-03-02 15:53 ` Markus Armbruster
2016-03-02 17:33 ` Marc-André Lureau
2016-03-02 19:15 ` Markus Armbruster
2016-02-29 18:40 ` [Qemu-devel] [PATCH 22/38] ivshmem: Plug leaks on unplug, fix peer disconnect Markus Armbruster
2016-03-02 17:47 ` Marc-André Lureau
2016-03-02 19:19 ` Markus Armbruster
2016-03-02 23:52 ` Marc-André Lureau
2016-02-29 18:40 ` [Qemu-devel] [PATCH 23/38] ivshmem: Receive shared memory synchronously in realize() Markus Armbruster
2016-03-02 18:11 ` Marc-André Lureau
2016-03-02 19:28 ` Markus Armbruster
2016-02-29 18:40 ` [Qemu-devel] [PATCH 24/38] ivshmem: Propagate errors through ivshmem_recv_setup() Markus Armbruster
2016-03-02 18:27 ` Marc-André Lureau
2016-03-02 19:35 ` Markus Armbruster
2016-03-03 0:03 ` Marc-André Lureau
2016-03-03 7:16 ` Markus Armbruster
2016-02-29 18:40 ` [Qemu-devel] [PATCH 25/38] ivshmem: Rely on server sending the ID right after the version Markus Armbruster
2016-03-02 18:36 ` Marc-André Lureau
2016-02-29 18:40 ` [Qemu-devel] [PATCH 26/38] ivshmem: Drop the hackish test for UNIX domain chardev Markus Armbruster
2016-03-02 18:38 ` Marc-André Lureau
2016-02-29 18:40 ` [Qemu-devel] [PATCH 27/38] ivshmem: Simplify how we cope with short reads from server Markus Armbruster
2016-03-02 18:41 ` Marc-André Lureau
2016-03-02 19:38 ` Markus Armbruster
2016-02-29 18:40 ` [Qemu-devel] [PATCH 28/38] ivshmem: Tighten check of property "size" Markus Armbruster
2016-03-02 18:44 ` Marc-André Lureau
2016-02-29 18:40 ` [Qemu-devel] [PATCH 29/38] ivshmem: Implement shm=... with a memory backend Markus Armbruster
2016-03-01 11:37 ` Paolo Bonzini
2016-03-01 12:08 ` Markus Armbruster
2016-02-29 18:40 ` [Qemu-devel] [PATCH 30/38] ivshmem: Simplify memory regions for BAR 2 (shared memory) Markus Armbruster
2016-03-01 11:42 ` Paolo Bonzini
2016-03-01 12:14 ` Markus Armbruster
2016-03-01 12:17 ` Paolo Bonzini
2016-03-01 11:46 ` Paolo Bonzini
2016-03-01 14:06 ` Markus Armbruster
2016-03-01 15:15 ` Paolo Bonzini
2016-03-02 11:06 ` Markus Armbruster
2016-02-29 18:40 ` [Qemu-devel] [PATCH 31/38] ivshmem: Inline check_shm_size() into its only caller Markus Armbruster
2016-03-02 18:49 ` Marc-André Lureau
2016-02-29 18:40 ` [Qemu-devel] [PATCH 32/38] qdev: New DEFINE_PROP_ON_OFF_AUTO Markus Armbruster
2016-03-02 18:54 ` Marc-André Lureau
2016-02-29 18:40 ` [Qemu-devel] [PATCH 33/38] ivshmem: Replace int role_val by OnOffAuto master Markus Armbruster
2016-03-02 18:56 ` Marc-André Lureau
2016-03-02 19:39 ` Markus Armbruster
2016-02-29 18:40 ` [Qemu-devel] [PATCH 34/38] ivshmem: Split ivshmem-plain, ivshmem-doorbell off ivshmem Markus Armbruster
2016-03-03 13:53 ` Marc-André Lureau
2016-02-29 18:40 ` [Qemu-devel] [PATCH 35/38] ivshmem: Clean up after the previous commit Markus Armbruster
2016-03-03 13:56 ` Marc-André Lureau
2016-02-29 18:40 ` [Qemu-devel] [PATCH 36/38] ivshmem: Drop ivshmem property x-memdev Markus Armbruster
2016-03-03 14:03 ` Marc-André Lureau
2016-03-03 14:17 ` Markus Armbruster
2016-02-29 18:40 ` [Qemu-devel] [PATCH 37/38] ivshmem: Require master to have ID zero Markus Armbruster
2016-03-03 14:11 ` Marc-André Lureau
2016-02-29 18:40 ` [Qemu-devel] [PATCH 38/38] contrib/ivshmem-server: Print "not for production" warning Markus Armbruster
2016-03-03 14:15 ` Marc-André Lureau
2016-03-07 18:42 ` Markus Armbruster [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87bn6qp1tk.fsf@blackfin.pond.sub.org \
--to=armbru@redhat.com \
--cc=cam@cs.ualberta.ca \
--cc=claudio.fontana@huawei.com \
--cc=david.marchand@6wind.com \
--cc=marcandre.lureau@gmail.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.