Re: [Qemu-devel] [PATCH] virtio-net: properly check the vhost status during status set

[Michael S. Tsirkin]

On Fri, Apr 26, 2013 at 06:27:40PM +0800, Jason Wang wrote:
> Commit 32993698 (vhost: disable on tap link down) tries to disable the vhost
> also when the peer's link is down. But the check was not done properly, the
> vhost were only started when:
> 
> 1) peer's link is not down
> 2) virtio-net has already been started.
> 
> Since == have a higher precedence than &&, place a brace to make sure both the
> conditions were met then does the check. This fixes the crash when doing a 
> savem
> after set the link off which let qemu crash and complains:
> 
> virtio_net_save: Assertion `!n->vhost_started' failed.
> 
> Cc: Michael S. Tsirkin <address@hidden>
> Signed-off-by: Jason Wang <address@hidden>

Hmm okay, but now that I think about this,
e.g. if link is up later, vhost will not be started.

So the correct thing is maybe to start vhost but use
some backend that will drop all packets.
And add a callback so we know peer state changed.
Hmm do we need a kernel change for this?

> ---
>  hw/net/virtio-net.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 4d2cdd2..6222039 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -114,8 +114,8 @@ static void virtio_net_vhost_status(VirtIONet *n, uint8_t 
> status)
>          return;
>      }
>  
> -    if (!!n->vhost_started == virtio_net_started(n, status) &&
> -                              !nc->peer->link_down) {
> +    if (!!n->vhost_started ==
> +        (virtio_net_started(n, status) && !nc->peer->link_down)) {
>          return;
>      }
>      if (!n->vhost_started) {
> -- 
> 1.7.1