[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 27/38] ivshmem: Simplify how we cope with short
From: |
Marc-André Lureau |
Subject: |
Re: [Qemu-devel] [PATCH 27/38] ivshmem: Simplify how we cope with short reads from server |
Date: |
Wed, 2 Mar 2016 19:41:49 +0100 |
Hi
On Mon, Feb 29, 2016 at 7:40 PM, Markus Armbruster <address@hidden> wrote:
> Short reads from a UNIX domain sockets are exceedingly unlikely when
> the other side always sends eight bytes and we always read eight
> bytes. We cope with them anyway. However, the code doing that is
> rather convoluted. Dumb it down radically.
>
> Replace the convoluted code
agreed
>
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
> hw/misc/ivshmem.c | 76
> ++++++++++++-------------------------------------------
> 1 file changed, 16 insertions(+), 60 deletions(-)
>
> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> index e578b8a..fb8a4f7 100644
> --- a/hw/misc/ivshmem.c
> +++ b/hw/misc/ivshmem.c
> @@ -26,7 +26,6 @@
> #include "migration/migration.h"
> #include "qemu/error-report.h"
> #include "qemu/event_notifier.h"
> -#include "qemu/fifo8.h"
> #include "sysemu/char.h"
> #include "sysemu/hostmem.h"
> #include "qapi/visitor.h"
> @@ -80,7 +79,6 @@ typedef struct IVShmemState {
> uint32_t intrstatus;
>
> CharDriverState *server_chr;
> - Fifo8 incoming_fifo;
> MemoryRegion ivshmem_mmio;
>
> /* We might need to register the BAR before we actually have the memory.
> @@ -99,6 +97,8 @@ typedef struct IVShmemState {
> uint32_t vectors;
> uint32_t features;
> MSIVector *msi_vectors;
> + uint64_t msg_buf; /* buffer for receiving server messages */
> + int msg_buffered_bytes; /* #bytes in @msg_buf */
>
> Error *migration_blocker;
>
> @@ -255,11 +255,6 @@ static const MemoryRegionOps ivshmem_mmio_ops = {
> },
> };
>
> -static int ivshmem_can_receive(void * opaque)
> -{
> - return sizeof(int64_t);
> -}
> -
> static void ivshmem_vector_notify(void *opaque)
> {
> MSIVector *entry = opaque;
> @@ -459,53 +454,6 @@ static void resize_peers(IVShmemState *s, int nb_peers)
> }
> }
>
> -static bool fifo_update_and_get(IVShmemState *s, const uint8_t *buf, int
> size,
> - void *data, size_t len)
> -{
> - const uint8_t *p;
> - uint32_t num;
> -
> - assert(len <= sizeof(int64_t)); /* limitation of the fifo */
> - if (fifo8_is_empty(&s->incoming_fifo) && size == len) {
> - memcpy(data, buf, size);
> - return true;
> - }
> -
> - IVSHMEM_DPRINTF("short read of %d bytes\n", size);
> -
> - num = MIN(size, sizeof(int64_t) - fifo8_num_used(&s->incoming_fifo));
> - fifo8_push_all(&s->incoming_fifo, buf, num);
> -
> - if (fifo8_num_used(&s->incoming_fifo) < len) {
> - assert(num == 0);
> - return false;
> - }
> -
> - size -= num;
> - buf += num;
> - p = fifo8_pop_buf(&s->incoming_fifo, len, &num);
> - assert(num == len);
> -
> - memcpy(data, p, len);
> -
> - if (size > 0) {
> - fifo8_push_all(&s->incoming_fifo, buf, size);
> - }
> -
> - return true;
> -}
> -
> -static bool fifo_update_and_get_i64(IVShmemState *s,
> - const uint8_t *buf, int size, int64_t
> *i64)
> -{
> - if (fifo_update_and_get(s, buf, size, i64, sizeof(*i64))) {
> - *i64 = GINT64_FROM_LE(*i64);
> - return true;
> - }
> -
> - return false;
> -}
> -
> static void ivshmem_add_kvm_msi_virq(IVShmemState *s, int vector,
> Error **errp)
> {
> @@ -658,6 +606,14 @@ static void process_msg(IVShmemState *s, int64_t msg,
> int fd, Error **errp)
> }
> }
>
> +static int ivshmem_can_receive(void *opaque)
> +{
> + IVShmemState *s = opaque;
> +
> + assert(s->msg_buffered_bytes < sizeof(s->msg_buf));
> + return sizeof(s->msg_buf) - s->msg_buffered_bytes;
> +}
> +
> static void ivshmem_read(void *opaque, const uint8_t *buf, int size)
> {
> IVShmemState *s = opaque;
> @@ -665,8 +621,12 @@ static void ivshmem_read(void *opaque, const uint8_t
> *buf, int size)
> int incoming_fd;
> int64_t incoming_posn;
>
> - if (!fifo_update_and_get_i64(s, buf, size, &incoming_posn)) {
> - return;
> + assert(size >= 0 && s->msg_buffered_bytes + size <= sizeof(s->msg_buf));
> + memcpy((unsigned char *)&s->msg_buf + s->msg_buffered_bytes, buf, size);
> + s->msg_buffered_bytes += size;
> + if (s->msg_buffered_bytes == sizeof(s->msg_buf)) {
> + incoming_posn = le64_to_cpu(s->msg_buf);
> + s->msg_buffered_bytes = 0;
> }
>
missing "else return" though.
> incoming_fd = qemu_chr_fe_get_msgfd(s->server_chr);
> @@ -1019,8 +979,6 @@ static void pci_ivshmem_realize(PCIDevice *dev, Error
> **errp)
> }
> }
>
> - fifo8_create(&s->incoming_fifo, sizeof(int64_t));
> -
> if (s->role_val == IVSHMEM_PEER) {
> error_setg(&s->migration_blocker,
> "Migration is disabled when using feature 'peer mode' in
> device 'ivshmem'");
> @@ -1033,8 +991,6 @@ static void pci_ivshmem_exit(PCIDevice *dev)
> IVShmemState *s = IVSHMEM(dev);
> int i;
>
> - fifo8_destroy(&s->incoming_fifo);
> -
> if (s->migration_blocker) {
> migrate_del_blocker(s->migration_blocker);
> error_free(s->migration_blocker);
> --
> 2.4.3
>
>
--
Marc-André Lureau
- Re: [Qemu-devel] [PATCH 27/38] ivshmem: Simplify how we cope with short reads from server,
Marc-André Lureau <=