From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:48034) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UFqls-0008UW-Gy for qemu-devel@nongnu.org; Wed, 13 Mar 2013 14:51:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UFqlq-0008WM-Dj for qemu-devel@nongnu.org; Wed, 13 Mar 2013 14:51:28 -0400 Received: from mail-oa0-f47.google.com ([209.85.219.47]:64872) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UFqlq-0008Vu-7t for qemu-devel@nongnu.org; Wed, 13 Mar 2013 14:51:26 -0400 Received: by mail-oa0-f47.google.com with SMTP id o17so1484730oag.34 for ; Wed, 13 Mar 2013 11:51:25 -0700 (PDT) From: Anthony Liguori In-Reply-To: <1447652481.7556290.1363198114363.JavaMail.root@redhat.com> References: <1447652481.7556290.1363198114363.JavaMail.root@redhat.com> Date: Wed, 13 Mar 2013 13:51:06 -0500 Message-ID: <878v5r3zhh.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [RFC PATCH 0/2] port network layer onto glib List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: mdroth , Stefan Hajnoczi , "Michael S. Tsirkin" , Liu Ping Fan , qemu-devel@nongnu.org Paolo Bonzini writes: >> 1) It has no facility for timer events > > Yup, it's on the todo list. > >> 2) It's tied to file descriptors (only a problem for win32) > > The other way round: it's not tied to file descriptors for win32, > which is already a problem for e.g. networked backends. main-loop.c > has the code that is needed, but it's low on the todo list. Okay, I'll change this to: 2) It doesn't have a cross-platform API for file descriptor/handle events. If we tried to use AioContext for networking, the result would be that we'd break win32. That's the point I was trying to make. > Note that tap-win32 for example doesn't need this. Because tap-win32 doesn't share any code with tap :-/ This is unfortunate really. >> 3) The fd support is tied to read/write without an extensibility >> mechanism for other fd events > > Yes. Though internally it uses g_poll, so it's "just" a matter > of defining the right API. Ack especially the "just" part :-) > >> 4) It's got no mechanism to interact with signals > > signalfd? (GSource doesn't have any such mechanism directly). What I was referring to was the GChildWatch support. I don't think we make extensive use of SIGCHLD today though so perhaps that's a minor point. > >> 5) It's not obvious how we would integrate with polling-based >> callbacks like we have in the character layer and networking layer. > > Isn't io_flush one such mechanism? Right now it applies to both > io_read and io_write, but really it is never used for io_write. Sort of but it's not extensible. GSource really gets this right from an abstraction point of view. It was pretty straight forward to convert the character layer by just writing a GSource that allowed polling. > Also, this and (3) might be the same problem. I agree. I think it needs a GSource-like abstraction which is why I've been arguing to just copy glib's main loop routines out-right. They've solved this problem already :-) >> So I agree it's simple but I don't think it can reasonably stay simple. >> I think if we added all of the above, the best we would expect to end >> up with is something that looked like glib. >> >> As it stands, the lack of (5) would make it extremely difficult to >> convert the networking layer. > > Quite possible, I've never looked very much at the networking layer. There's another thing that argues in your favor--or at least for !glib directly. I think we want to make the main loops first-class QOM objects. Right now all we support is 1-thread per dataplane but I'm positive we're going to want to be able to group devices on threads. Being able to express AioContexts as properties and even having a command line interface to create threads woudl be really nice. Something like: qemu -object io-thread,id=thread0 \ -device virtio-blk,queue[0]=thread0 \ -device virtio-net,rx[0]=thread0,tx=thread0 Likewise, from a libvirt PoV, being able to do: qom-get /threads/thread0.tid Is a real nice touch for pinning purposes. Regards, Anthony Liguori > > Paolo > >> Regards, >> >> Anthony Liguori >> >> > >> >>> and AioContext's code is vastly simpler than GMainLoop's. >> >> >> >> For now. >> > >> > Fair enough. :) >> > >> >>> AioContext is also documented and unit tested, with tests >> >>> for both standalone and GSource operation. Unit tests for >> >>> AioContext >> >>> users are trivial to write, we have one in test-thread-pool. >> >>> >> >>>> Did you have a specific concern with using glib vs. AioContext? >> >>>> Is it >> >>>> about reusing code in the block layer where AioContext is >> >>>> required? >> >>> >> >>> In the short term yes, code duplication is a concern. We already >> >>> have >> >>> two implementation of virtio. >> >> >> >> I share your concern but in the opposite direction. We have three >> >> main >> >> loops today. >> > >> > Yes, and two of them (main-loop.c/qemu-timer.c and async.c) can be >> > merged. >> > >> >>> I would like the dataplane virtio code to >> >>> grow everything else that needs to be in all dataplane-style >> >>> devices >> >>> (for example, things such as setting up the guest<->host >> >>> notifiers), and >> >>> the hw/virtio.c API implemented on top of it (or dead >> >>> altogether). >> >>> Usage of AioContext is pretty much forced by the block layer. >> >> >> >> I don't think that AioContext is the right answer because it makes >> >> it >> >> too easy to shoot yourself in the foot. >> > >> > See above, if nesting is the problem it's gone. >> > >> > Paolo >>