Re: [PATCH v2 29/53] qapi: introduce x-query-registers QMP command

[Philippe Mathieu-Daudé]

On 9/14/21 7:15 PM, Philippe Mathieu-Daudé wrote:
> On 9/14/21 6:04 PM, Eric Blake wrote:
>> On Tue, Sep 14, 2021 at 03:20:18PM +0100, Daniel P. Berrangé wrote:
>>> This is a counterpart to the HMP "info registers" command. It is being
>>> added with an "x-" prefix because this QMP command is intended as an
>>> ad hoc debugging tool and will thus not be modelled in QAPI as fully
>>> structured data, nor will it have long term guaranteed stability.
>>> The existing HMP command is rewritten to call the QMP command.
>>>
>>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>>> ---
> 
>>> +##
>>> +# @HumanReadableText:
>>> +#
>>> +# @human-readable-text: Formatted output intended for humans.
>>> +#
>>> +# Since: 6.2.0
>>
>> Should be '6.2', not '6.2.0', to match...
>>
>>> +#
>>> +##
>>> +{ 'struct': 'HumanReadableText',
>>> +  'data': { 'human-readable-text': 'str' } }
>>> diff --git a/qapi/machine.json b/qapi/machine.json
>>> index 157712f006..8737efa865 100644
>>> --- a/qapi/machine.json
>>> +++ b/qapi/machine.json
>>> @@ -1312,3 +1312,18 @@
>>>       '*cores': 'int',
>>>       '*threads': 'int',
>>>       '*maxcpus': 'int' } }
>>> +
>>> +##
>>> +# @x-query-registers:
>>> +#
>>> +# @cpu: the CPU number to query. If omitted, queries all CPUs
>>> +#
>>> +# Query information on the CPU registers
>>> +#
>>> +# Returns: CPU state in an architecture-specific format
>>> +#
>>> +# Since: 6.2
>>
>> ...the prevailing style.
>>
>> If it were likely that someone might backport just some (but not all)
>> added x- commands, it may be wise to separate the creation of
>> HumanReadableText into its own patch to backport that but not
>> x-query-registers.  But I rather suspect anyone backporting this will
>> take the series wholesale, so the coupling in this patch is not worth
>> worrying about.
> 
> IIUC the problem is this breaks bisection, as you get a QAPI error:
> 
>  qapi/qapi-commands-machine.c:123:13: error:
> ‘qmp_marshal_output_HumanReadableText’
> defined but not used [-Werror=unused-function]
>   123 | static void
> qmp_marshal_output_HumanReadableText(HumanReadableText *ret_in,
>       |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
> 
> See the comment added in commit 1f7b9f3181e
> ("qapi/commands: add #if conditions to commands").

Oh we already talked about this together in this thread:
https://lore.kernel.org/qemu-devel/20210609202952.r4nb2smrptyckvk2@redhat.com/

> 
>>
>>> +##
>>> +{ 'command': 'x-query-registers',
>>> +  'data': {'*cpu': 'int' },
>>> +  'returns': 'HumanReadableText' }
>>> -- 
>>> 2.31.1
>>>
>>