From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:43358) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S7U8A-0001Z0-KL for qemu-devel@nongnu.org; Tue, 13 Mar 2012 11:59:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S7U83-0003Dh-Rg for qemu-devel@nongnu.org; Tue, 13 Mar 2012 11:59:22 -0400 Received: from mx1.redhat.com ([209.132.183.28]:15390) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S7U83-0003C2-Jp for qemu-devel@nongnu.org; Tue, 13 Mar 2012 11:59:15 -0400 Date: Tue, 13 Mar 2012 12:59:17 -0300 From: Luiz Capitulino Message-ID: <20120313125917.2b03a70c@doriath.home> In-Reply-To: <20120313144514.GK27659@garlic.redhat.com> References: <1331483977-18910-1-git-send-email-alevy@redhat.com> <1331494004-26177-1-git-send-email-alevy@redhat.com> <1331494004-26177-5-git-send-email-alevy@redhat.com> <20120313103555.0ae4b834@doriath.home> <20120313144514.GK27659@garlic.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 4/5] console: pass Monitor to vga_hw_screen_dump/hw_vga_dump List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alon Levy Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org, kraxel@redhat.com On Tue, 13 Mar 2012 16:46:12 +0200 Alon Levy wrote: > On Tue, Mar 13, 2012 at 10:35:55AM -0300, Luiz Capitulino wrote: > > On Sun, 11 Mar 2012 21:26:43 +0200 > > Alon Levy wrote: > >=20 > > > Passes the Monitor ptr to the screendump implementation to all for > > > monitor suspend and resume for qxl to fix screendump regression. > > >=20 > > > graphics_console_init signature change required touching every > > > implemented of screen_dump. There is no change other then an added > > > parameter. qxl will make use of it in the next patch. > >=20 > > NACK on this one. > >=20 > > The Monitor object should be restricted to HMP. This patch spreads it to > > what's going to be the QMP implementation of screendump. > >=20 > > The first step here should be to convert the screendump command to the = qapi, > > and lock the HMP shell in hmp_screendump(). > >=20 > > However, this brings a new interesting problem: the HMP implementation = is > > actually a QMP client, meaning that it won't have a way to figure out > > screendump completion either :) > >=20 > > Some solutions that come to my mind: > >=20 > > 1. Pool the screendump file creation from a timer. > >=20 > > Cons: it may return before the file is fully written to disk > >=20 >=20 > We know what the file size should be, so we can poll for the actual > size. Actually why do we need to poll? we could add a > "internal.screendump.complete" or "internal-query-screendump", no? Neither is possible because hmp.c really uses QMP as client, except that it= 's done via C. > > 2. Use inotify > >=20 > > Cons: what about windows? > >=20 > > 3. Introduce query-screendump that returns the last screendump status > >=20 > > Cons: this is actually making screendump async > >=20 > >=20 > > Anthony, do you have any ideas? > >=20 > > Btw, I've started doing the screendump conversion to the qapi, I'll pos= t it > > soon. >=20 > I've already sent patches once for a new qapi command, I don't mind you > doing this of course. It would actually help me if you do it, I have an initial version at: git://repo.or.cz/qemu/qmp-unstable.git qmp-wip/qapi-commands-conv/screendu= mp/rfc It's old and it's on top of some uninteresting stuff, the only commits that matter there are 48e7c01b and 0f5509a8. But this rfc is just to have an idea how the final command will look like. =46rom what I barely remember, I'd suggest you to do the following: 1. Pass the Error object to hw_screen_dump() 2. Convert the screendump command to the qapi 3. Report errors from ppm_save() via Error Important note: my code reports only QERR_OPEN_FILE_FAILED in ppm_save(), b= ut the right thing to do is to report most likely errors like EACCESS, ENOSPC, EPERM, EIO etc.