From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:60371) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R0DI5-0002Jq-B8 for qemu-devel@nongnu.org; Sun, 04 Sep 2011 10:03:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R0DI4-0005hf-6y for qemu-devel@nongnu.org; Sun, 04 Sep 2011 10:03:17 -0400 Received: from mx1.redhat.com ([209.132.183.28]:64576) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R0DI3-0005ha-Uu for qemu-devel@nongnu.org; Sun, 04 Sep 2011 10:03:16 -0400 Message-ID: <4E63851F.3070205@redhat.com> Date: Sun, 04 Sep 2011 17:03:11 +0300 From: Avi Kivity 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> In-Reply-To: <1314018774-27482-2-git-send-email-aliguori@us.ibm.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: Anthony Liguori Cc: Blue Swirl , Paolo Bonzini , qemu-devel@nongnu.org, Kevin Wolf 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. > > 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) -- error compiling committee.c: too many arguments to function