Re: [Qemu-devel] [PATCH] tap: use an exit notifier to call down_script

[Jason Wang <jasowang@redhat.com>]

On 2016年07月11日 22:48, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> We would like to move back net_cleanup() at the end of main function,
> like it used to be until f30dbae63a46f23116715dff8d130c, but minimum
> tap cleanup is necessary regarless at exit() time. Use an exit notifier
> to call TAP down_script. If net_cleanup() is called first, then remove
> the exit notifier as it will become a dangling pointer otherwise.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>

This looks like a tap specific solution. But at least slirp wants to do 
some cleanup too. We need a generic solution. Does it work if we use 
atexit(net_chr_cleanup) and first cleanup net and then chr at that function?

> ---
>   net/tap.c | 27 ++++++++++++++++++++-------
>   1 file changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/net/tap.c b/net/tap.c
> index 676bad4..e9c32f3 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -58,6 +58,7 @@ typedef struct TAPState {
>       bool enabled;
>       VHostNetState *vhost_net;
>       unsigned host_vnet_hdr_len;
> +    Notifier exit;
>   } TAPState;
>   
>   static void launch_script(const char *setup_script, const char *ifname,
> @@ -292,10 +293,22 @@ static void tap_set_offload(NetClientState *nc, int csum, int tso4,
>       tap_fd_set_offload(s->fd, csum, tso4, tso6, ecn, ufo);
>   }
>   
> +static void tap_exit_notify(Notifier *notifier, void *data)
> +{
> +    TAPState *s = container_of(notifier, TAPState, exit);
> +    Error *err = NULL;
> +
> +    if (s->down_script[0]) {
> +        launch_script(s->down_script, s->down_script_arg, s->fd, &err);
> +        if (err) {
> +            error_report_err(err);
> +        }
> +    }
> +}
> +
>   static void tap_cleanup(NetClientState *nc)
>   {
>       TAPState *s = DO_UPCAST(TAPState, nc, nc);
> -    Error *err = NULL;
>   
>       if (s->vhost_net) {
>           vhost_net_cleanup(s->vhost_net);
> @@ -304,12 +317,8 @@ static void tap_cleanup(NetClientState *nc)
>   
>       qemu_purge_queued_packets(nc);
>   
> -    if (s->down_script[0]) {
> -        launch_script(s->down_script, s->down_script_arg, s->fd, &err);
> -        if (err) {
> -            error_report_err(err);
> -        }
> -    }
> +    tap_exit_notify(&s->exit, NULL);
> +    qemu_remove_exit_notifier(&s->exit);
>   
>       tap_read_poll(s, false);
>       tap_write_poll(s, false);
> @@ -379,6 +388,10 @@ static TAPState *net_tap_fd_init(NetClientState *peer,
>       }
>       tap_read_poll(s, true);
>       s->vhost_net = NULL;
> +
> +    s->exit.notify = tap_exit_notify;
> +    qemu_add_exit_notifier(&s->exit);
> +
>       return s;
>   }
>