From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Mw08g-0005rz-3N for qemu-devel@nongnu.org; Thu, 08 Oct 2009 17:03:06 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Mw08b-0005lr-Af for qemu-devel@nongnu.org; Thu, 08 Oct 2009 17:03:05 -0400 Received: from [199.232.76.173] (port=49397 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Mw08b-0005l1-0w for qemu-devel@nongnu.org; Thu, 08 Oct 2009 17:03:01 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46271) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Mw08Z-0006kF-Mn for qemu-devel@nongnu.org; Thu, 08 Oct 2009 17:03:00 -0400 Date: Thu, 8 Oct 2009 23:00:37 +0200 From: "Michael S. Tsirkin" Message-ID: <20091008210037.GA7182@redhat.com> References: <20091008203740.GA20727@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20091008203740.GA20727@redhat.com> Subject: [Qemu-devel] Re: [PATCH] qemu: work around for "posix-aio-compat" List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: malc , qemu-devel@nongnu.org, anthony@codemonkey.ws On Thu, Oct 08, 2009 at 10:37:40PM +0200, Michael S. Tsirkin wrote: > With commit ee3993069ff55fa6f1c64daf1e09963e340db8e4, > "posix-aio-compat: avoid signal race when spawning a thread" > winxp installation on a raw format file fails > during disk format, with a message "your > disk may be damaged". > > This commit moved signal mask from aio thread to creating thread. > It turns out if we keep the mask in aio thread as well, the problem > disappears. It should not be needed, but since this is harmless, let's > keep it around until someone inclined to debug pthread library internals > can check this issue. > > While we are at it, convert sigprocmask to pthread_sigmask > as per posix. > > Signed-off-by: Michael S. Tsirkin > > > For the benefit of whoever tries to debug this any deeper here's > documentation on how to reproduce the issue to be saved in git logs > (need to tweak paths obviously): > > The test was done on FC11, 32 bit userspace, 64 bit kernel 2.6.31. Vanilla kernel from FC11 does the same btw. > > ./configure --prefix=/scm/qemu-kvm-debug --target-list=x86_64-softmmu \ > --enable-kvm > make -j 8 && make install > rm /scm/images/winxp-bisect.raw > > /scm/qemu-kvm-debug/bin/qemu-img create -f raw \ > /scm/images/winxp-bisect.raw 2G > > /scm/qemu-kvm-debug/bin/qemu-system-x86_64 -enable-kvm -cdrom \ > /home/mst/en_windows_xp_professional_with_service_pack_3_x86_cd_x14-80428.iso \ > /scm/images/winxp-bisect.raw > > Select format to NTFS. > --- > > OK, I'm out of time with this issue, let's apply a work-around > until someone interested comes around? > > posix-aio-compat.c | 14 ++++++++++++-- > > diff --git a/posix-aio-compat.c b/posix-aio-compat.c > index 400d898..4abbe3a 100644 > --- a/posix-aio-compat.c > +++ b/posix-aio-compat.c > @@ -301,6 +301,16 @@ static size_t handle_aiocb_rw(struct qemu_paiocb *aiocb) > static void *aio_thread(void *unused) > { > pid_t pid; > + sigset_t set; > + > + /* block all signals */ > + /* Should not be necessary as we should inherit mask > + * from creating thread. However, without this, > + * on FC11, using raw file, WinXP installation fails during disk format > + * saying disk was damaged. pthread library bug? > + * */ > + if (sigfillset(&set)) die("sigfillset"); > + if (pthread_sigmask(SIG_BLOCK, &set, NULL)) die("pthread_sigmask"); By the way, things seem to work even when I comment out pthread_sigmask: just sigfillset does it. Seems pretty wild. > > pid = getpid(); > > @@ -371,11 +381,11 @@ static void spawn_thread(void) > > /* block all signals */ > if (sigfillset(&set)) die("sigfillset"); > - if (sigprocmask(SIG_SETMASK, &set, &oldset)) die("sigprocmask"); > + if (pthread_sigmask(SIG_SETMASK, &set, &oldset)) die("pthread_sigmask"); > > thread_create(&thread_id, &attr, aio_thread, NULL); > > - if (sigprocmask(SIG_SETMASK, &oldset, NULL)) die("sigprocmask restore"); > + if (pthread_sigmask(SIG_SETMASK, &oldset, NULL)) die("pthread_sigmask restore"); > } As I said before, just this second hunk alone does not help. > static void qemu_paio_submit(struct qemu_paiocb *aiocb) > -- > 1.6.5.rc2