On Wed, Dec 4, 2013 at 2:47 PM, Stefan Hajnoczi <stefa...@gmail.com> 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
