* [PATCH 1/3] util: WSAEWOULDBLOCK on connect should map to EINPROGRESS
2019-10-01 13:26 [PATCH 0/3] Some win32 fixes Marc-André Lureau
@ 2019-10-01 13:26 ` Marc-André Lureau
2019-10-01 13:56 ` Philippe Mathieu-Daudé
2019-10-01 13:26 ` [PATCH 2/3] tests: skip serial test on windows Marc-André Lureau
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Marc-André Lureau @ 2019-10-01 13:26 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, berrange, Marc-André Lureau, sw
In general, WSAEWOULDBLOCK can be mapped to EAGAIN as done by
socket_error() (or EWOULDBLOCK). But for connect() with non-blocking
sockets, it actually means the operation is in progress:
https://docs.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-connect
"The socket is marked as nonblocking and the connection cannot be completed immediately."
(this is also the behaviour implemented by GLib GSocket)
This fixes socket_can_bind_connect() test on win32.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
util/oslib-win32.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index c62cd4328c..886e400d6a 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -585,7 +585,11 @@ int qemu_connect_wrap(int sockfd, const struct sockaddr *addr,
int ret;
ret = connect(sockfd, addr, addrlen);
if (ret < 0) {
- errno = socket_error();
+ if (WSAGetLastError() == WSAEWOULDBLOCK) {
+ errno = EINPROGRESS;
+ } else {
+ errno = socket_error();
+ }
}
return ret;
}
--
2.23.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 1/3] util: WSAEWOULDBLOCK on connect should map to EINPROGRESS
2019-10-01 13:26 ` [PATCH 1/3] util: WSAEWOULDBLOCK on connect should map to EINPROGRESS Marc-André Lureau
@ 2019-10-01 13:56 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-10-01 13:56 UTC (permalink / raw)
To: Marc-André Lureau, qemu-devel; +Cc: pbonzini, berrange, sw
On 10/1/19 3:26 PM, Marc-André Lureau wrote:
> In general, WSAEWOULDBLOCK can be mapped to EAGAIN as done by
> socket_error() (or EWOULDBLOCK). But for connect() with non-blocking
> sockets, it actually means the operation is in progress:
>
> https://docs.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-connect
> "The socket is marked as nonblocking and the connection cannot be completed immediately."
>
> (this is also the behaviour implemented by GLib GSocket)
>
> This fixes socket_can_bind_connect() test on win32.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> util/oslib-win32.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> index c62cd4328c..886e400d6a 100644
> --- a/util/oslib-win32.c
> +++ b/util/oslib-win32.c
> @@ -585,7 +585,11 @@ int qemu_connect_wrap(int sockfd, const struct sockaddr *addr,
> int ret;
> ret = connect(sockfd, addr, addrlen);
> if (ret < 0) {
> - errno = socket_error();
> + if (WSAGetLastError() == WSAEWOULDBLOCK) {
> + errno = EINPROGRESS;
> + } else {
> + errno = socket_error();
> + }
> }
> return ret;
> }
>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/3] tests: skip serial test on windows
2019-10-01 13:26 [PATCH 0/3] Some win32 fixes Marc-André Lureau
2019-10-01 13:26 ` [PATCH 1/3] util: WSAEWOULDBLOCK on connect should map to EINPROGRESS Marc-André Lureau
@ 2019-10-01 13:26 ` Marc-André Lureau
2019-10-01 13:48 ` Philippe Mathieu-Daudé
2019-10-01 13:26 ` [PATCH 3/3] win32: fix main-loop busy loop on socket/fd event Marc-André Lureau
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Marc-André Lureau @ 2019-10-01 13:26 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, berrange, Marc-André Lureau, sw
Serial test is currently hard-coded to /dev/null.
On Windows, serial chardev expect a COM: device, which may not be
availble.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
tests/test-char.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/test-char.c b/tests/test-char.c
index d62de1b088..45e42af290 100644
--- a/tests/test-char.c
+++ b/tests/test-char.c
@@ -1103,7 +1103,7 @@ static void char_socket_server_two_clients_test(gconstpointer opaque)
}
-#ifdef HAVE_CHARDEV_SERIAL
+#if defined(HAVE_CHARDEV_SERIAL) && !defined(WIN32)
static void char_serial_test(void)
{
QemuOpts *opts;
@@ -1460,7 +1460,7 @@ int main(int argc, char **argv)
#endif
g_test_add_func("/char/udp", char_udp_test);
-#ifdef HAVE_CHARDEV_SERIAL
+#if defined(HAVE_CHARDEV_SERIAL) && !defined(WIN32)
g_test_add_func("/char/serial", char_serial_test);
#endif
g_test_add_func("/char/hotswap", char_hotswap_test);
--
2.23.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 2/3] tests: skip serial test on windows
2019-10-01 13:26 ` [PATCH 2/3] tests: skip serial test on windows Marc-André Lureau
@ 2019-10-01 13:48 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-10-01 13:48 UTC (permalink / raw)
To: Marc-André Lureau, qemu-devel; +Cc: pbonzini, berrange, sw
On 10/1/19 3:26 PM, Marc-André Lureau wrote:
> Serial test is currently hard-coded to /dev/null.
>
> On Windows, serial chardev expect a COM: device, which may not be
> availble.
"available"
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> tests/test-char.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/test-char.c b/tests/test-char.c
> index d62de1b088..45e42af290 100644
> --- a/tests/test-char.c
> +++ b/tests/test-char.c
> @@ -1103,7 +1103,7 @@ static void char_socket_server_two_clients_test(gconstpointer opaque)
> }
>
>
> -#ifdef HAVE_CHARDEV_SERIAL
> +#if defined(HAVE_CHARDEV_SERIAL) && !defined(WIN32)
> static void char_serial_test(void)
> {
> QemuOpts *opts;
> @@ -1460,7 +1460,7 @@ int main(int argc, char **argv)
> #endif
>
> g_test_add_func("/char/udp", char_udp_test);
> -#ifdef HAVE_CHARDEV_SERIAL
> +#if defined(HAVE_CHARDEV_SERIAL) && !defined(WIN32)
> g_test_add_func("/char/serial", char_serial_test);
> #endif
> g_test_add_func("/char/hotswap", char_hotswap_test);
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/3] win32: fix main-loop busy loop on socket/fd event
2019-10-01 13:26 [PATCH 0/3] Some win32 fixes Marc-André Lureau
2019-10-01 13:26 ` [PATCH 1/3] util: WSAEWOULDBLOCK on connect should map to EINPROGRESS Marc-André Lureau
2019-10-01 13:26 ` [PATCH 2/3] tests: skip serial test on windows Marc-André Lureau
@ 2019-10-01 13:26 ` Marc-André Lureau
2019-10-01 21:20 ` James Le Cuirot
2019-10-01 13:39 ` [PATCH 0/3] Some win32 fixes Paolo Bonzini
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Marc-André Lureau @ 2019-10-01 13:26 UTC (permalink / raw)
To: qemu-devel
Cc: pbonzini, James Le Cuirot, berrange, Marc-André Lureau, sw
Commit 05e514b1d4d5bd4209e2c8bbc76ff05c85a235f3 introduced an AIO
context optimization to avoid calling event_notifier_test_and_clear() on
ctx->notifier. On Windows, the same notifier is being used to wakeup the
wait on socket events (see commit
d3385eb448e38f828c78f8f68ec5d79c66a58b5d).
The ctx->notifier event is added to the gpoll sources in
aio_set_event_notifier(), aio_ctx_check() should clear the event
regardless of ctx->notified, since Windows sets the event by itself,
bypassing the aio->notified. This fixes qemu not clearing the event
resulting in a busy loop.
Paolo suggested to me on irc to call event_notifier_test_and_clear()
after select() >0 from aio-win32.c's aio_prepare. Unfortunately, not all
fds associated with ctx->notifiers are in AIO fd handlers set.
(qemu_set_nonblock() in util/oslib-win32.c calls qemu_fd_register()).
This is essentially a v2 of a patch that was sent earlier:
https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg00420.html
that resurfaced when James investigated Spice performance issues on Windows:
https://gitlab.freedesktop.org/spice/spice/issues/36
In order to test that patch, I simply tried running test-char on
win32, and it hangs. Applying that patch solves it. QIO idle sources
are not dispatched. I haven't investigated much further, I suspect
source priorities and busy looping still come into play.
This version keeps the "notified" field, so event_notifier_poll()
should still work as expected.
Cc: James Le Cuirot <chewi@gentoo.org>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
util/async.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/util/async.c b/util/async.c
index 4e4c7af51e..ca83e32c7f 100644
--- a/util/async.c
+++ b/util/async.c
@@ -354,7 +354,11 @@ void aio_notify(AioContext *ctx)
void aio_notify_accept(AioContext *ctx)
{
- if (atomic_xchg(&ctx->notified, false)) {
+ if (atomic_xchg(&ctx->notified, false)
+#ifdef WIN32
+ || true
+#endif
+ ) {
event_notifier_test_and_clear(&ctx->notifier);
}
}
--
2.23.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 3/3] win32: fix main-loop busy loop on socket/fd event
2019-10-01 13:26 ` [PATCH 3/3] win32: fix main-loop busy loop on socket/fd event Marc-André Lureau
@ 2019-10-01 21:20 ` James Le Cuirot
0 siblings, 0 replies; 13+ messages in thread
From: James Le Cuirot @ 2019-10-01 21:20 UTC (permalink / raw)
To: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2588 bytes --]
On Tue, 1 Oct 2019 17:26:09 +0400
Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
> Commit 05e514b1d4d5bd4209e2c8bbc76ff05c85a235f3 introduced an AIO
> context optimization to avoid calling event_notifier_test_and_clear() on
> ctx->notifier. On Windows, the same notifier is being used to wakeup the
> wait on socket events (see commit
> d3385eb448e38f828c78f8f68ec5d79c66a58b5d).
>
> The ctx->notifier event is added to the gpoll sources in
> aio_set_event_notifier(), aio_ctx_check() should clear the event
> regardless of ctx->notified, since Windows sets the event by itself,
> bypassing the aio->notified. This fixes qemu not clearing the event
> resulting in a busy loop.
>
> Paolo suggested to me on irc to call event_notifier_test_and_clear()
> after select() >0 from aio-win32.c's aio_prepare. Unfortunately, not all
> fds associated with ctx->notifiers are in AIO fd handlers set.
> (qemu_set_nonblock() in util/oslib-win32.c calls qemu_fd_register()).
>
> This is essentially a v2 of a patch that was sent earlier:
> https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg00420.html
>
> that resurfaced when James investigated Spice performance issues on Windows:
> https://gitlab.freedesktop.org/spice/spice/issues/36
>
> In order to test that patch, I simply tried running test-char on
> win32, and it hangs. Applying that patch solves it. QIO idle sources
> are not dispatched. I haven't investigated much further, I suspect
> source priorities and busy looping still come into play.
>
> This version keeps the "notified" field, so event_notifier_poll()
> should still work as expected.
>
> Cc: James Le Cuirot <chewi@gentoo.org>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> util/async.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/util/async.c b/util/async.c
> index 4e4c7af51e..ca83e32c7f 100644
> --- a/util/async.c
> +++ b/util/async.c
> @@ -354,7 +354,11 @@ void aio_notify(AioContext *ctx)
>
> void aio_notify_accept(AioContext *ctx)
> {
> - if (atomic_xchg(&ctx->notified, false)) {
> + if (atomic_xchg(&ctx->notified, false)
> +#ifdef WIN32
> + || true
> +#endif
> + ) {
> event_notifier_test_and_clear(&ctx->notifier);
> }
> }
I can confirm that this updated patch fixes my performance issue. The
idle CPU usage drops from around 35% to around 2%. Moving the mouse now
makes the usage go up, not down. :) Many thanks!
Regards,
--
James Le Cuirot (chewi)
Gentoo Linux Developer
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] Some win32 fixes
2019-10-01 13:26 [PATCH 0/3] Some win32 fixes Marc-André Lureau
` (2 preceding siblings ...)
2019-10-01 13:26 ` [PATCH 3/3] win32: fix main-loop busy loop on socket/fd event Marc-André Lureau
@ 2019-10-01 13:39 ` Paolo Bonzini
2019-10-01 13:44 ` Peter Maydell
2019-10-01 14:03 ` Alex Bennée
2019-10-01 14:11 ` Thomas Huth
5 siblings, 1 reply; 13+ messages in thread
From: Paolo Bonzini @ 2019-10-01 13:39 UTC (permalink / raw)
To: Marc-André Lureau, qemu-devel; +Cc: sw, berrange
On 01/10/19 15:26, Marc-André Lureau wrote:
> Hi,
>
> Here are a few patches that should fix some busy looping issues
> already reported >2y ago
> (https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg00420.html),
> and fixing test-char on win32.
Queued thanks (with s/fix/work around/ in the commit subject of
patch 3).
> hmm, do we have any automated testing/CI on Windows (beside just
> cross-compilation)?
Nope. :(
Paolo
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 0/3] Some win32 fixes
2019-10-01 13:39 ` [PATCH 0/3] Some win32 fixes Paolo Bonzini
@ 2019-10-01 13:44 ` Peter Maydell
2019-10-01 13:54 ` Philippe Mathieu-Daudé
2019-10-01 14:43 ` Stefan Weil
0 siblings, 2 replies; 13+ messages in thread
From: Peter Maydell @ 2019-10-01 13:44 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Marc-André Lureau, Daniel P. Berrange, QEMU Developers,
Stefan Weil
On Tue, 1 Oct 2019 at 14:43, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 01/10/19 15:26, Marc-André Lureau wrote:
> > hmm, do we have any automated testing/CI on Windows (beside just
> > cross-compilation)?
>
> Nope. :(
I did wonder whether it would be possible to get 'make check'
passing under Wine, but never investigated...
thanks
-- PMM
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] Some win32 fixes
2019-10-01 13:44 ` Peter Maydell
@ 2019-10-01 13:54 ` Philippe Mathieu-Daudé
2019-10-01 14:43 ` Stefan Weil
1 sibling, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-10-01 13:54 UTC (permalink / raw)
To: Peter Maydell, Paolo Bonzini, Nelson Chen
Cc: Marc-André Lureau, Daniel P. Berrange, QEMU Developers,
Stefan Weil
On 10/1/19 3:44 PM, Peter Maydell wrote:
> On Tue, 1 Oct 2019 at 14:43, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> On 01/10/19 15:26, Marc-André Lureau wrote:
>>> hmm, do we have any automated testing/CI on Windows (beside just
>>> cross-compilation)?
>>
>> Nope. :(
>
> I did wonder whether it would be possible to get 'make check'
> passing under Wine, but never investigated...
Nelson Chen started to use Azure CI:
https://github.com/nelsonjchen/nc-qemu/blob/master/.azure/msys2-build.sh
He posted some notes here:
https://mindflakes.com/projects/nc-qemu/
I have this link in my CI TODO to try to upstream it and run 'make
check' but it is not a high priority.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] Some win32 fixes
2019-10-01 13:44 ` Peter Maydell
2019-10-01 13:54 ` Philippe Mathieu-Daudé
@ 2019-10-01 14:43 ` Stefan Weil
1 sibling, 0 replies; 13+ messages in thread
From: Stefan Weil @ 2019-10-01 14:43 UTC (permalink / raw)
To: Peter Maydell, Paolo Bonzini
Cc: Marc-André Lureau, Daniel P. Berrange, QEMU Developers
Am 01.10.2019 um 15:44 schrieb Peter Maydell:
> On Tue, 1 Oct 2019 at 14:43, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> On 01/10/19 15:26, Marc-André Lureau wrote:
>>> hmm, do we have any automated testing/CI on Windows (beside just
>>> cross-compilation)?
>> Nope. :(
> I did wonder whether it would be possible to get 'make check'
> passing under Wine, but never investigated...
>
> thanks
> -- PMM
Running "make check" under Wine had worked for me (well, at least
partially) in the past. Of course it is not a full replacement for tests
under Windows, but it should be sufficient to cover most of the
functionality. Usually I also use Wine to examine bug reports for Windows.
Are there test servers with Wine available?
Regards,
Stefan Weil
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] Some win32 fixes
2019-10-01 13:26 [PATCH 0/3] Some win32 fixes Marc-André Lureau
` (3 preceding siblings ...)
2019-10-01 13:39 ` [PATCH 0/3] Some win32 fixes Paolo Bonzini
@ 2019-10-01 14:03 ` Alex Bennée
2019-10-01 14:11 ` Thomas Huth
5 siblings, 0 replies; 13+ messages in thread
From: Alex Bennée @ 2019-10-01 14:03 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, sw, berrange, Marc-André Lureau
Marc-André Lureau <marcandre.lureau@redhat.com> writes:
> Hi,
>
> Here are a few patches that should fix some busy looping issues
> already reported >2y ago
> (https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg00420.html),
> and fixing test-char on win32.
>
> hmm, do we have any automated testing/CI on Windows (beside just
> cross-compilation)?
Not that I'm aware of. I did briefly look at CircleCI but never beyond
playing with my own experiments. Ultimately I find it hard to care about
Windows as I don't have a copy myself. Maybe someone else does care and
would like to explore further?
FWIW I believe Travis has Windows support now and there is also
AppVeyor. It's probably worth spreading the love around given how loaded
our Travis jobs are.
>
> thanks
>
> Marc-André Lureau (3):
> util: WSAEWOULDBLOCK on connect should map to EINPROGRESS
> tests: skip serial test on windows
> win32: fix main-loop busy loop on socket/fd event
>
> tests/test-char.c | 4 ++--
> util/async.c | 6 +++++-
> util/oslib-win32.c | 6 +++++-
> 3 files changed, 12 insertions(+), 4 deletions(-)
--
Alex Bennée
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 0/3] Some win32 fixes
2019-10-01 13:26 [PATCH 0/3] Some win32 fixes Marc-André Lureau
` (4 preceding siblings ...)
2019-10-01 14:03 ` Alex Bennée
@ 2019-10-01 14:11 ` Thomas Huth
5 siblings, 0 replies; 13+ messages in thread
From: Thomas Huth @ 2019-10-01 14:11 UTC (permalink / raw)
To: Marc-André Lureau, qemu-devel; +Cc: pbonzini, berrange, sw
On 01/10/2019 15.26, Marc-André Lureau wrote:
> Hi,
>
> Here are a few patches that should fix some busy looping issues
> already reported >2y ago
> (https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg00420.html),
> and fixing test-char on win32.
>
> hmm, do we have any automated testing/CI on Windows (beside just
> cross-compilation)?
AFAIK we do not have any automated CI on Windows yet. But seems like
both, Travis and Cirrus, have Windows support, so you could add a test
to our .travis.yml or .cirrus.yml file, I think.
Thomas
^ permalink raw reply [flat|nested] 13+ messages in thread