From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:58120) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qoxis-0008F1-1j for qemu-devel@nongnu.org; Thu, 04 Aug 2011 09:12:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Qoxir-0000NP-0A for qemu-devel@nongnu.org; Thu, 04 Aug 2011 09:12:25 -0400 Received: from e9.ny.us.ibm.com ([32.97.182.139]:58092) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qoxiq-0000NL-RK for qemu-devel@nongnu.org; Thu, 04 Aug 2011 09:12:24 -0400 Received: from d01relay03.pok.ibm.com (d01relay03.pok.ibm.com [9.56.227.235]) by e9.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id p74CdPUw028459 for ; Thu, 4 Aug 2011 08:39:26 -0400 Received: from d03av02.boulder.ibm.com (d03av02.boulder.ibm.com [9.17.195.168]) by d01relay03.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p74DCGLl185236 for ; Thu, 4 Aug 2011 09:12:17 -0400 Received: from d03av02.boulder.ibm.com (loopback [127.0.0.1]) by d03av02.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p747AvPe005348 for ; Thu, 4 Aug 2011 01:10:57 -0600 Message-ID: <4E3A9A7B.6040205@us.ibm.com> Date: Thu, 04 Aug 2011 08:11:23 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1312208590-25502-1-git-send-email-aliguori@us.ibm.com> <20110804064533.GB23922@amit-x200.redhat.com> In-Reply-To: <20110804064533.GB23922@amit-x200.redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 00/12][RFC] char: add flow control and fix guest_[open|close] List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Amit Shah Cc: Hans de Goede , qemu-devel@nongnu.org On 08/04/2011 01:45 AM, Amit Shah wrote: > On (Mon) 01 Aug 2011 [09:22:58], Anthony Liguori wrote: >> The char layer has been growing some nasty warts for some time now as we ask it >> to do things it was never intended on doing. It's been long over due for an >> overhaul and its become evident to me that we need to address this first before >> adding any more features to the char layer. >> >> This series is the start at sanitizing the char layer. It effectively turns >> the char layer into an internal pipe. It supports flow control using an >> intermediate ring queue for each direction. > > Let's break down what we have now: > > * chardev -> guest (backend writes): > we have connect/disconnect notifications, we have an ugly > can_read() implementation that is executed each time iohandlers are > run. However, it gives us reasonable flow control. It has one major flaw. It assumes that the backends can implement polling to determine when the front end can read. This makes it impossible to implement a backend that isn't associated with a file descriptor (like a QMP server as required by the guest agent). > * guest -> chardev (frontend writes): > we don't get chardev connect/disconnect events, neither do we get > to know if the chardev is overwhelmed with data and to stop sending > any more till it has some room in its queue. We do have connect/disconnect event--that's open/close. > This is because we > need poll semantics instead of select to get POLLIN and POLLOUT > events, using which we can ascertain what state the chardev is in. > There's no call corresponding to the existing qemu_chr_can_read(), > which essentially confirms if a chardev is able to handle data for > output. This series only adds a qemu_char_fe_can_write(), doesn't > add callbacks for connect/disconnect events since that depends on > polling. There are already events for connect/disconnect so I'm not sure what you're talking about. > > The problem I think with adding a buffer is it just solves the flow > control problem without solving the connect/disconnect events issue by > just switching to poll, I don't understand at all what you're saying here :-) Hopefully you'd agree, that from a design perspective, the closer a chrdrv looks to a normal unix socket, the more correct it is. After this series, we have: 1) read/write methods that behave like unix read/write (except zero indicates EAGAIN, not EOF). 2) OPENED/CLOSE events that map to accept/EOF 3) edge events for readability that semantically map to epoll() I think that's pretty complete. I don't see anything that's missing. > and solving all the problems at once. Is > there something that we solve by having a buffer in addition to > poll()? > > Also, introducing a buffer doesn't really solve all the problems; just > shifts them into the char layer instead of letting the OS handle the > queues naturally. What does "naturally" mean? Regards, Anthony Liguori > > Amit