Re: [Qemu-devel] [PATCH v2 2/2] qapi: Allow setting default values for optional parameters

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 04/29/2014 05:24 AM, Kevin Wolf wrote:
>> Am 29.04.2014 um 11:44 hat Fam Zheng geschrieben:
>>> In command definition, 'defaults' is now parsed as a dict of default
>>> values. Only optional parameters will have effect in generated code.
>>>
>>> 'str' and 'int' are supported. In generated code, 'str' will be
>>> converted to g_strdup'ed string pointer, 'int' will be identically
>>> assigned.
>>>
>
>>>   { 'command': 'my-command',
>>> -   'data': { 'arg1': 'str', '*arg2': 'str' },
>>> +   'data': { 'arg1': 'str', '*arg2': 'str', '*arg3': 'int' },
>>> +   'defaults': { 'arg2': 'default value for arg2', 'arg3': 12345 },
>>>     'returns': 'str' }
>> 
>> I think we should document what effect setting a default has for the
>> code generation, and that it can be left out even for optional arguments
>> (because your example sets it for every optional member).
>
> That includes documenting that omitting a defaults entry for an optional
> parameter will now guarantee 0-initialization of that member.

Currently, an optional FOO should only be used when has_FOO.  We
initialize it anyway, to suppress complaints from static analyzers.

Do we want to relax the rule "officially", so that code can rely on
zero-initialization instead of checking has_FOO?

>> Also, for the actual qmp_foo() handler, I would very much prefer if it
>> didn't get a has_foo argument which is meaningless because it is always
>> true. Instead, the has_foo argument shouldn't even be generated in the
>> first place if the schema has a default for foo.
>
> Yes, that's a nice idea - anywhere we have a qapi-documented default, we
> can simplify the code to take advantage of the default.  It's backward
> compatible since none of the existing code has any contract on defaults.

s/can simplify/should simplify/!  Defining a default value for FOO
renders anything guarded by !has_FOO dead code.  Not generating has_FOO
is good, because it forces us to bury the dead code properly.

Losely related: I want to get rid of has_FOO for pointer-valued FOOs,
too.

> Also, is the default value allowed to change between qemu versions?
> What are the back-compat ramifications if two different releases want to
> tweak an omitted variable to different defaults?  The documentation
> should mention the rule of thumb that we plan on enforcing during
> reviews.  I could go either way: the wire format is unimpacted by what
> default value is used when an argument is omitted, and management can
> always explicitly specify a parameter if they don't trust the default;
> on the other hand, if changing a default changes visible behavior, then
> we have not maintained ABI compatibility.

We should use common sense.

Changing the schema in a way that alters the meaning of existing usage
is a no-no, and that applies to defaults as much as to anything else.
But not every change of a default necessarily does that.  Contrieved
example: if the value defines a buffer size, and it's specified to
default to a sensible size, then we can and should let the default value
evolve along with our idea of what a sensible size is.