diff mbox

[v5,11/21] vhost: only use shared log if in use by backend

Message ID 1443111741-4736-12-git-send-email-marcandre.lureau@redhat.com
State New
Headers show

Commit Message

Marc-André Lureau Sept. 24, 2015, 4:22 p.m. UTC 
From: Marc-André Lureau <marcandre.lureau@redhat.com>

Do not allocate a shared log if the backend doesn't support it.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/virtio/vhost-user.c            |  9 +++++++++
 hw/virtio/vhost.c                 | 15 ++++++++++-----
 include/hw/virtio/vhost-backend.h |  4 ++++
 3 files changed, 23 insertions(+), 5 deletions(-)

Comments

Michael S. Tsirkin Sept. 29, 2015, 2:59 p.m. UTC | #1 
On Thu, Sep 24, 2015 at 06:22:11PM +0200, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Do not allocate a shared log if the backend doesn't support it.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

So squash this in the previous patch introducing the shared log?
This split makes review harder, not easier.

> ---
>  hw/virtio/vhost-user.c            |  9 +++++++++
>  hw/virtio/vhost.c                 | 15 ++++++++++-----
>  include/hw/virtio/vhost-backend.h |  4 ++++
>  3 files changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 41e0364..455caba 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -553,6 +553,14 @@ static int vhost_user_get_vq_index(struct vhost_dev *dev, int idx)
>      return idx;
>  }
>  
> +static bool vhost_user_requires_shm_log(struct vhost_dev *dev)
> +{
> +    assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
> +
> +    return virtio_has_feature(dev->protocol_features,
> +                              VHOST_USER_PROTOCOL_F_LOG_SHMFD);
> +}
> +
>  const VhostOps user_ops = {
>          .backend_type = VHOST_BACKEND_TYPE_USER,
>          .vhost_backend_init = vhost_user_init,
> @@ -573,4 +581,5 @@ const VhostOps user_ops = {
>          .vhost_reset_device = vhost_user_reset_device,
>          .vhost_get_vq_index = vhost_user_get_vq_index,
>          .vhost_set_vring_enable = vhost_user_set_vring_enable,
> +        .vhost_requires_shm_log = vhost_user_requires_shm_log,
>  };
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index d9f2131..ec27c7b 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -357,10 +357,15 @@ static void vhost_log_put(struct vhost_dev *dev, bool sync)
>      }
>  }
>  
> -static inline void vhost_dev_log_resize(struct vhost_dev* dev, uint64_t size)
> +static bool vhost_dev_log_is_shared(struct vhost_dev *dev)
>  {
> -    bool share = dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER;
> -    struct vhost_log *log = vhost_log_get(size, share);
> +    return dev->vhost_ops->vhost_requires_shm_log &&
> +           dev->vhost_ops->vhost_requires_shm_log(dev);
> +}
> +
> +static inline void vhost_dev_log_resize(struct vhost_dev *dev, uint64_t size)
> +{
> +    struct vhost_log *log = vhost_log_get(size, vhost_dev_log_is_shared(dev));
>      uint64_t log_base = (uintptr_t)log->log;
>      int r;
>  
> @@ -1166,10 +1171,10 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
>  
>      if (hdev->log_enabled) {
>          uint64_t log_base;
> -        bool share = hdev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER;
>  
>          hdev->log_size = vhost_get_log_size(hdev);
> -        hdev->log = vhost_log_get(hdev->log_size, share);
> +        hdev->log = vhost_log_get(hdev->log_size,
> +                                  vhost_dev_log_is_shared(hdev));
>          log_base = (uintptr_t)hdev->log->log;
>          r = hdev->vhost_ops->vhost_set_log_base(hdev,
>                                                  hdev->log_size ? log_base : 0,
> diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
> index 868a78a..e07118c 100644
> --- a/include/hw/virtio/vhost-backend.h
> +++ b/include/hw/virtio/vhost-backend.h
> @@ -11,6 +11,8 @@
>  #ifndef VHOST_BACKEND_H_
>  #define VHOST_BACKEND_H_
>  
> +#include <stdbool.h>
> +
>  typedef enum VhostBackendType {
>      VHOST_BACKEND_TYPE_NONE = 0,
>      VHOST_BACKEND_TYPE_KERNEL = 1,
> @@ -64,6 +66,7 @@ typedef int (*vhost_reset_device_op)(struct vhost_dev *dev);
>  typedef int (*vhost_get_vq_index_op)(struct vhost_dev *dev, int idx);
>  typedef int (*vhost_set_vring_enable_op)(struct vhost_dev *dev,
>                                           int enable);
> +typedef bool (*vhost_requires_shm_log_op)(struct vhost_dev *dev);
>  
>  typedef struct VhostOps {
>      VhostBackendType backend_type;
> @@ -90,6 +93,7 @@ typedef struct VhostOps {
>      vhost_reset_device_op vhost_reset_device;
>      vhost_get_vq_index_op vhost_get_vq_index;
>      vhost_set_vring_enable_op vhost_set_vring_enable;
> +    vhost_requires_shm_log_op vhost_requires_shm_log;
>  } VhostOps;
>  
>  extern const VhostOps user_ops;
> -- 
> 2.4.3
Marc-Andre Lureau Sept. 29, 2015, 3:26 p.m. UTC | #2 
Hi

----- Original Message -----
> On Thu, Sep 24, 2015 at 06:22:11PM +0200, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > 
> > Do not allocate a shared log if the backend doesn't support it.
> > 
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> So squash this in the previous patch introducing the shared log?
> This split makes review harder, not easier.

Ok, I'll try to reorder things

> 
> > ---
> >  hw/virtio/vhost-user.c            |  9 +++++++++
> >  hw/virtio/vhost.c                 | 15 ++++++++++-----
> >  include/hw/virtio/vhost-backend.h |  4 ++++
> >  3 files changed, 23 insertions(+), 5 deletions(-)
> > 
> > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > index 41e0364..455caba 100644
> > --- a/hw/virtio/vhost-user.c
> > +++ b/hw/virtio/vhost-user.c
> > @@ -553,6 +553,14 @@ static int vhost_user_get_vq_index(struct vhost_dev
> > *dev, int idx)
> >      return idx;
> >  }
> >  
> > +static bool vhost_user_requires_shm_log(struct vhost_dev *dev)
> > +{
> > +    assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
> > +
> > +    return virtio_has_feature(dev->protocol_features,
> > +                              VHOST_USER_PROTOCOL_F_LOG_SHMFD);
> > +}
> > +
> >  const VhostOps user_ops = {
> >          .backend_type = VHOST_BACKEND_TYPE_USER,
> >          .vhost_backend_init = vhost_user_init,
> > @@ -573,4 +581,5 @@ const VhostOps user_ops = {
> >          .vhost_reset_device = vhost_user_reset_device,
> >          .vhost_get_vq_index = vhost_user_get_vq_index,
> >          .vhost_set_vring_enable = vhost_user_set_vring_enable,
> > +        .vhost_requires_shm_log = vhost_user_requires_shm_log,
> >  };
> > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> > index d9f2131..ec27c7b 100644
> > --- a/hw/virtio/vhost.c
> > +++ b/hw/virtio/vhost.c
> > @@ -357,10 +357,15 @@ static void vhost_log_put(struct vhost_dev *dev, bool
> > sync)
> >      }
> >  }
> >  
> > -static inline void vhost_dev_log_resize(struct vhost_dev* dev, uint64_t
> > size)
> > +static bool vhost_dev_log_is_shared(struct vhost_dev *dev)
> >  {
> > -    bool share = dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER;
> > -    struct vhost_log *log = vhost_log_get(size, share);
> > +    return dev->vhost_ops->vhost_requires_shm_log &&
> > +           dev->vhost_ops->vhost_requires_shm_log(dev);
> > +}
> > +
> > +static inline void vhost_dev_log_resize(struct vhost_dev *dev, uint64_t
> > size)
> > +{
> > +    struct vhost_log *log = vhost_log_get(size,
> > vhost_dev_log_is_shared(dev));
> >      uint64_t log_base = (uintptr_t)log->log;
> >      int r;
> >  
> > @@ -1166,10 +1171,10 @@ int vhost_dev_start(struct vhost_dev *hdev,
> > VirtIODevice *vdev)
> >  
> >      if (hdev->log_enabled) {
> >          uint64_t log_base;
> > -        bool share = hdev->vhost_ops->backend_type ==
> > VHOST_BACKEND_TYPE_USER;
> >  
> >          hdev->log_size = vhost_get_log_size(hdev);
> > -        hdev->log = vhost_log_get(hdev->log_size, share);
> > +        hdev->log = vhost_log_get(hdev->log_size,
> > +                                  vhost_dev_log_is_shared(hdev));
> >          log_base = (uintptr_t)hdev->log->log;
> >          r = hdev->vhost_ops->vhost_set_log_base(hdev,
> >                                                  hdev->log_size ? log_base
> >                                                  : 0,
> > diff --git a/include/hw/virtio/vhost-backend.h
> > b/include/hw/virtio/vhost-backend.h
> > index 868a78a..e07118c 100644
> > --- a/include/hw/virtio/vhost-backend.h
> > +++ b/include/hw/virtio/vhost-backend.h
> > @@ -11,6 +11,8 @@
> >  #ifndef VHOST_BACKEND_H_
> >  #define VHOST_BACKEND_H_
> >  
> > +#include <stdbool.h>
> > +
> >  typedef enum VhostBackendType {
> >      VHOST_BACKEND_TYPE_NONE = 0,
> >      VHOST_BACKEND_TYPE_KERNEL = 1,
> > @@ -64,6 +66,7 @@ typedef int (*vhost_reset_device_op)(struct vhost_dev
> > *dev);
> >  typedef int (*vhost_get_vq_index_op)(struct vhost_dev *dev, int idx);
> >  typedef int (*vhost_set_vring_enable_op)(struct vhost_dev *dev,
> >                                           int enable);
> > +typedef bool (*vhost_requires_shm_log_op)(struct vhost_dev *dev);
> >  
> >  typedef struct VhostOps {
> >      VhostBackendType backend_type;
> > @@ -90,6 +93,7 @@ typedef struct VhostOps {
> >      vhost_reset_device_op vhost_reset_device;
> >      vhost_get_vq_index_op vhost_get_vq_index;
> >      vhost_set_vring_enable_op vhost_set_vring_enable;
> > +    vhost_requires_shm_log_op vhost_requires_shm_log;
> >  } VhostOps;
> >  
> >  extern const VhostOps user_ops;
> > --
> > 2.4.3
>
diff mbox

Patch

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 41e0364..455caba 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -553,6 +553,14 @@  static int vhost_user_get_vq_index(struct vhost_dev *dev, int idx)
     return idx;
 }
 
+static bool vhost_user_requires_shm_log(struct vhost_dev *dev)
+{
+    assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
+
+    return virtio_has_feature(dev->protocol_features,
+                              VHOST_USER_PROTOCOL_F_LOG_SHMFD);
+}
+
 const VhostOps user_ops = {
         .backend_type = VHOST_BACKEND_TYPE_USER,
         .vhost_backend_init = vhost_user_init,
@@ -573,4 +581,5 @@  const VhostOps user_ops = {
         .vhost_reset_device = vhost_user_reset_device,
         .vhost_get_vq_index = vhost_user_get_vq_index,
         .vhost_set_vring_enable = vhost_user_set_vring_enable,
+        .vhost_requires_shm_log = vhost_user_requires_shm_log,
 };
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index d9f2131..ec27c7b 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -357,10 +357,15 @@  static void vhost_log_put(struct vhost_dev *dev, bool sync)
     }
 }
 
-static inline void vhost_dev_log_resize(struct vhost_dev* dev, uint64_t size)
+static bool vhost_dev_log_is_shared(struct vhost_dev *dev)
 {
-    bool share = dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER;
-    struct vhost_log *log = vhost_log_get(size, share);
+    return dev->vhost_ops->vhost_requires_shm_log &&
+           dev->vhost_ops->vhost_requires_shm_log(dev);
+}
+
+static inline void vhost_dev_log_resize(struct vhost_dev *dev, uint64_t size)
+{
+    struct vhost_log *log = vhost_log_get(size, vhost_dev_log_is_shared(dev));
     uint64_t log_base = (uintptr_t)log->log;
     int r;
 
@@ -1166,10 +1171,10 @@  int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
 
     if (hdev->log_enabled) {
         uint64_t log_base;
-        bool share = hdev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER;
 
         hdev->log_size = vhost_get_log_size(hdev);
-        hdev->log = vhost_log_get(hdev->log_size, share);
+        hdev->log = vhost_log_get(hdev->log_size,
+                                  vhost_dev_log_is_shared(hdev));
         log_base = (uintptr_t)hdev->log->log;
         r = hdev->vhost_ops->vhost_set_log_base(hdev,
                                                 hdev->log_size ? log_base : 0,
diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
index 868a78a..e07118c 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -11,6 +11,8 @@ 
 #ifndef VHOST_BACKEND_H_
 #define VHOST_BACKEND_H_
 
+#include <stdbool.h>
+
 typedef enum VhostBackendType {
     VHOST_BACKEND_TYPE_NONE = 0,
     VHOST_BACKEND_TYPE_KERNEL = 1,
@@ -64,6 +66,7 @@  typedef int (*vhost_reset_device_op)(struct vhost_dev *dev);
 typedef int (*vhost_get_vq_index_op)(struct vhost_dev *dev, int idx);
 typedef int (*vhost_set_vring_enable_op)(struct vhost_dev *dev,
                                          int enable);
+typedef bool (*vhost_requires_shm_log_op)(struct vhost_dev *dev);
 
 typedef struct VhostOps {
     VhostBackendType backend_type;
@@ -90,6 +93,7 @@  typedef struct VhostOps {
     vhost_reset_device_op vhost_reset_device;
     vhost_get_vq_index_op vhost_get_vq_index;
     vhost_set_vring_enable_op vhost_set_vring_enable;
+    vhost_requires_shm_log_op vhost_requires_shm_log;
 } VhostOps;
 
 extern const VhostOps user_ops;