On Sat, Apr 27, 2013 at 01:11:16PM +0800, Jason Wang wrote:
> On 04/26/2013 08:26 PM, Michael S. Tsirkin wrote:
> > 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 <m...@redhat.com>
> >> Signed-off-by: Jason Wang <jasow...@redhat.com>
> > Hmm okay, but now that I think about this,
> > e.g. if link is up later, vhost will not be started.
>
> If vm has been stopeed, and the link is up later, vhost won't be
> started. this is expected.
> If vm has been started, and the link is up later, since n->vhost_started
> is false but both virtio_net_started() and !nc->peer->link_down is true,
> so the vhost will be started.
>
> Looks ok?
Let me clarify: virtio link is up but peer link is down.
So guest will send packets. Will they never be
completed?
> > 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
