Re: [Qemu-devel] [RfC PATCH 0/5] console: qom-ify & extent screendump monitor command

[Anthony Liguori <aliguori@us.ibm.com>]

Paolo Bonzini <pbonzini@redhat.com> 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