Hi On Mon, Feb 29, 2016 at 7:40 PM, Markus Armbruster <arm...@redhat.com> 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 <arm...@redhat.com> > --- > 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