* Re: [Qemu-devel] [PATCH] Added cleanup for Win32 TAP interface
[not found] <12545.9562209018$1363177481@news.gmane.org>
@ 2013-04-15 15:55 ` Paolo Bonzini
2013-04-15 18:32 ` Stefan Weil
0 siblings, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2013-04-15 15:55 UTC (permalink / raw)
To: Pavel Dovgaluk; +Cc: Stefan Weil, 'qemu-devel'
Il 13/03/2013 13:23, Pavel Dovgaluk ha scritto:
> Added cleanup for Win32 TAP interface.
>
> Signed-off-by: Pavel Dovgalyuk<pavel.dovgaluk@gmail.com>
Stefan, did this slip?
Paolo
> ---
> net/tap-win32.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/net/tap-win32.c b/net/tap-win32.c
> index 91e9e84..1c1176c 100644
> --- a/net/tap-win32.c
> +++ b/net/tap-win32.c
> @@ -99,6 +99,7 @@ typedef struct tap_win32_overlapped {
> HANDLE output_queue_semaphore;
> HANDLE free_list_semaphore;
> HANDLE tap_semaphore;
> + HANDLE hThread;
> CRITICAL_SECTION output_queue_cs;
> CRITICAL_SECTION free_list_cs;
> OVERLAPPED read_overlapped;
> @@ -625,7 +626,7 @@ static int tap_win32_open(tap_win32_overlapped_t **phandle,
>
> *phandle = &tap_overlapped;
>
> - CreateThread(NULL, 0, tap_win32_thread_entry,
> + tap_overlapped.hThread = CreateThread(NULL, 0, tap_win32_thread_entry,
> (LPVOID)&tap_overlapped, 0, &idThread);
> return 0;
> }
> @@ -643,9 +644,8 @@ static void tap_cleanup(NetClientState *nc)
>
> qemu_del_wait_object(s->handle->tap_semaphore, NULL, NULL);
>
> - /* FIXME: need to kill thread and close file handle:
> - tap_win32_close(s);
> - */
> + TerminateThread(s->handle->hThread, 0);
> + CloseHandle(s->handle->handle);
> }
>
> static ssize_t tap_receive(NetClientState *nc, const uint8_t *buf, size_t size)
>
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] Added cleanup for Win32 TAP interface
2013-04-15 15:55 ` [Qemu-devel] [PATCH] Added cleanup for Win32 TAP interface Paolo Bonzini
@ 2013-04-15 18:32 ` Stefan Weil
2013-04-16 7:43 ` Pavel Dovgaluk
2013-04-16 7:47 ` Stefan Hajnoczi
0 siblings, 2 replies; 6+ messages in thread
From: Stefan Weil @ 2013-04-15 18:32 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Stefan Hajnoczi, 'qemu-devel', Pavel Dovgaluk
Am 15.04.2013 17:55, schrieb Paolo Bonzini:
> Il 13/03/2013 13:23, Pavel Dovgaluk ha scritto:
>> Added cleanup for Win32 TAP interface. Signed-off-by: Pavel
>> Dovgalyuk<pavel.dovgaluk@gmail.com>
> Stefan, did this slip? Paolo
Yes, I had not noticed this patch before. But there was a comment
from Stefan H. which is still unanswered. See
https://lists.gnu.org/archive/html/qemu-devel/2013-03/msg02185.html
Pavel, please have a look on that mail and send an update of your patch.
Please cc me for w32/w64 related patches.
Regards,
Stefan W.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] Added cleanup for Win32 TAP interface
2013-04-15 18:32 ` Stefan Weil
@ 2013-04-16 7:43 ` Pavel Dovgaluk
2013-04-16 7:47 ` Stefan Hajnoczi
1 sibling, 0 replies; 6+ messages in thread
From: Pavel Dovgaluk @ 2013-04-16 7:43 UTC (permalink / raw)
To: 'Stefan Weil', 'Paolo Bonzini'
Cc: 'qemu-devel', 'Stefan Hajnoczi'
Hello!
> From: Stefan Weil [mailto:sw@weilnetz.de]
> Am 15.04.2013 17:55, schrieb Paolo Bonzini:
> > Il 13/03/2013 13:23, Pavel Dovgaluk ha scritto:
> >> Added cleanup for Win32 TAP interface. Signed-off-by: Pavel
> >> Dovgalyuk<pavel.dovgaluk@gmail.com>
> > Stefan, did this slip? Paolo
>
> Yes, I had not noticed this patch before. But there was a comment
> from Stefan H. which is still unanswered. See
>
> https://lists.gnu.org/archive/html/qemu-devel/2013-03/msg02185.html
>
> Pavel, please have a look on that mail and send an update of your patch.
> Please cc me for w32/w64 related patches.
I've read this comment and I plan to rewrite the patch later,
but is not the highest priority task (as ROR bug).
I'll send you an update after finishing it.
Pavel Dovgaluk
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] Added cleanup for Win32 TAP interface
2013-04-15 18:32 ` Stefan Weil
2013-04-16 7:43 ` Pavel Dovgaluk
@ 2013-04-16 7:47 ` Stefan Hajnoczi
1 sibling, 0 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2013-04-16 7:47 UTC (permalink / raw)
To: Stefan Weil
Cc: Pavel Dovgaluk, Paolo Bonzini, 'qemu-devel',
Stefan Hajnoczi
On Mon, Apr 15, 2013 at 08:32:06PM +0200, Stefan Weil wrote:
> Am 15.04.2013 17:55, schrieb Paolo Bonzini:
> >Il 13/03/2013 13:23, Pavel Dovgaluk ha scritto:
> >>Added cleanup for Win32 TAP interface. Signed-off-by: Pavel
> >>Dovgalyuk<pavel.dovgaluk@gmail.com>
> >Stefan, did this slip? Paolo
>
>
> Yes, I had not noticed this patch before. But there was a comment
> from Stefan H. which is still unanswered. See
>
> https://lists.gnu.org/archive/html/qemu-devel/2013-03/msg02185.html
>
> Pavel, please have a look on that mail and send an update of your patch.
> Please cc me for w32/w64 related patches.
I reviewed the patch, replied (see Stefan Weil's link) and didn't see an
updated patch after that.
Stefan
^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <51407009.05c6e00a.355c.5275SMTPIN_ADDED_BROKEN@mx.google.com>]
* Re: [Qemu-devel] [PATCH] Added cleanup for Win32 TAP interface
[not found] <51407009.05c6e00a.355c.5275SMTPIN_ADDED_BROKEN@mx.google.com>
@ 2013-03-13 12:55 ` Stefan Hajnoczi
0 siblings, 0 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2013-03-13 12:55 UTC (permalink / raw)
To: Pavel Dovgaluk; +Cc: 'qemu-devel'
On Wed, Mar 13, 2013 at 04:23:52PM +0400, Pavel Dovgaluk wrote:
> Added cleanup for Win32 TAP interface.
>
> Signed-off-by: Pavel Dovgalyuk<pavel.dovgaluk@gmail.com>
> ---
> net/tap-win32.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/net/tap-win32.c b/net/tap-win32.c
> index 91e9e84..1c1176c 100644
> --- a/net/tap-win32.c
> +++ b/net/tap-win32.c
> @@ -99,6 +99,7 @@ typedef struct tap_win32_overlapped {
> HANDLE output_queue_semaphore;
> HANDLE free_list_semaphore;
> HANDLE tap_semaphore;
> + HANDLE hThread;
Please follow QEMU coding style: "thread" or "hthread" or "h_thread" but
not "hThread".
> CRITICAL_SECTION output_queue_cs;
> CRITICAL_SECTION free_list_cs;
> OVERLAPPED read_overlapped;
> @@ -625,7 +626,7 @@ static int tap_win32_open(tap_win32_overlapped_t **phandle,
>
> *phandle = &tap_overlapped;
>
> - CreateThread(NULL, 0, tap_win32_thread_entry,
> + tap_overlapped.hThread = CreateThread(NULL, 0, tap_win32_thread_entry,
> (LPVOID)&tap_overlapped, 0, &idThread);
> return 0;
> }
> @@ -643,9 +644,8 @@ static void tap_cleanup(NetClientState *nc)
>
> qemu_del_wait_object(s->handle->tap_semaphore, NULL, NULL);
>
> - /* FIXME: need to kill thread and close file handle:
> - tap_win32_close(s);
> - */
> + TerminateThread(s->handle->hThread, 0);
> + CloseHandle(s->handle->handle);
http://msdn.microsoft.com/en-us/library/windows/desktop/ms686717(v=vs.85).aspx
"Windows Server 2003 and Windows XP: The target thread's initial stack
is not freed, causing a resource leak."
"TerminateThread is a dangerous function that should only be used in the
most extreme cases. You should call TerminateThread only if you know
exactly what the target thread is doing, and you control all of the code
that the target thread could possibly be running at the time of the
termination."
Please terminate tap_win32_thread_entry() cleanly instead of using
TerminateThread(). Typically this means signalling that the thread
should stop and then waiting for the thread to exit by itself.
Stefan
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH] Added cleanup for Win32 TAP interface
@ 2013-03-13 12:23 Pavel Dovgaluk
0 siblings, 0 replies; 6+ messages in thread
From: Pavel Dovgaluk @ 2013-03-13 12:23 UTC (permalink / raw)
To: 'qemu-devel'
Added cleanup for Win32 TAP interface.
Signed-off-by: Pavel Dovgalyuk<pavel.dovgaluk@gmail.com>
---
net/tap-win32.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/tap-win32.c b/net/tap-win32.c
index 91e9e84..1c1176c 100644
--- a/net/tap-win32.c
+++ b/net/tap-win32.c
@@ -99,6 +99,7 @@ typedef struct tap_win32_overlapped {
HANDLE output_queue_semaphore;
HANDLE free_list_semaphore;
HANDLE tap_semaphore;
+ HANDLE hThread;
CRITICAL_SECTION output_queue_cs;
CRITICAL_SECTION free_list_cs;
OVERLAPPED read_overlapped;
@@ -625,7 +626,7 @@ static int tap_win32_open(tap_win32_overlapped_t **phandle,
*phandle = &tap_overlapped;
- CreateThread(NULL, 0, tap_win32_thread_entry,
+ tap_overlapped.hThread = CreateThread(NULL, 0, tap_win32_thread_entry,
(LPVOID)&tap_overlapped, 0, &idThread);
return 0;
}
@@ -643,9 +644,8 @@ static void tap_cleanup(NetClientState *nc)
qemu_del_wait_object(s->handle->tap_semaphore, NULL, NULL);
- /* FIXME: need to kill thread and close file handle:
- tap_win32_close(s);
- */
+ TerminateThread(s->handle->hThread, 0);
+ CloseHandle(s->handle->handle);
}
static ssize_t tap_receive(NetClientState *nc, const uint8_t *buf, size_t size)
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-04-16 7:47 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <12545.9562209018$1363177481@news.gmane.org>
2013-04-15 15:55 ` [Qemu-devel] [PATCH] Added cleanup for Win32 TAP interface Paolo Bonzini
2013-04-15 18:32 ` Stefan Weil
2013-04-16 7:43 ` Pavel Dovgaluk
2013-04-16 7:47 ` Stefan Hajnoczi
[not found] <51407009.05c6e00a.355c.5275SMTPIN_ADDED_BROKEN@mx.google.com>
2013-03-13 12:55 ` Stefan Hajnoczi
2013-03-13 12:23 Pavel Dovgaluk
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).