On 07/10/2015 05:24 PM, Fam Zheng wrote: > On Wed, 07/08 17:40, Jason Wang wrote: >> >> On 07/07/2015 05:03 PM, Fam Zheng wrote: >>> On Tue, 07/07 15:44, Jason Wang wrote: >>>> On 07/07/2015 09:21 AM, Fam Zheng wrote: >>>>> Since commit 6e99c63 "net/socket: Drop net_socket_can_send" and friends, >>>>> net queues need to be explicitly flushed after qemu_can_send_packet() >>>>> returns false, because the netdev side will disable the polling of fd. >>>>> >>>>> This fixes the case of "cont" after "stop" (or migration). >>>>> >>>>> Signed-off-by: Fam Zheng <f...@redhat.com> >>>>> >>>>> --- >>>>> >>>>> v2: Unify with VM stop handler. (Stefan) >>>>> --- >>>>> net/net.c | 19 ++++++++++++------- >>>>> 1 file changed, 12 insertions(+), 7 deletions(-) >>>>> >>>>> diff --git a/net/net.c b/net/net.c >>>>> index 6ff7fec..28a5597 100644 >>>>> --- a/net/net.c >>>>> +++ b/net/net.c >>>>> @@ -1257,14 +1257,19 @@ void qmp_set_link(const char *name, bool up, >>>>> Error **errp) >>>>> static void net_vm_change_state_handler(void *opaque, int running, >>>>> RunState state) >>>>> { >>>>> - /* Complete all queued packets, to guarantee we don't modify >>>>> - * state later when VM is not running. >>>>> - */ >>>>> - if (!running) { >>>>> - NetClientState *nc; >>>>> - NetClientState *tmp; >>>>> + NetClientState *nc; >>>>> + NetClientState *tmp; >>>>> >>>>> - QTAILQ_FOREACH_SAFE(nc, &net_clients, next, tmp) { >>>>> + QTAILQ_FOREACH_SAFE(nc, &net_clients, next, tmp) { >>>>> + if (running) { >>>>> + /* Flush queued packets and wake up backends. */ >>>>> + if (nc->peer && qemu_can_send_packet(nc)) { >>>>> + qemu_flush_queued_packets(nc->peer); >>>>> + } >>>>> + } else { >>>>> + /* Complete all queued packets, to guarantee we don't modify >>>>> + * state later when VM is not running. >>>>> + */ >>>>> qemu_flush_or_purge_queued_packets(nc, true); >>>>> } >>>> Looks like qemu_can_send_packet() checks both nc->peer and runstate. So >>>> probably, we can simplify this to: >>>> >>>> if (qemu_can_send_packet(nc)) >>>> qemu_flush_queued_packets(nc->peer); >>>> else >>>> qemu_flush_or_purge_queued_packets(nc, true); >>>> >>>>> } >>> qemu_can_send_packet returns 1 if !nc->peer, so this doesn't work. >>> >>> Fam >> Yes, I was wrong. >> >> Btw, instead of depending on vm handler (which seems racy with other >> state change handler). Can we do this in places like vm_start() and >> vm_stop(). Like we drain and flush block queue during vm stop. >> > Because that's a bit hacky. > > It won't be racy if we replace vdev->vm_running with runstate_is_running() in > virtio-net, and/or don't check it in virtio_net_can_receive(). Is that OK?
Ok but not necessary. I believe this patch will be used with the patch that drops virtio_net_can_receive(). So at any order two vmstate handlers were called, the sent_cb will be called and poll will be enabled again. I'm ok with the patch. So Reviewed-by: Jason Wang <jasow...@redhat.com> Thanks > > Fam