From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:58269)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <lcapitulino@redhat.com>) id 1S7RtO-0007Nv-MK
	for qemu-devel@nongnu.org; Tue, 13 Mar 2012 09:36:04 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <lcapitulino@redhat.com>) id 1S7RtJ-0003sJ-6n
	for qemu-devel@nongnu.org; Tue, 13 Mar 2012 09:35:58 -0400
Received: from mx1.redhat.com ([209.132.183.28]:6321)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <lcapitulino@redhat.com>) id 1S7RtI-0003s8-Uj
	for qemu-devel@nongnu.org; Tue, 13 Mar 2012 09:35:53 -0400
Date: Tue, 13 Mar 2012 10:35:55 -0300
From: Luiz Capitulino <lcapitulino@redhat.com>
Message-ID: <20120313103555.0ae4b834@doriath.home>
In-Reply-To: <1331494004-26177-5-git-send-email-alevy@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>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v2 4/5] console: pass Monitor to
 vga_hw_screen_dump/hw_vga_dump
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Alon Levy <alevy@redhat.com>
Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org, kraxel@redhat.com

On Sun, 11 Mar 2012 21:26:43 +0200
Alon Levy <alevy@redhat.com> wrote:

> Passes the Monitor ptr to the screendump implementation to all for
> monitor suspend and resume for qxl to fix screendump regression.
> 
> 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.

NACK on this one.

The Monitor object should be restricted to HMP. This patch spreads it to
what's going to be the QMP implementation of screendump.

The first step here should be to convert the screendump command to the qapi,
and lock the HMP shell in hmp_screendump().

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 :)

Some solutions that come to my mind:

 1. Pool the screendump file creation from a timer.

    Cons: it may return before the file is fully written to disk

 2. Use inotify

    Cons: what about windows?

 3. Introduce query-screendump that returns the last screendump status

    Cons: this is actually making screendump async


Anthony, do you have any ideas?

Btw, I've started doing the screendump conversion to the qapi, I'll post it
soon.