From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51293) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UoalG-0003Wn-BI for qemu-devel@nongnu.org; Mon, 17 Jun 2013 10:50:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UoalB-0001UO-Cm for qemu-devel@nongnu.org; Mon, 17 Jun 2013 10:50:26 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39385) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UoalB-0001UG-5v for qemu-devel@nongnu.org; Mon, 17 Jun 2013 10:50:21 -0400 Date: Mon, 17 Jun 2013 10:50:18 -0400 From: Luiz Capitulino Message-ID: <20130617105018.26c2eadc@redhat.com> In-Reply-To: <87a9mo4x38.fsf@codemonkey.ws> References: <1371477707-7039-1-git-send-email-kraxel@redhat.com> <1371477707-7039-3-git-send-email-kraxel@redhat.com> <87a9mo4x38.fsf@codemonkey.ws> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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: Anthony Liguori Cc: alevy@redhat.com, Gerd Hoffmann , Markus Armbruster , qemu-devel@nongnu.org On Mon, 17 Jun 2013 09:43:07 -0500 Anthony Liguori wrote: > 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'd agree with this. > 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 >