[Qemu-devel] [PATCH] slirp: use exit notifier for slirp_smb_cleanup

[Qemu-devel] [PATCH] slirp: use exit notifier for slirp_smb_cleanup

[Paolo Bonzini]

We would like to move back net_cleanup() at the end of main function,
like it used to be until f30dbae63a46f23116715dff8d130c, but minimum
cleanup is needed regardless at exit() time for slirp's SMB
functionality.  Use an exit notifier to call slirp_smb_cleanup.
If net_cleanup() is called first, then remove the exit notifier as it
will become a dangling pointer otherwise.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 net/slirp.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/net/slirp.c b/net/slirp.c
index 31630f0..28207b6 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -38,6 +38,7 @@
 #include "slirp/libslirp.h"
 #include "slirp/ip6.h"
 #include "sysemu/char.h"
+#include "sysemu/sysemu.h"
 #include "qemu/cutils.h"
 
 static int get_str_sep(char *buf, int buf_size, const char **pp, int sep)
@@ -76,6 +77,7 @@ typedef struct SlirpState {
     NetClientState nc;
     QTAILQ_ENTRY(SlirpState) entry;
     Slirp *slirp;
+    Notifier exit_notifier;
 #ifndef _WIN32
     char smb_dir[128];
 #endif
@@ -118,11 +120,18 @@ static ssize_t net_slirp_receive(NetClientState *nc, const uint8_t *buf, size_t
     return size;
 }
 
+static void slirp_smb_exit(Notifier *n, void *data)
+{
+    SlirpState *s = container_of(n, SlirpState, exit_notifier);
+    slirp_smb_cleanup(s);
+}
+
 static void net_slirp_cleanup(NetClientState *nc)
 {
     SlirpState *s = DO_UPCAST(SlirpState, nc, nc);
 
     slirp_cleanup(s->slirp);
+    qemu_remove_exit_notifier(&s->exit_notifier);
     slirp_smb_cleanup(s);
     QTAILQ_REMOVE(&slirp_stacks, s, entry);
 }
@@ -349,6 +358,8 @@ static int net_slirp_init(NetClientState *peer, const char *model,
     }
 #endif
 
+    s->exit_notifier.notify = slirp_smb_exit;
+    qemu_add_exit_notifier(&s->exit_notifier);
     return 0;
 
 error:
-- 
2.7.4

Re: [Qemu-devel] [PATCH] slirp: use exit notifier for slirp_smb_cleanup

[Marc-André Lureau]

Hi

On Tue, Jul 12, 2016 at 11:23 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> We would like to move back net_cleanup() at the end of main function,
> like it used to be until f30dbae63a46f23116715dff8d130c, but minimum
> cleanup is needed regardless at exit() time for slirp's SMB
> functionality.  Use an exit notifier to call slirp_smb_cleanup.
> If net_cleanup() is called first, then remove the exit notifier as it
> will become a dangling pointer otherwise.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>



> ---
>  net/slirp.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/net/slirp.c b/net/slirp.c
> index 31630f0..28207b6 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -38,6 +38,7 @@
>  #include "slirp/libslirp.h"
>  #include "slirp/ip6.h"
>  #include "sysemu/char.h"
> +#include "sysemu/sysemu.h"
>  #include "qemu/cutils.h"
>
>  static int get_str_sep(char *buf, int buf_size, const char **pp, int sep)
> @@ -76,6 +77,7 @@ typedef struct SlirpState {
>      NetClientState nc;
>      QTAILQ_ENTRY(SlirpState) entry;
>      Slirp *slirp;
> +    Notifier exit_notifier;

we may want to rename the notifier in my patch too, for consistency
(you did that already, probably)

>  #ifndef _WIN32
>      char smb_dir[128];
>  #endif
> @@ -118,11 +120,18 @@ static ssize_t net_slirp_receive(NetClientState *nc, const uint8_t *buf, size_t
>      return size;
>  }
>
> +static void slirp_smb_exit(Notifier *n, void *data)
> +{
> +    SlirpState *s = container_of(n, SlirpState, exit_notifier);
> +    slirp_smb_cleanup(s);
> +}
> +
>  static void net_slirp_cleanup(NetClientState *nc)
>  {
>      SlirpState *s = DO_UPCAST(SlirpState, nc, nc);
>
>      slirp_cleanup(s->slirp);
> +    qemu_remove_exit_notifier(&s->exit_notifier);
>      slirp_smb_cleanup(s);
>      QTAILQ_REMOVE(&slirp_stacks, s, entry);
>  }
> @@ -349,6 +358,8 @@ static int net_slirp_init(NetClientState *peer, const char *model,
>      }
>  #endif
>
> +    s->exit_notifier.notify = slirp_smb_exit;
> +    qemu_add_exit_notifier(&s->exit_notifier);
>      return 0;
>
>  error:
> --
> 2.7.4
>
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH] slirp: use exit notifier for slirp_smb_cleanup

[Jason Wang]

On 2016年07月12日 18:15, Marc-André Lureau wrote:
> Hi
>
> On Tue, Jul 12, 2016 at 11:23 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> We would like to move back net_cleanup() at the end of main function,
>> like it used to be until f30dbae63a46f23116715dff8d130c, but minimum
>> cleanup is needed regardless at exit() time for slirp's SMB
>> functionality.  Use an exit notifier to call slirp_smb_cleanup.
>> If net_cleanup() is called first, then remove the exit notifier as it
>> will become a dangling pointer otherwise.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Reviewed-by: Jason Wang <jasowang@redhat.com>

>
>
>
>> ---
>>   net/slirp.c | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/net/slirp.c b/net/slirp.c
>> index 31630f0..28207b6 100644
>> --- a/net/slirp.c
>> +++ b/net/slirp.c
>> @@ -38,6 +38,7 @@
>>   #include "slirp/libslirp.h"
>>   #include "slirp/ip6.h"
>>   #include "sysemu/char.h"
>> +#include "sysemu/sysemu.h"
>>   #include "qemu/cutils.h"
>>
>>   static int get_str_sep(char *buf, int buf_size, const char **pp, int sep)
>> @@ -76,6 +77,7 @@ typedef struct SlirpState {
>>       NetClientState nc;
>>       QTAILQ_ENTRY(SlirpState) entry;
>>       Slirp *slirp;
>> +    Notifier exit_notifier;
> we may want to rename the notifier in my patch too, for consistency
> (you did that already, probably)
>
>>   #ifndef _WIN32
>>       char smb_dir[128];
>>   #endif
>> @@ -118,11 +120,18 @@ static ssize_t net_slirp_receive(NetClientState *nc, const uint8_t *buf, size_t
>>       return size;
>>   }
>>
>> +static void slirp_smb_exit(Notifier *n, void *data)
>> +{
>> +    SlirpState *s = container_of(n, SlirpState, exit_notifier);
>> +    slirp_smb_cleanup(s);
>> +}
>> +
>>   static void net_slirp_cleanup(NetClientState *nc)
>>   {
>>       SlirpState *s = DO_UPCAST(SlirpState, nc, nc);
>>
>>       slirp_cleanup(s->slirp);
>> +    qemu_remove_exit_notifier(&s->exit_notifier);
>>       slirp_smb_cleanup(s);
>>       QTAILQ_REMOVE(&slirp_stacks, s, entry);
>>   }
>> @@ -349,6 +358,8 @@ static int net_slirp_init(NetClientState *peer, const char *model,
>>       }
>>   #endif
>>
>> +    s->exit_notifier.notify = slirp_smb_exit;
>> +    qemu_add_exit_notifier(&s->exit_notifier);
>>       return 0;
>>
>>   error:
>> --
>> 2.7.4
>>
>>
>
>

Re: [Qemu-devel] [PATCH] slirp: use exit notifier for slirp_smb_cleanup

[Paolo Bonzini]

On 13/07/2016 04:34, Jason Wang wrote:
> 
> 
> On 2016年07月12日 18:15, Marc-André Lureau wrote:
>> Hi
>>
>> On Tue, Jul 12, 2016 at 11:23 AM, Paolo Bonzini <pbonzini@redhat.com>
>> wrote:
>>> We would like to move back net_cleanup() at the end of main function,
>>> like it used to be until f30dbae63a46f23116715dff8d130c, but minimum
>>> cleanup is needed regardless at exit() time for slirp's SMB
>>> functionality.  Use an exit notifier to call slirp_smb_cleanup.
>>> If net_cleanup() is called first, then remove the exit notifier as it
>>> will become a dangling pointer otherwise.
>>>
>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Reviewed-by: Jason Wang <jasowang@redhat.com>

Thanks!

Paolo