diff mbox

[2/5] Add vhost-kernel and the vhost-user skeleton

Message ID 1385754746-21172-3-git-send-email-a.motakis@virtualopensystems.com
State New
Headers show

Commit Message

Antonios Motakis Nov. 29, 2013, 7:52 p.m. UTC 
Introduce the backend type - vhost-kernel and vhost-user.
Add basic ioctl, open, close multiplexing depending on selected backend.

Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
Signed-off-by: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
---
 hw/net/vhost_net.c                |  3 +--
 hw/scsi/vhost-scsi.c              |  4 ++--
 hw/virtio/vhost-backend.c         | 25 ++++++++++++++++++++++---
 hw/virtio/vhost.c                 |  6 ++++--
 include/hw/virtio/vhost-backend.h |  7 +++++++
 include/hw/virtio/vhost.h         |  4 +++-
 6 files changed, 39 insertions(+), 10 deletions(-)

Comments

Stefan Hajnoczi Dec. 4, 2013, 1:47 p.m. UTC | #1 
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).
Antonios Motakis Dec. 4, 2013, 3:23 p.m. UTC | #2 
On Wed, Dec 4, 2013 at 2:47 PM, Stefan Hajnoczi <stefanha@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
diff mbox

Patch

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 0d1943f..58a1880 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -36,7 +36,6 @@ 
 #include <stdio.h>
 
 #include "hw/virtio/vhost.h"
-#include "hw/virtio/vhost-backend.h"
 #include "hw/virtio/virtio-bus.h"
 
 struct vhost_net {
@@ -113,7 +112,7 @@  struct vhost_net *vhost_net_init(NetClientState *backend, int devfd,
     net->dev.nvqs = 2;
     net->dev.vqs = net->vqs;
 
-    r = vhost_dev_init(&net->dev, devfd, "/dev/vhost-net", force);
+    r = vhost_dev_init(&net->dev, devfd, "/dev/vhost-net", VHOST_BACKEND_TYPE_KERNEL, force);
     if (r < 0) {
         goto fail;
     }
diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index 14d5030..fdc5d44 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -21,7 +21,6 @@ 
 #include "migration/migration.h"
 #include "hw/virtio/vhost-scsi.h"
 #include "hw/virtio/vhost.h"
-#include "hw/virtio/vhost-backend.h"
 #include "hw/virtio/virtio-scsi.h"
 #include "hw/virtio/virtio-bus.h"
 
@@ -226,7 +225,8 @@  static int vhost_scsi_init(VirtIODevice *vdev)
     s->dev.vqs = g_new(struct vhost_virtqueue, s->dev.nvqs);
     s->dev.vq_index = 0;
 
-    ret = vhost_dev_init(&s->dev, vhostfd, "/dev/vhost-scsi", true);
+    ret = vhost_dev_init(&s->dev, vhostfd, "/dev/vhost-scsi",
+                         VHOST_BACKEND_TYPE_KERNEL, true);
     if (ret < 0) {
         error_report("vhost-scsi: vhost initialization failed: %s\n",
                 strerror(-ret));
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");
+    }
 
     return result;
 }
@@ -36,7 +45,17 @@  int vhost_backend_init(struct vhost_dev *dev, const char *devpath)
 {
     int fd = -1;
 
-    fd = open(devpath, O_RDWR);
+    switch (dev->backend_type) {
+    case VHOST_BACKEND_TYPE_KERNEL:
+        fd = open(devpath, O_RDWR);
+        break;
+    case VHOST_BACKEND_TYPE_USER:
+        fprintf(stderr, "vhost-user not implemented\n");
+        break;
+    default:
+        fprintf(stderr, "Unknown vhost backend type\n");
+    }
+
     dev->control = fd;
 
     return fd;
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 42f4d5f..35eeb5f 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -14,7 +14,6 @@ 
  */
 
 #include "hw/virtio/vhost.h"
-#include "hw/virtio/vhost-backend.h"
 #include "hw/hw.h"
 #include "qemu/atomic.h"
 #include "qemu/range.h"
@@ -815,10 +814,13 @@  static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq)
 }
 
 int vhost_dev_init(struct vhost_dev *hdev, int devfd, const char *devpath,
-                   bool force)
+                   VhostBackendType backend_type, bool force)
 {
     uint64_t features;
     int i, r;
+
+    hdev->backend_type = backend_type;
+
     if (devfd >= 0) {
         hdev->control = devfd;
     } else {
diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
index fc51b72..970f033 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -12,6 +12,13 @@ 
 #ifndef VHOST_BACKEND_H_
 #define VHOST_BACKEND_H_
 
+typedef enum VhostBackendType {
+    VHOST_BACKEND_TYPE_NONE = 0,
+    VHOST_BACKEND_TYPE_KERNEL = 1,
+    VHOST_BACKEND_TYPE_USER = 2,
+    VHOST_BACKEND_TYPE_MAX = 3,
+} VhostBackendType;
+
 struct vhost_dev;
 int vhost_call(struct vhost_dev *dev, unsigned long int request, void *arg);
 
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index de24746..c08ae46 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -2,6 +2,7 @@ 
 #define VHOST_H
 
 #include "hw/hw.h"
+#include "hw/virtio/vhost-backend.h"
 #include "hw/virtio/virtio.h"
 #include "exec/memory.h"
 
@@ -48,10 +49,11 @@  struct vhost_dev {
     bool memory_changed;
     hwaddr mem_changed_start_addr;
     hwaddr mem_changed_end_addr;
+    VhostBackendType backend_type;
 };
 
 int vhost_dev_init(struct vhost_dev *hdev, int devfd, const char *devpath,
-                   bool force);
+                   VhostBackendType backend_type, bool force);
 void vhost_dev_cleanup(struct vhost_dev *hdev);
 bool vhost_dev_query(struct vhost_dev *hdev, VirtIODevice *vdev);
 int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev);