From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:55260) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UUKzC-0004ej-MC for qemu-devel@nongnu.org; Mon, 22 Apr 2013 13:57:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UUKzB-0002nd-I9 for qemu-devel@nongnu.org; Mon, 22 Apr 2013 13:57:06 -0400 Received: from e23smtp05.au.ibm.com ([202.81.31.147]:46865) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UUKzB-0002n9-0P for qemu-devel@nongnu.org; Mon, 22 Apr 2013 13:57:05 -0400 Received: from /spool/local by e23smtp05.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 23 Apr 2013 03:52:02 +1000 Received: from d23relay05.au.ibm.com (d23relay05.au.ibm.com [9.190.235.152]) by d23dlp02.au.ibm.com (Postfix) with ESMTP id B160B2BB0050 for ; Tue, 23 Apr 2013 03:56:59 +1000 (EST) Received: from d23av03.au.ibm.com (d23av03.au.ibm.com [9.190.234.97]) by d23relay05.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r3MHhGD19830754 for ; Tue, 23 Apr 2013 03:43:17 +1000 Received: from d23av03.au.ibm.com (loopback [127.0.0.1]) by d23av03.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r3MHuw1Q025060 for ; Tue, 23 Apr 2013 03:56:58 +1000 From: Anthony Liguori In-Reply-To: <51756D68.1050705@redhat.com> References: <1366275680-15416-1-git-send-email-kraxel@redhat.com> <87ppxq6hmn.fsf@blackfin.pond.sub.org> <5174DEE5.20406@redhat.com> <517504CB.6040303@redhat.com> <20130422085043.4a3e891c@redhat.com> <51753471.5080803@redhat.com> <8738uiqzm1.fsf@codemonkey.ws> <51756D68.1050705@redhat.com> Date: Mon, 22 Apr 2013 12:56:51 -0500 Message-ID: <87obd6h2jg.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [RfC PATCH 0/5] console: qom-ify & extent screendump monitor command List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Markus Armbruster , Gerd Hoffmann , qemu-devel@nongnu.org, Luiz Capitulino Paolo Bonzini writes: > Il 22/04/2013 18:49, Anthony Liguori ha scritto: >>> We've been adding fields to types since 0.15, sometimes in the middle of >>> a struct (since 1.2). >> >> You can safely add fields to the end of a struct. > > For QEMU->user structs it is. For user->QEMU structs you need to add a > sizeof() at the beginning, or ensure that everything is heap-allocated > (and zero-initialized). Think library generated from qapi-schema.json. We want this library to have a backwards compatible CABI. There are a couple ways to deal with adding to the end. You could do it kernel-style and zero pad structures. Another option is to have a flags fields as the first member and use that to indicate optional parameters. A 64-bit flags value would allow 64 optional parameters which should keep us comfortable for quite a while. > At that point you could also use structs to pass arguments to the > functions (in the C client API) that execute a QMP command. That's > similar to having keyword arguments in C. Ack. >> Well this is all well and good in abstract, in practice, we want a new >> screendump command anyway. >> >> It'd be *much* nicer to return the screenshot data via the QMP session >> instead of writing it to a file. So let's take the opportunity to fix >> the command. > > That's debatable... the "nicest" way could also be to pass a pipe fd and > retrieve the dump from that fd. That's quite easy to do with fdsets. > The choice is between implementing SCM_RIGHTS sendfd and a base64 > decoder. Granted, base64 increases the size by a 66% but I don't think it's a huge issue. >> We can also introduce a "format" parameter to allow specifying formats >> othe than PPM. > > True, but I'm not sure we want to go there. We'd need to add support > for options like JPG quality factor etc. PNG would be extremely handy and would go a long way to eliminating the concern about size. We already link against libpng too. You can imagine an interface like: { "type": "Blob", "data": { "format": "DataFormat", "data": "str" } } ... { "union": "ImageOptions", "data": { "ppm": "PPMOptions", "png": "PNGOptions" } } { "command": "display-get-screenshot", "data": { "id": "str", "*ImageOptions": "options", "*format": "DataFormat" }, "returns": "Blob" } I think it's worth implementing. A local screenshot I have is 2.3Mb as a PPM but only 320k as a PNG. base64 encoded the PNG is 428k which is still significantly smaller than the PPM. Regards, Anthony Liguori > > Paolo