Re: [Qemu-devel] [PATCH] net: Forbid dealing with packets when VM is not running

[zhanghailiang]

On 2014/8/19 20:29, Stefan Hajnoczi wrote:
> On Mon, Aug 18, 2014 at 04:32:42PM +0800, zhanghailiang wrote:
> > On 2014/8/18 14:55, Jason Wang wrote:
> > > On 08/18/2014 12:46 PM, zhanghailiang wrote:
> > > > diff --git a/net/net.c b/net/net.c
> > > > index 6d930ea..21f0d48 100644
> > > > --- a/net/net.c
> > > > +++ b/net/net.c
> > > > @@ -242,6 +242,29 @@ NetClientState *qemu_new_net_client(NetClientInfo *info,
> > > >       return nc;
> > > >   }
> > > >
> > > > +static void nic_vmstate_change_handler(void *opaque,
> > > > +                                       int running,
> > > > +                                       RunState state)
> > > > +{
> > > > +    NICState *nic = opaque;
> > > > +    NetClientState *nc;
> > > > +    int i, queues;
> > > > +
> > > > +    if (!running) {
> > > > +        return;
> > > > +    }
> > > > +
> > > > +    queues = MAX(1, nic->conf->peers.queues);
> > > > +    for (i = 0; i<   queues; i++) {
> > > > +        nc =&nic->ncs[i];
> > > > +        if (nc->receive_disabled
> > > > +            || (nc->info->can_receive&&   !nc->info->can_receive(nc))) {
> > > > +            continue;
> > > > +        }
> > > > +        qemu_flush_queued_packets(nc);
> > >
> > > How about simply purge the receive queue during stop? If ok, there's no
> > > need to introduce extra vmstate change handler.
> >
> > I don't know whether it is OK to purge the receive packages, it was
> > suggested by Stefan Hajnoczi, and i am waiting for his opinion .:)
> >
> > I think we still need the extra vmstate change handler, Without the
> > change handler, we don't know if the VM will go to stop and the time
> > when to call qemu_purge_queued_packets.
>
> qemu_flush_queued_packets() sets nc->received_disabled = 0.  This may be
> needed to get packets flowing again if ->receive() previously returned 0.
>
> Purging the queue does not clear nc->received_disabled so it is not
> enough.

So this is the reason why we need to do flush after VM runstate come 
back to running again.:)

Oh, In the above patch, we don't need check the nc->receive_disabled
before qemu_flush_queued_packets(nc), but still need check
nc->info->can_receive(nc), is it?

Before send another patch, I will check if this patch has side-effect 
when we use vhost_net.

Thanks,
zhanghailiang