[Qemu-devel] [PATCH] vhost_net: cleanup start/stop condition

[Qemu-devel] [PATCH] vhost_net: cleanup start/stop condition

[Michael S. Tsirkin]

Checking vhost device internal state in vhost_net looks like
a layering violation since vhost_net does not
set this flag: it is set and tested by vhost.c.
There seems to be no reason to check this:
caller in virtio net uses its own flag,
vhost_started, to ensure vhost is started/stopped
as appropriate.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/net/vhost_net.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index f87c798..d9423d1 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -195,10 +195,6 @@ static int vhost_net_start_one(struct vhost_net *net,
     struct vhost_vring_file file = { };
     int r;
 
-    if (net->dev.started) {
-        return 0;
-    }
-
     net->dev.nvqs = 2;
     net->dev.vqs = net->vqs;
     net->dev.vq_index = vq_index;
@@ -251,15 +247,11 @@ fail_notifiers:
     return r;
 }
 
 static void vhost_net_stop_one(struct vhost_net *net,
                                VirtIODevice *dev)
 {
     struct vhost_vring_file file = { .fd = -1 };
 
-    if (!net->dev.started) {
-        return;
-    }
-
     if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_TAP) {
         for (file.index = 0; file.index < net->dev.nvqs; ++file.index) {
             const VhostOps *vhost_ops = net->dev.vhost_ops;
-- 
MST

Re: [Qemu-devel] [PATCH] vhost_net: cleanup start/stop condition

[Amos Kong]

On Mon, Aug 04, 2014 at 02:12:39PM +0200, Michael S. Tsirkin wrote:
> Checking vhost device internal state in vhost_net looks like
> a layering violation since vhost_net does not
> set this flag: it is set and tested by vhost.c.
> There seems to be no reason to check this:
> caller in virtio net uses its own flag,
> vhost_started, to ensure vhost is started/stopped
> as appropriate.


Reasonable, we already checked n->vhost_started flag in virtio-net.c
when vhost_net_start() is called.

Reviewed-by: Amos Kong <akong@redhat.com>
 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  hw/net/vhost_net.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index f87c798..d9423d1 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -195,10 +195,6 @@ static int vhost_net_start_one(struct vhost_net *net,
>      struct vhost_vring_file file = { };
>      int r;
>  
> -    if (net->dev.started) {
> -        return 0;
> -    }
> -
>      net->dev.nvqs = 2;
>      net->dev.vqs = net->vqs;
>      net->dev.vq_index = vq_index;
> @@ -251,15 +247,11 @@ fail_notifiers:
>      return r;
>  }
>  
>  static void vhost_net_stop_one(struct vhost_net *net,
>                                 VirtIODevice *dev)
>  {
>      struct vhost_vring_file file = { .fd = -1 };
>  
> -    if (!net->dev.started) {
> -        return;
> -    }
> -
>      if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_TAP) {
>          for (file.index = 0; file.index < net->dev.nvqs; ++file.index) {
>              const VhostOps *vhost_ops = net->dev.vhost_ops;
> -- 
> MST

-- 
			Amos.