From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:47254) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RIUvr-0003q2-L1 for qemu-devel@nongnu.org; Mon, 24 Oct 2011 20:31:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RIUvq-0002bk-8Z for qemu-devel@nongnu.org; Mon, 24 Oct 2011 20:31:55 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48395) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RIUvp-0002az-Vs for qemu-devel@nongnu.org; Mon, 24 Oct 2011 20:31:54 -0400 Date: Mon, 24 Oct 2011 22:31:48 -0200 From: Luiz Capitulino Message-ID: <20111024223148.47b0dec7@doriath> In-Reply-To: <20111024172937.GC12513@bow.tlv.redhat.com> References: <1319457739-14562-1-git-send-email-alevy@redhat.com> <1319457739-14562-2-git-send-email-alevy@redhat.com> <4EA5808A.7050409@redhat.com> <20111024134516.57596735@doriath> <20111024172937.GC12513@bow.tlv.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/5] monitor: screen_dump async List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alon Levy Cc: qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com, mlureau@redhat.com, Gerd Hoffmann , armbru@redhat.com On Mon, 24 Oct 2011 19:29:37 +0200 Alon Levy wrote: > On Mon, Oct 24, 2011 at 01:45:16PM -0200, Luiz Capitulino wrote: > > On Mon, 24 Oct 2011 17:13:14 +0200 > > Gerd Hoffmann wrote: > > > > > On 10/24/11 14:02, Alon Levy wrote: > > > > Make screen_dump monitor command an async command to allow next for qxl > > > > to implement it as a initiating call to red_worker and completion on > > > > callback, to fix a deadlock when issueing a screendump command via > > > > libvirt while connected with a libvirt controlled spice-gtk client. > > > > > > Approach looks reasonable to me. Patch breaks the build though, you've > > > missed a bunch of screen_dump functions in non-x86 targets. > > > > There are two problems actually. > > > > The first one is that changing an existing command from synchronous > > to asynchronous is an incompatible change because asynchronous commands > > semantics is different. For an example of possible problems please > > check: https://bugzilla.redhat.com/show_bug.cgi?id=623903. > > > > The second problem is that the existing asynchronous interface in the > > monitor is incomplete and has never been used for real. Our plan is to > > use QAPI's async support, but that has not landed in master yet and iirc > > there wasn't consensus about it. I also think it's a bit late for its > > inclusion in 1.0 (and certainly not a candidate for stable). > > > > If all you need here is to delay sending the response, then maybe the > > current interface could work (although I honestly don't trust it and > > regret not having dropped it). Otherwise our only choice would be to > > work on getting the QAPI async support merged. > > My problem is that the io thread keeps the global mutex during the wait, > that's why the async monitor is perfect for what I want - this is > exactly what it does. Let's not mix internal implementation details with what we want as an external interface. Can't you just make a vga_hw_screen_dump() specific callback? > I haven't looked at QAPI async support, but I > understand it's a bit in the future. Yes, it's not for the immediate term.