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

[Anthony Liguori]

Paolo Bonzini <address@hidden> writes:

> Il 22/04/2013 14:50, Luiz Capitulino ha scritto:
>> On Mon, 22 Apr 2013 11:37:15 +0200
>> Paolo Bonzini <address@hidden> wrote:
>> 
>>> Il 22/04/2013 08:55, Gerd Hoffmann ha scritto:
>>>>>>>> Question for the libvirt guys:  Is it ok for libvirt to just extend the
>>>>>>>> existing screendump command?  Can libvirt figure there is a new
>>>>>>>> (optional) parameter?  See patch #5.
>>>>>>
>>>>>> Nope, QMP can't do that.  I argued for such capabilities, but the
>>>>>> "create a new command" philosophy prevailed.
>>>>>>
>>>>>> Go forth and multiply commands!  And have fun picking command names that
>>>>>> aren't fugly.
>>>> Oh joy.  Lets just enumerate things & use "screendump2" ...
>>>
>>> QMP can't do that _yet_.
>>>
>>> Let's fix it instead...
>> 
>> The point is that we have chosen not to do so a while ago. In a nutshell,
>> Anthony thinks that we should have the same compatibility contract of
>> a C API.
>
> 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.  If you think about it
in terms of a shipped qmp.h file, it would be:

struct CharInfo
{
};

CharInfo *qmp_query_char(QMPSession *session, Error **errp);

But adding parameters to functions breaks the ABI and there's no way
around unless you make all parameters passed as a single structure.

> If the C API is a requirement, it should also be
> a requirement for structs.  But there are plenty of ways (some nicer,
> some uglier) to have different API versions in C.
>
> For example, I think the QIDL patch had ways to annotate each parameter
> independently.  You can annotate each argument with the "first version
> this appeared in" and complicate the C API generator to generate
> multiple C functions for the same command.
>
> It is then the downstream's responsibility not to backport extra
> arguments without a full-blown rebase to a newer version (the same as
> for C libraries).

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.

We can also introduce a "format" parameter to allow specifying formats
othe than PPM.

Regards,

Anthony Liguori

>
> Paolo