On 24.09.2015 13:37, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lur...@redhat.com>
>
> The current ivshmem protocol uses 'long' for integers. But the
> sizeof(long) depends on the host and the endianess is not defined, which
> may cause portability troubles.
>
> Instead, switch to using little-endian int64_t. This breaks the
> protocol, except on x64 little-endian host where this change
> should be compatible.
>
> Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> ---
> contrib/ivshmem-client/ivshmem-client.c | 11 ++++++-----
> contrib/ivshmem-client/ivshmem-client.h | 4 ++--
> contrib/ivshmem-server/ivshmem-server.c | 5 +++--
> contrib/ivshmem-server/ivshmem-server.h | 4 ++--
> docs/specs/ivshmem_device_spec.txt | 2 +-
> hw/misc/ivshmem.c | 29 +++++++++++++++++++----------
> 6 files changed, 33 insertions(+), 22 deletions(-)
>
> diff --git a/contrib/ivshmem-client/ivshmem-client.c
> b/contrib/ivshmem-client/ivshmem-client.c
> index a8477d8..4b5786c 100644
> --- a/contrib/ivshmem-client/ivshmem-client.c
> +++ b/contrib/ivshmem-client/ivshmem-client.c
> @@ -24,7 +24,7 @@
>
> /* read message from the unix socket */
> static int
> -ivshmem_client_read_one_msg(IvshmemClient *client, long *index, int *fd)
> +ivshmem_client_read_one_msg(IvshmemClient *client, int64_t *index, int *fd)
> {
> int ret;
> struct msghdr msg;
> @@ -45,7 +45,7 @@ ivshmem_client_read_one_msg(IvshmemClient *client, long
> *index, int *fd)
> msg.msg_controllen = sizeof(msg_control);
>
> ret = recvmsg(client->sock_fd, &msg, 0);
> - if (ret < 0) {
> + if (ret < sizeof(*index)) {
> IVSHMEM_CLIENT_DEBUG(client, "cannot read message: %s\n",
> strerror(errno));
> return -1;
> @@ -55,6 +55,7 @@ ivshmem_client_read_one_msg(IvshmemClient *client, long
> *index, int *fd)
> return -1;
> }
>
> + *index = GINT64_FROM_LE(*index);
> *fd = -1;
>
> for (cmsg = CMSG_FIRSTHDR(&msg); cmsg; cmsg = CMSG_NXTHDR(&msg, cmsg)) {
> @@ -91,7 +92,7 @@ static int
> ivshmem_client_handle_server_msg(IvshmemClient *client)
> {
> IvshmemClientPeer *peer;
> - long peer_id;
> + int64_t peer_id;
> int ret, fd;
>
> ret = ivshmem_client_read_one_msg(client, &peer_id, &fd);
> @@ -179,7 +180,7 @@ ivshmem_client_connect(IvshmemClient *client)
> {
> struct sockaddr_un sun;
> int fd, ret;
> - long tmp;
> + int64_t tmp;
>
> IVSHMEM_CLIENT_DEBUG(client, "connect to client %s\n",
> client->unix_sock_path);
> @@ -401,7 +402,7 @@ ivshmem_client_notify_broadcast(const IvshmemClient
> *client)
>
> /* lookup peer from its id */
> IvshmemClientPeer *
> -ivshmem_client_search_peer(IvshmemClient *client, long peer_id)
> +ivshmem_client_search_peer(IvshmemClient *client, int64_t peer_id)
> {
> IvshmemClientPeer *peer;
>
> diff --git a/contrib/ivshmem-client/ivshmem-client.h
> b/contrib/ivshmem-client/ivshmem-client.h
> index 9215f34..3a4f809 100644
> --- a/contrib/ivshmem-client/ivshmem-client.h
> +++ b/contrib/ivshmem-client/ivshmem-client.h
> @@ -43,7 +43,7 @@
> */
> typedef struct IvshmemClientPeer {
> QTAILQ_ENTRY(IvshmemClientPeer) next; /**< next in list*/
> - long id; /**< the id of the peer */
> + int64_t id; /**< the id of the peer */
> int vectors[IVSHMEM_CLIENT_MAX_VECTORS]; /**< one fd per vector */
> unsigned vectors_count; /**< number of vectors */
> } IvshmemClientPeer;
> @@ -198,7 +198,7 @@ int ivshmem_client_notify_broadcast(const IvshmemClient
> *client);
> * Returns: The peer structure, or NULL if not found
> */
> IvshmemClientPeer *
> -ivshmem_client_search_peer(IvshmemClient *client, long peer_id);
> +ivshmem_client_search_peer(IvshmemClient *client, int64_t peer_id);
>
> /**
> * Dump information of this ivshmem client on stdout
> diff --git a/contrib/ivshmem-server/ivshmem-server.c
> b/contrib/ivshmem-server/ivshmem-server.c
> index 060f414..3742a78 100644
> --- a/contrib/ivshmem-server/ivshmem-server.c
> +++ b/contrib/ivshmem-server/ivshmem-server.c
> @@ -33,7 +33,7 @@
>
> /* send message to a client unix socket */
> static int
> -ivshmem_server_send_one_msg(int sock_fd, long peer_id, int fd)
> +ivshmem_server_send_one_msg(int sock_fd, int64_t peer_id, int fd)
> {
> int ret;
> struct msghdr msg;
> @@ -44,6 +44,7 @@ ivshmem_server_send_one_msg(int sock_fd, long peer_id, int
> fd)
> } msg_control;
> struct cmsghdr *cmsg;
>
> + peer_id = GINT64_TO_LE(peer_id);
> iov[0].iov_base = &peer_id;
> iov[0].iov_len = sizeof(peer_id);
>
> @@ -448,7 +449,7 @@ ivshmem_server_handle_fds(IvshmemServer *server, fd_set
> *fds, int maxfd)
>
> /* lookup peer from its id */
> IvshmemServerPeer *
> -ivshmem_server_search_peer(IvshmemServer *server, long peer_id)
> +ivshmem_server_search_peer(IvshmemServer *server, int64_t peer_id)
> {
> IvshmemServerPeer *peer;
>
> diff --git a/contrib/ivshmem-server/ivshmem-server.h
> b/contrib/ivshmem-server/ivshmem-server.h
> index 65b3c2d..d179f22 100644
> --- a/contrib/ivshmem-server/ivshmem-server.h
> +++ b/contrib/ivshmem-server/ivshmem-server.h
> @@ -50,7 +50,7 @@
> typedef struct IvshmemServerPeer {
> QTAILQ_ENTRY(IvshmemServerPeer) next; /**< next in list*/
> int sock_fd; /**< connected unix sock */
> - long id; /**< the id of the peer */
> + int64_t id; /**< the id of the peer */
> int vectors[IVSHMEM_SERVER_MAX_VECTORS]; /**< one fd per vector */
> unsigned vectors_count; /**< number of vectors */
> } IvshmemServerPeer;
> @@ -154,7 +154,7 @@ int ivshmem_server_handle_fds(IvshmemServer *server,
> fd_set *fds, int maxfd);
> * Returns: The peer structure, or NULL if not found
> */
> IvshmemServerPeer *
> -ivshmem_server_search_peer(IvshmemServer *server, long peer_id);
> +ivshmem_server_search_peer(IvshmemServer *server, int64_t peer_id);
>
> /**
> * Dump information of this ivshmem server and its peers on stdout
> diff --git a/docs/specs/ivshmem_device_spec.txt
> b/docs/specs/ivshmem_device_spec.txt
> index 3435116..d318d65 100644
> --- a/docs/specs/ivshmem_device_spec.txt
> +++ b/docs/specs/ivshmem_device_spec.txt
> @@ -61,7 +61,7 @@ This server code is available in
> qemu.git/contrib/ivshmem-server.
>
> The server must be started on the host before any guest.
> It creates a shared memory object then waits for clients to connect on a unix
> -socket.
> +socket. All the messages are little-endian int64_t integer.
>
> For each client (QEMU process) that connects to the server:
> - the server sends a protocol version, if client does not support it, the
> client
> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> index 39c0791..71e58b8 100644
> --- a/hw/misc/ivshmem.c
> +++ b/hw/misc/ivshmem.c
> @@ -276,7 +276,7 @@ static void ivshmem_receive(void *opaque, const uint8_t
> *buf, int size)
>
> static int ivshmem_can_receive(void * opaque)
> {
> - return sizeof(long);
> + return sizeof(int64_t);
> }
>
> static void ivshmem_event(void *opaque, int event)
> @@ -516,7 +516,7 @@ static bool fifo_update_and_get(IVShmemState *s, const
> uint8_t *buf, int size,
> const uint8_t *p;
> uint32_t num;
>
> - assert(len <= sizeof(long)); /* limitation of the fifo */
> + assert(len <= sizeof(int64_t)); /* limitation of the fifo */
> if (fifo8_is_empty(&s->incoming_fifo) && size == len) {
> memcpy(data, buf, size);
> return true;
> @@ -524,7 +524,7 @@ static bool fifo_update_and_get(IVShmemState *s, const
> uint8_t *buf, int size,
>
> IVSHMEM_DPRINTF("short read of %d bytes\n", size);
>
> - num = MIN(size, sizeof(long) - fifo8_num_used(&s->incoming_fifo));
> + 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) {
> @@ -546,6 +546,17 @@ static bool fifo_update_and_get(IVShmemState *s, const
> uint8_t *buf, int 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 int ivshmem_add_kvm_msi_virq(IVShmemState *s, int vector)
> {
> PCIDevice *pdev = PCI_DEVICE(s);
> @@ -598,12 +609,11 @@ static void ivshmem_read(void *opaque, const uint8_t
> *buf, int size)
> IVShmemState *s = opaque;
> int incoming_fd;
> int new_eventfd;
> - long incoming_posn;
> + int64_t incoming_posn;
> Error *err = NULL;
> Peer *peer;
>
> - if (!fifo_update_and_get(s, buf, size,
> - &incoming_posn, sizeof(incoming_posn))) {
> + if (!fifo_update_and_get_i64(s, buf, size, &incoming_posn)) {
> return;
> }
>
> @@ -711,10 +721,9 @@ static void ivshmem_check_version(void *opaque, const
> uint8_t * buf, int size)
> {
> IVShmemState *s = opaque;
> int tmp;
> - long version;
> + int64_t version;
>
> - if (!fifo_update_and_get(s, buf, size,
> - &version, sizeof(version))) {
> + if (!fifo_update_and_get_i64(s, buf, size, &version)) {
> return;
> }
>
> @@ -869,7 +878,7 @@ static void pci_ivshmem_realize(PCIDevice *dev, Error
> **errp)
> s->ivshmem_size = size;
> }
>
> - fifo8_create(&s->incoming_fifo, sizeof(long));
> + fifo8_create(&s->incoming_fifo, sizeof(int64_t));
>
> /* IRQFD requires MSI */
> if (ivshmem_has_feature(s, IVSHMEM_IOEVENTFD) &&
>
Reviewed-by: Claudio Fontana <claudio.font...@huawei.com>
