Re: [Qemu-devel] [PATCH 2/5] Add vhost-kernel and the vhost-user skeleton

[Antonios Motakis]

On Wed, Dec 4, 2013 at 2:47 PM, Stefan Hajnoczi <address@hidden> wrote:

On Fri, Nov 29, 2013 at 08:52:23PM +0100, Antonios Motakis wrote:
> diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
> index 05de174..80defe4 100644
> --- a/hw/virtio/vhost-backend.c
> +++ b/hw/virtio/vhost-backend.c
> @@ -25,9 +25,18 @@ static int vhost_kernel_call(struct vhost_dev *dev, unsigned long int request,
>
>  int vhost_call(struct vhost_dev *dev, unsigned long int request, void *arg)
>  {
> -    int result;
> +    int result = -1;
>
> -    result = vhost_kernel_call(dev, request, arg);
> +    switch (dev->backend_type) {
> +    case VHOST_BACKEND_TYPE_KERNEL:
> +        result = vhost_kernel_call(dev, request, arg);
> +        break;
> +    case VHOST_BACKEND_TYPE_USER:
> +        fprintf(stderr, "vhost-user not implemented\n");
> +        break;
> +    default:
> +        fprintf(stderr, "Unknown vhost backend type\n");
> +    }

The switch statement approach gets messy fast when local variables are
needed inside some case labels.  It also makes it hard to conditionally
compile features without using #ifdef.

Perhaps instead a VhostOps struct could be used:

/* Vhost backends implement this interface */
typedef struct {

    int vhost_call(struct vhost_dev *dev,
                   unsigned long int request,

                   void *arg);
    ...
} VhostOps;

const VhostOps vhost_kernel_ops = {
    ...
};

const VhostOps vhost_user_opts = {
    ...
};

ret = dev->vhost_ops->vhost_call(dev, request, arg);

Something along those lines.  It keeps the different backend
implementations separate (they can live in separate files and be
conditional in Makefile.objs).

We will take this into account as well, thanks.

Antonios