[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 29/46] ivshmem: error on too many eventfd rec
From: |
Marc-André Lureau |
Subject: |
Re: [Qemu-devel] [PATCH v3 29/46] ivshmem: error on too many eventfd received |
Date: |
Wed, 23 Sep 2015 12:47:44 +0200 |
Hi
On Wed, Sep 16, 2015 at 2:14 PM, Claudio Fontana
<address@hidden> wrote:
> On 15.09.2015 18:07, address@hidden wrote:
>> From: Marc-André Lureau <address@hidden>
>>
>> The number of eventfd that can be handled per peer is limited by the
>> number of vectors. Return an error when receiving too many of them.
>>
>> Signed-off-by: Marc-André Lureau <address@hidden>
>> ---
>> hw/misc/ivshmem.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
>> index b9c78cd..63e4c4f 100644
>> --- a/hw/misc/ivshmem.c
>> +++ b/hw/misc/ivshmem.c
>> @@ -569,6 +569,13 @@ static void ivshmem_read(void *opaque, const uint8_t
>> *buf, int size)
>> }
>>
>> /* get a new eventfd */
>> + if (peer->nb_eventfds >= s->vectors) {
>> + error_report("Too many eventfd received, device has %d vectors",
>> + s->vectors);
>> + close(incoming_fd);
>> + return;
>> + }
>> +
>> nth_eventfd = peer->nb_eventfds++;
>>
>> /* this is an eventfd for a particular peer VM */
>>
>
> can the device still operate if we detect these errors at ivshmem_read time?
>
> I am referring also to the other checks happening in ivshmem_read doing
>
> if ([something fails]) {
> error_report("abcabc");
> /* close(), ... */
> return;
> }
>
> Can the device stop operating if these conditions happen?
Yes, it simply closes the "new" incoming_fd. So if the server sends
extra eventfd for peers that we can't handle, we won't be able to
notify those peers. But the rest of the peers and function is still
working.
This is btw, what the current code is doing (only the variable is
called tmp_fd before I removed it in "remove unnecessary dup()" patch)
> If so, do we have to put the device into a non-operating state, where all
> read/writes are ignored? Should there be a ivshmem status flag for ERROR?
> Should we exit(1)?
>
> note: I don't know what the "proper" behavior should be, but I am concerned
> about the runtime stability of the software which uses the device.
It's likely a misconfiguration. So having error reported seems like a
sane and enough thing to do.
--
Marc-André Lureau
- [Qemu-devel] [PATCH v3 25/46] ivshmem: check shm isn't already initialized, (continued)
- [Qemu-devel] [PATCH v3 25/46] ivshmem: check shm isn't already initialized, marcandre . lureau, 2015/09/15
- [Qemu-devel] [PATCH v3 24/46] ivshmem: shmfd can be 0, marcandre . lureau, 2015/09/15
- [Qemu-devel] [PATCH v3 27/46] ivshmem: fix pci_ivshmem_exit(), marcandre . lureau, 2015/09/15
- [Qemu-devel] [PATCH v3 26/46] ivshmem: add device description, marcandre . lureau, 2015/09/15
- [Qemu-devel] [PATCH v3 28/46] ivshmem: replace 'guest' for 'peer' appropriately, marcandre . lureau, 2015/09/15
- [Qemu-devel] [PATCH v3 29/46] ivshmem: error on too many eventfd received, marcandre . lureau, 2015/09/15
- [Qemu-devel] [PATCH v3 30/46] ivshmem: reset mask on device reset, marcandre . lureau, 2015/09/15
- [Qemu-devel] [PATCH v3 32/46] ivshmem-client: check the number of vectors, marcandre . lureau, 2015/09/15
- [Qemu-devel] [PATCH v3 33/46] ivshmem-server: use a uint16 for client ID, marcandre . lureau, 2015/09/15
- [Qemu-devel] [PATCH v3 31/46] contrib: add ivshmem client and server, marcandre . lureau, 2015/09/15
- [Qemu-devel] [PATCH v3 34/46] ivshmem-server: fix hugetlbfs support, marcandre . lureau, 2015/09/15