From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:52642) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R0ECe-0006E8-WF for qemu-devel@nongnu.org; Sun, 04 Sep 2011 11:01:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R0ECd-0006uL-Em for qemu-devel@nongnu.org; Sun, 04 Sep 2011 11:01:44 -0400 Received: from mail-yi0-f45.google.com ([209.85.218.45]:39971) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R0ECd-0006uF-8x for qemu-devel@nongnu.org; Sun, 04 Sep 2011 11:01:43 -0400 Received: by yih10 with SMTP id 10so3319188yih.4 for ; Sun, 04 Sep 2011 08:01:42 -0700 (PDT) Message-ID: <4E6392D4.5030905@codemonkey.ws> Date: Sun, 04 Sep 2011 10:01:40 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1314018774-27482-1-git-send-email-aliguori@us.ibm.com> <1314018774-27482-2-git-send-email-aliguori@us.ibm.com> <4E63851F.3070205@redhat.com> In-Reply-To: <4E63851F.3070205@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/2] main: switch qemu_set_fd_handler to g_io_add_watch List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Avi Kivity Cc: Blue Swirl , Paolo Bonzini , Anthony Liguori , qemu-devel@nongnu.org, Kevin Wolf On 09/04/2011 09:03 AM, Avi Kivity wrote: > On 08/22/2011 04:12 PM, Anthony Liguori wrote: >> This patch changes qemu_set_fd_handler to be implemented in terms of >> g_io_add_watch(). The semantics are a bit different so some glue is >> required. >> >> qemu_set_fd_handler2 is much harder to convert because of its use of >> polling. >> >> The glib main loop has the major of advantage of having a proven >> thread safe >> architecture. By using the glib main loop instead of our own, it will >> allow us >> to eventually introduce multiple I/O threads. >> >> I'm pretty sure that this will work on Win32, but I would appreciate >> some help >> testing. I think the semantics of g_io_channel_unix_new() are really >> just tied >> to the notion of a "unix fd" and not necessarily unix itself. > > 'git bisect' fingered this as responsible for breaking > qcow2+cache=unsafe. I think there's an off-by-one here and the guilty > patch is the one that switches the main loop, but that's just a guess. > > The symptoms are that a guest that is restarted (new qemu process) after > install doesn't make it through grub - some image data didn't make it do > disk. With qcow2 and cache=unsafe that can easily happen through exit > notifiers not being run and the entire qcow2 metadata being thrown out > the window. Running with raw+cache=unsafe works. Can you share your full command line? Nothing that would be in the obvious path actually uses qemu_set_fd_handler... Regards, Anthony Liguori > > >> >> diff --git a/iohandler.c b/iohandler.c >> index 4deae1e..5ef66fb 100644 >> --- a/iohandler.c >> +++ b/iohandler.c >> @@ -80,12 +80,67 @@ int qemu_set_fd_handler2(int fd, >> return 0; >> } >> >> +typedef struct IOTrampoline >> +{ >> + GIOChannel *chan; >> + IOHandler *fd_read; >> + IOHandler *fd_write; >> + void *opaque; >> + guint tag; >> +} IOTrampoline; >> + >> +static gboolean fd_trampoline(GIOChannel *chan, GIOCondition cond, >> gpointer opaque) >> +{ >> + IOTrampoline *tramp = opaque; >> + >> + if (tramp->opaque == NULL) { >> + return FALSE; >> + } >> + >> + if ((cond& G_IO_IN)&& tramp->fd_read) { >> + tramp->fd_read(tramp->opaque); >> + } >> + >> + if ((cond& G_IO_OUT)&& tramp->fd_write) { >> + tramp->fd_write(tramp->opaque); >> + } >> + >> + return TRUE; >> +} >> + >> int qemu_set_fd_handler(int fd, >> IOHandler *fd_read, >> IOHandler *fd_write, >> void *opaque) >> { >> - return qemu_set_fd_handler2(fd, NULL, fd_read, fd_write, opaque); >> + static IOTrampoline fd_trampolines[FD_SETSIZE]; >> + IOTrampoline *tramp =&fd_trampolines[fd]; >> + >> + if (tramp->tag != 0) { >> + g_io_channel_unref(tramp->chan); >> + g_source_remove(tramp->tag); >> + } >> + >> + if (opaque) { >> + GIOCondition cond = 0; >> + >> + tramp->fd_read = fd_read; >> + tramp->fd_write = fd_write; >> + tramp->opaque = opaque; >> + >> + if (fd_read) { >> + cond |= G_IO_IN | G_IO_ERR; >> + } >> + >> + if (fd_write) { >> + cond |= G_IO_OUT | G_IO_ERR; >> + } >> + >> + tramp->chan = g_io_channel_unix_new(fd); >> + tramp->tag = g_io_add_watch(tramp->chan, cond, fd_trampoline, tramp); >> + } >> + >> + return 0; >> } >> >> void qemu_iohandler_fill(int *pnfds, fd_set *readfds, fd_set >> *writefds, fd_set *xfds) > >