Re: [Qemu-devel] [PATCH] net: prevent sending packets while guest is stopped

[Michael S. Tsirkin]

On Tue, Sep 02, 2014 at 03:23:10PM +0100, Stefan Hajnoczi wrote:
> Do not modify guest memory or devices when the guest is stopped.
> Currently the netdevs still send packets while the guest is stopped if
> their file descriptor was being monitored for write (e.g. the socket
> write buffer filled before the guest was stopped).
> 
> Netdevs call qemu_flush_queued_packets() when the file descriptor
> becomes writable again.  Don't resume packet processing when this
> happens.
> 
> Instead we flush queues when the guest resumes.
> 
> Cc: address@hidden
> Reported-by: Michael S. Tsirkin <address@hidden>
> Signed-off-by: Stefan Hajnoczi <address@hidden>

Reviewed-by: Michael S. Tsirkin <address@hidden>


> ---
> Note this fixes the transmit side.  The receive side was recently fixed in
> "net: Forbid dealing with packets when VM is not running".
> 
>  net/net.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/net/net.c b/net/net.c
> index 6d930ea..74ec07a 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -47,6 +47,7 @@
>  # define CONFIG_NET_BRIDGE
>  #endif
>  
> +static VMChangeStateEntry *net_change_state_entry;
>  static QTAILQ_HEAD(, NetClientState) net_clients;
>  
>  const char *host_net_devices[] = {
> @@ -506,6 +507,11 @@ void qemu_purge_queued_packets(NetClientState *nc)
>  
>  void qemu_flush_queued_packets(NetClientState *nc)
>  {
> +    /* Guest memory and devices must not be modified while stopped */
> +    if (!runstate_is_running()) {
> +        return;
> +    }
> +
>      nc->receive_disabled = 0;
>  
>      if (nc->peer && nc->peer->info->type == NET_CLIENT_OPTIONS_KIND_HUBPORT) 
> {
> @@ -1168,6 +1174,25 @@ void qmp_set_link(const char *name, bool up, Error 
> **errp)
>      }
>  }
>  
> +/* Kick net clients when guest resumes.  If a file descriptor was monitored 
> for
> + * writing before the guest was stopped, there will be nothing monitoring it
> + * right now so a kick is required to get packets flowing again.
> + */
> +static void net_vm_change_state_handler(void *opaque, int running,
> +                                        RunState state)
> +{
> +    NetClientState *nc;
> +    NetClientState *tmp;
> +
> +    if (!running) {
> +        return;
> +    }
> +
> +    QTAILQ_FOREACH_SAFE(nc, &net_clients, next, tmp) {
> +        qemu_flush_queued_packets(nc);
> +    }
> +}
> +
>  void net_cleanup(void)
>  {
>      NetClientState *nc;
> @@ -1183,6 +1208,8 @@ void net_cleanup(void)
>              qemu_del_net_client(nc);
>          }
>      }
> +
> +    qemu_del_vm_change_state_handler(net_change_state_entry);
>  }
>  
>  void net_check_clients(void)
> @@ -1268,6 +1295,9 @@ int net_init_clients(void)
>  #endif
>      }
>  
> +    net_change_state_entry =
> +        qemu_add_vm_change_state_handler(net_vm_change_state_handler, NULL);
> +
>      QTAILQ_INIT(&net_clients);
>  
>      if (qemu_opts_foreach(qemu_find_opts("netdev"), net_init_netdev, NULL, 
> 1) == -1)
> -- 
> 1.9.3