[Qemu-devel] [PATCH] net: clean up network at qemu process termination

[Qemu-devel] [PATCH] net: clean up network at qemu process termination

[Amos Kong]

We don't clean up network if fails to parse "-device" parameters without
calling net_cleanup(). I touch a problem, the tap device which is
created by qemu-ifup script could not be removed by qemu-ifdown script.
Some similar problems also exist in vl.c

In this patch, if network initialization successes, a cleanup function
will be registered to be called at qemu process termination.

Signed-off-by: Amos Kong <akong@redhat.com>
---
 vl.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/vl.c b/vl.c
index a3ab384..842f987 100644
--- a/vl.c
+++ b/vl.c
@@ -3749,6 +3749,9 @@ int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
+    /* clean up network at qemu process termination */
+    atexit(&net_cleanup);
+
     /* init the bluetooth world */
     if (foreach_device_config(DEV_BT, bt_parse))
         exit(1);
@@ -3999,7 +4002,6 @@ int main(int argc, char **argv, char **envp)
     main_loop();
     bdrv_close_all();
     pause_all_vcpus();
-    net_cleanup();
     res_free();
 
     return 0;
-- 
1.7.1

Re: [Qemu-devel] [PATCH] net: clean up network at qemu process termination

[Michael S. Tsirkin]

On Tue, Dec 11, 2012 at 09:45:54PM +0800, Amos Kong wrote:
> We don't clean up network if fails to parse "-device" parameters without
> calling net_cleanup(). I touch a problem, the tap device which is
> created by qemu-ifup script could not be removed by qemu-ifdown script.
> Some similar problems also exist in vl.c
> 
> In this patch, if network initialization successes, a cleanup function
> will be registered to be called at qemu process termination.
> 
> Signed-off-by: Amos Kong <akong@redhat.com>


If there are multiple net clients, this still leaves
the problem where one client is initialized,
the second one fails, and we exit without ifdown.

I think registering before net_init_clients
will fix this case but please do test.

> ---
>  vl.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index a3ab384..842f987 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3749,6 +3749,9 @@ int main(int argc, char **argv, char **envp)
>          exit(1);
>      }
>  
> +    /* clean up network at qemu process termination */
> +    atexit(&net_cleanup);
> +
>      /* init the bluetooth world */
>      if (foreach_device_config(DEV_BT, bt_parse))
>          exit(1);
> @@ -3999,7 +4002,6 @@ int main(int argc, char **argv, char **envp)
>      main_loop();
>      bdrv_close_all();
>      pause_all_vcpus();
> -    net_cleanup();
>      res_free();
>  
>      return 0;
> -- 
> 1.7.1

[Qemu-devel] [PATCH v2] net: clean up network at qemu process termination

[Amos Kong]

We don't clean up network if fails to parse "-device" parameters without
calling net_cleanup(). I touch a problem, the tap device which is
created by qemu-ifup script could not be removed by qemu-ifdown script.
Some similar problems also exist in vl.c

In this patch, if network initialization successes, a cleanup function
will be registered to be called at qemu process termination.

Signed-off-by: Amos Kong <akong@redhat.com>
---
v2: register cleanup function before network initialization
---
 vl.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/vl.c b/vl.c
index a3ab384..3bd773e 100644
--- a/vl.c
+++ b/vl.c
@@ -3745,6 +3745,9 @@ int main(int argc, char **argv, char **envp)
     }
     configure_icount(icount_option);
 
+    /* clean up network at qemu process termination */
+    atexit(&net_cleanup);
+
     if (net_init_clients() < 0) {
         exit(1);
     }
@@ -3999,7 +4002,6 @@ int main(int argc, char **argv, char **envp)
     main_loop();
     bdrv_close_all();
     pause_all_vcpus();
-    net_cleanup();
     res_free();
 
     return 0;
-- 
1.7.1

Re: [Qemu-devel] [PATCH v2] net: clean up network at qemu process termination

[Michael S. Tsirkin]

On Tue, Dec 11, 2012 at 10:20:15PM +0800, Amos Kong wrote:
> We don't clean up network if fails to parse "-device" parameters without
> calling net_cleanup(). I touch a problem, the tap device which is
> created by qemu-ifup script could not be removed by qemu-ifdown script.
> Some similar problems also exist in vl.c
> 
> In this patch, if network initialization successes, a cleanup function
> will be registered to be called at qemu process termination.
> 
> Signed-off-by: Amos Kong <akong@redhat.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
> v2: register cleanup function before network initialization
> ---
>  vl.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index a3ab384..3bd773e 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3745,6 +3745,9 @@ int main(int argc, char **argv, char **envp)
>      }
>      configure_icount(icount_option);
>  
> +    /* clean up network at qemu process termination */
> +    atexit(&net_cleanup);
> +
>      if (net_init_clients() < 0) {
>          exit(1);
>      }
> @@ -3999,7 +4002,6 @@ int main(int argc, char **argv, char **envp)
>      main_loop();
>      bdrv_close_all();
>      pause_all_vcpus();
> -    net_cleanup();
>      res_free();
>  
>      return 0;
> -- 
> 1.7.1

Re: [Qemu-devel] [PATCH v2] net: clean up network at qemu process termination

[Stefan Hajnoczi]

On Tue, Dec 11, 2012 at 10:20:15PM +0800, Amos Kong wrote:
> We don't clean up network if fails to parse "-device" parameters without
> calling net_cleanup(). I touch a problem, the tap device which is
> created by qemu-ifup script could not be removed by qemu-ifdown script.
> Some similar problems also exist in vl.c
> 
> In this patch, if network initialization successes, a cleanup function
> will be registered to be called at qemu process termination.
> 
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
> v2: register cleanup function before network initialization
> ---
>  vl.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)

Thanks, applied to the net tree:
https://github.com/stefanha/qemu/commits/net

Stefan