Re: [PATCH v6 2/4] qmp: add dump machine type compatible properties

[Markus Armbruster]

Maksim Davydov <davydov-max@yandex-team.ru> writes:

> Thanks for reviewing
> Sorry for replying late
>
> On 12/1/23 12:49, Markus Armbruster wrote:
>> I apologize for the lateness of my review.
>>
>> Maksim Davydov <davydov-max@yandex-team.ru> writes:
>>
>>> To control that creating new machine type doesn't affect the previous
>>> types (their compat_props) and to check complex compat_props inheritance
>>> we need qmp command to print machine type compatible properties.
>>>
>>> This patch adds the ability to get list of all the compat_props of the
>>> corresponding supported machines for their comparison via new optional
>>> argument of "query-machines" command.
>>
>> Sounds like this is to let developers prevent unwanted change.  Such
>> testing interfaces need not and should not be stable interfaces.
>> Thoughts?
>
> I'm not sure that rightly understand your idea about unstable: do you mean
> that we can allow this interface to be not robust (e.g. compat-props in
> MachineInfo can be not presented) or that this is new thusit can be
> unstable?

It's about external interface stability: incompatible change requires
deprecation and a grace period.  See docs/about/deprecated.rst first
section.

QMP is a stable external interface, except for parts marked unstable.

In my review, I wondered whether the QMP interface you add needs to be
stable in that sense.  Does it?

If no, I'll gladly show you how to mark it unstable.

>>> Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
>>> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

[...]

>>> diff --git a/qapi/machine.json b/qapi/machine.json
>>> index b6d634b30d..8ca0c134a2 100644
>>> --- a/qapi/machine.json
>>> +++ b/qapi/machine.json

[...]

>>>  ##
>>>  # @query-machines:
>>>  #
>>>  # Return a list of supported machines
>>>  #
>>> +# @compat-props: if true return will contain information about machine type
>>> +#                compatible properties (since 8.2)
>>
>> "compatibility properties"
>>
>> Suppressing parts of the output makes sense only if it's fairly big.
>> How much additional compat-props output do you observe?
>
> now, there are 61 machines types, so this qmp command generates a 450KB JSON

Uff.

Recommend to explain this briefly in the commit message.  Something like
"Since information on compatibility properties can increase the command
output by a factor of 40, add an argument to enable it, default off."

Does make me wonder whether we want a separate command, though.

[...]