Re: [PATCH v6 1/3] contrib: add ivshmem client and server

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Sep 08, 2014 at 11:17:48AM +0200, David Marchand wrote:

...

> diff --git a/configure b/configure
> index 961bf6f..a41a16c 100755
> --- a/configure
> +++ b/configure
> @@ -4125,6 +4125,9 @@ if test "$want_tools" = "yes" ; then
>    if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" ] ; then
>      tools="qemu-nbd\$(EXESUF) $tools"
>    fi
> +  if [ "$kvm" = "yes" ] ; then
> +    tools="ivshmem-client\$(EXESUF) ivshmem-server\$(EXESUF) $tools"
> +  fi

Why are you tying this to kvm btw?
Shouldn't be for any linux host?

>  fi
>  if test "$softmmu" = yes ; then
>    if test "$virtfs" != no ; then

...

> diff --git a/contrib/ivshmem-client/ivshmem-client.h b/contrib/ivshmem-client/ivshmem-client.h
> new file mode 100644
> index 0000000..284c4a3
> --- /dev/null
> +++ b/contrib/ivshmem-client/ivshmem-client.h
> @@ -0,0 +1,212 @@
> +/*
> + * Copyright 6WIND S.A., 2014
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * (at your option) any later version.  See the COPYING file in the
> + * top-level directory.
> + */
> +
> +#ifndef _IVSHMEM_CLIENT_H_
> +#define _IVSHMEM_CLIENT_H_
> +
> +/**
> + * This file provides helper to implement an ivshmem client. It is used
> + * on the host to ask QEMU to send an interrupt to an ivshmem PCI device in a
> + * guest. QEMU also implements an ivshmem client similar to this one, they both
> + * connect to an ivshmem server.
> + *
> + * A standalone ivshmem client based on this file is provided for debug/test
> + * purposes.
> + */
> +
> +#include <limits.h>
> +#include <sys/select.h>
> +
> +#include "qemu/queue.h"
> +
> +/**
> + * Maximum number of notification vectors supported by the client
> + */
> +#define IVSHMEM_CLIENT_MAX_VECTORS 64
> +
> +/**
> + * Structure storing a peer
> + *
> + * Each time a client connects to an ivshmem server, it is advertised to
> + * all connected clients through the unix socket. When our ivshmem
> + * client receives a notification, it creates a IvshmemClientPeer
> + * structure to store the infos of this peer.
> + *
> + * This structure is also used to store the information of our own
> + * client in (IvshmemClient)->local.
> + */
> +typedef struct IvshmemClientPeer {
> +    QTAILQ_ENTRY(IvshmemClientPeer) next;    /**< next in list*/

Needs space before */

> +    long id;                                 /**< the id of the peer */
> +    int vectors[IVSHMEM_CLIENT_MAX_VECTORS]; /**< one fd per vector */
> +    unsigned vectors_count;                  /**< number of vectors */

I would do just /* text */ here, otherewise people might think
they have to copy this comment style.

> +} IvshmemClientPeer;


an empty line wouldn't hurt here.

> +QTAILQ_HEAD(IvshmemClientPeerList, IvshmemClientPeer);
> +
> +typedef struct IvshmemClientPeerList IvshmemClientPeerList;
> +typedef struct IvshmemClient IvshmemClient;
> +