qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: qemu-devel@nongnu.org
Cc: anthony@codemonkey.ws, laurent@vivier.eu, kvm@vger.kernel.org
Subject: [Qemu-devel] [RFC PATCH] fix select(2) race between main_loop_wait and qemu_aio_wait
Date: Mon,  5 Mar 2012 09:34:15 +0100	[thread overview]
Message-ID: <1330936455-23802-1-git-send-email-pbonzini@redhat.com> (raw)

This is quite ugly.  Two threads, one running main_loop_wait and
one running qemu_aio_wait, can race with each other on running the
same iohandler.  The result is that an iohandler could run while the
underlying socket is not readable or writable, with possibly ill effects.

This shows as a failure to boot an IDE disk using the NBD device.
We can consider it a bug in NBD or in the main loop.  The patch fixes
this in main_loop_wait, which is always going to lose the race because
qemu_aio_wait runs select with the global lock held.

Reported-by: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
	Anthony, if you think this is too ugly tell me and I can
	post an NBD fix too.

 main-loop.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/main-loop.c b/main-loop.c
index db23de0..3beccff 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -458,6 +458,13 @@ int main_loop_wait(int nonblocking)
 
     if (timeout > 0) {
         qemu_mutex_lock_iothread();
+
+        /* Poll again.  A qemu_aio_wait() on another thread
+         * could have made the fdsets stale.
+         */
+        tv.tv_sec = 0;
+        tv.tv_usec = 0;
+        ret = select(nfds + 1, &rfds, &wfds, &xfds, &tv);
     }
 
     glib_select_poll(&rfds, &wfds, &xfds, (ret < 0));
-- 
1.7.7.6

             reply	other threads:[~2012-03-05  8:34 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-05  8:34 Paolo Bonzini [this message]
2012-03-05  9:07 ` [Qemu-devel] [RFC PATCH] fix select(2) race between main_loop_wait and qemu_aio_wait Jan Kiszka
2012-03-05  9:25   ` Paolo Bonzini
2012-03-05 14:24   ` Avi Kivity
2012-03-05 14:30     ` Paolo Bonzini
2012-03-05 15:14       ` Avi Kivity
2012-03-05 16:14         ` Paolo Bonzini
2012-03-05 17:35           ` Avi Kivity
2012-03-06  9:01             ` Paolo Bonzini
2012-03-05 14:30     ` Jan Kiszka
2012-03-05 17:39       ` Avi Kivity
2012-03-05 17:55         ` Jan Kiszka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1330936455-23802-1-git-send-email-pbonzini@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=anthony@codemonkey.ws \
    --cc=kvm@vger.kernel.org \
    --cc=laurent@vivier.eu \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).