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

[Philippe Mathieu-Daudé]

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").

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