From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49032) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uoafd-0008MH-9F for qemu-devel@nongnu.org; Mon, 17 Jun 2013 10:44:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UoafY-0007fl-7R for qemu-devel@nongnu.org; Mon, 17 Jun 2013 10:44:37 -0400 Received: from e9.ny.us.ibm.com ([32.97.182.139]:34909) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UoafY-0007fY-35 for qemu-devel@nongnu.org; Mon, 17 Jun 2013 10:44:32 -0400 Received: from /spool/local by e9.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 17 Jun 2013 10:44:30 -0400 Received: from d01relay01.pok.ibm.com (d01relay01.pok.ibm.com [9.56.227.233]) by d01dlp03.pok.ibm.com (Postfix) with ESMTP id E1B2DC90046 for ; Mon, 17 Jun 2013 10:44:26 -0400 (EDT) Received: from d01av03.pok.ibm.com (d01av03.pok.ibm.com [9.56.224.217]) by d01relay01.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r5HEhBFp318200 for ; Mon, 17 Jun 2013 10:43:11 -0400 Received: from d01av03.pok.ibm.com (loopback [127.0.0.1]) by d01av03.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r5HEhBFK031232 for ; Mon, 17 Jun 2013 11:43:11 -0300 From: Anthony Liguori In-Reply-To: <1371477707-7039-3-git-send-email-kraxel@redhat.com> References: <1371477707-7039-1-git-send-email-kraxel@redhat.com> <1371477707-7039-3-git-send-email-kraxel@redhat.com> Date: Mon, 17 Jun 2013 09:43:07 -0500 Message-ID: <87a9mo4x38.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [RfC PATCH 2/2] console: add screendump-device qmp cmd List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann , qemu-devel@nongnu.org Cc: alevy@redhat.com, Markus Armbruster , lcapitulino@redhat.com Gerd Hoffmann writes: > Adds a screendump-device qmp command, which has an additional 'device' > parameter. This way it is possible to specify the device you want a > screendump from. > > For the hmp monitor an optional device parameter has been added to the > esisting screendump command. > > https://bugzilla.redhat.com/show_bug.cgi?id=903910 > > Signed-off-by: Gerd Hoffmann > --- > qapi-schema.json | 15 +++++++++++++++ > qmp-commands.hx | 25 +++++++++++++++++++++++++ > ui/console.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 92 insertions(+) > > diff --git a/qapi-schema.json b/qapi-schema.json > index adcf801..719dc6e 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -3125,6 +3125,21 @@ > { 'command': 'screendump', 'data': {'filename': 'str'} } > > ## > +# @screendump-device: > +# > +# Write a PPM from the specified device to a file. > +# > +# @filename: the path of a new PPM file to store the image > +# @device: #optional device to take the screenshot from > +# > +# Returns: Nothing on success > +# > +# Since: 1.6 > +## > +{ 'command': 'screendump-device', 'data': {'filename': 'str', > + '*device' : 'str' }} > + > +## > # @nbd-server-start: > # > # Start an NBD server listening on the given host and port. Block > diff --git a/qmp-commands.hx b/qmp-commands.hx > index 8e69fba..6361561 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -167,6 +167,31 @@ Example: > EQMP > > { > + .name = "screendump-device", > + .args_type = "filename:F,device:B?", > + .mhandler.cmd_new = qmp_marshal_input_screendump_device, > + }, > + > +SQMP > +screendump-device > +----------------- > + > +Save screen into PPM image. > + > +Arguments: > + > +- "filename": file path (json-string) > +- "device": video device (json-string, optional) > + > +Example: > + > +-> { "execute": "screendump-device", > + "arguments": { "filename": "/tmp/image", "device" : "video0" } } > +<- { "return": {} } > + > +EQMP > + > + { > .name = "stop", > .args_type = "", > .mhandler.cmd_new = qmp_marshal_input_stop, > diff --git a/ui/console.c b/ui/console.c > index 020805c..1895acf 100644 > --- a/ui/console.c > +++ b/ui/console.c > @@ -343,6 +343,58 @@ void qmp_screendump(const char *filename, Error **errp) > ppm_save(filename, surface, errp); > } > > +struct screendump_job { > + QEMUBH *bh; > + QemuConsole *con; > + char *filename; > +}; We have a job API in the block layer. Would it make sense to have a QMP-level job interface? I don't think we want to replace the block layer API, but I think having a simple interface that returns a job id, allows querying jobs (including whether they are cancelable), canceling cancelable jobs, and then a single event notifying completion of jobs, would solve a lot of problems in the current interface. Regards, Anthony Liguori > +static void qmp_screendump_bh(void *opaque) > +{ > + Error *local_err; > + struct screendump_job *j = opaque; > + DisplaySurface *surface; > + > + surface = qemu_console_surface(j->con); > + ppm_save(j->filename, surface, &local_err); > + /* TODO: send qmp completion (or error) event */ > + qemu_bh_delete(j->bh); > + free(j->filename); > + free(j); > +} > + > +void qmp_screendump_device(const char *filename, > + bool has_device, const char *device, Error **errp) > +{ > + struct screendump_job *j; > + QemuConsole *con; > + > + if (has_device) { > + DeviceState *dev = qdev_find_recursive(sysbus_get_default(), device); > + if (NULL == dev) { > + error_set(errp, QERR_DEVICE_NOT_FOUND, device); > + return; > + } > + con = qemu_console_lookup_by_device(dev); > + if (NULL == con) { > + error_setg(errp, "There is no QemuConsole linked to %s", device); > + return; > + } > + } else { > + con = qemu_console_lookup_by_index(0); > + if (con == NULL) { > + error_setg(errp, "There is no QemuConsole I can screendump from."); > + return; > + } > + } > + > + j = g_new0(struct screendump_job, 1); > + j->bh = qemu_bh_new(qmp_screendump_bh, j); > + j->con = con; > + j->filename = g_strdup(filename); > + graphic_hw_update_notify(con, j->bh); > +} > + > void graphic_hw_text_update(QemuConsole *con, console_ch_t *chardata) > { > if (!con) { > -- > 1.7.9.7