Eric Blake <ebl...@redhat.com> writes:
> On Tue, Feb 27, 2024 at 12:56:16PM +0100, Markus Armbruster wrote:
>> The tutorial doesn't match reality since at least 2013. Repairing it
>> involves fixing the following issues:
>>
>> * Update for commit 6d327171551 (aio / timers: Remove alarm timers):
>> replace the broken examples. Instead of having one for returning a
>> struct and another for returning a list of structs, do just one for
>> the latter. This resolves the FIXME added in commit
>> e218052f928 (aio / timers: De-document -clock) back in 2014.
>>
>> * Update for commit 895a2a80e0e (qapi: Use 'struct' instead of 'type'
>> in schema).
>>
>> * Update for commit 3313b6124b5 (qapi: add qapi2texi script): add
>> required documentation to the schema snippets, and drop section
>> "Command Documentation".
>>
>> * Update for commit a3c45b3e629 (qapi: New special feature flag
>> "unstable"): supply the required feature, deemphasize the x- prefix.
>>
>> * Update for commit dd98234c059 (qapi: introduce x-query-roms QMP
>> command): rephrase from "add new command" to "examine existing
>> command".
>>
>> * Update for commit 9492718b7c0 (qapi misc: Elide redundant has_FOO in
>> generated C): hello-world's message argument no longer comes with a
>> has_message, add a second argument that does.
>>
>> * Update for moved and renamed files.
>>
>> While there, update QMP version output to current output.
>
> Nice cleanups.
Thanks!
>> Signed-off-by: Markus Armbruster <arm...@redhat.com>
>> ---
>> docs/devel/writing-monitor-commands.rst | 460 ++++++++++--------------
>> 1 file changed, 181 insertions(+), 279 deletions(-)
>
>>
>> -The "type" keyword defines a new QAPI type. Its "data" member contains the
>> -type's members. In this example our members are the "clock-name" and the
>> -"next-deadline" one, which is optional.
>> +The "struct" keyword defines a new QAPI type. Its "data" member
>> +contains the type's members. In this example our members are
>> +"filename" and "bootindex". The latter is optional.
>
> Is there any rhyme or reason behind one vs. two spaces after sentences here?
Accident. I habitually use two spaces, but this document uses just one.
I tried to be locally consistent, but habit won in this case. I'll fix
it.
>>
>> The HMP command
>> ~~~~~~~~~~~~~~~
>>
>> -Here's the HMP counterpart of the query-alarm-clock command::
>> +Here's the HMP counterpart of the query-option-roms command::
>>
>> - void hmp_info_alarm_clock(Monitor *mon)
>> + void hmp_info_option_roms(Monitor *mon, const QDict *qdict)
>> {
>> - QemuAlarmClock *clock;
>> Error *err = NULL;
>> + OptionRomInfoList *info_list, *tail;
>> + OptionRomInfo *info;
>>
>> - clock = qmp_query_alarm_clock(&err);
>> + info_list = qmp_query_option_roms(&err);
>> if (hmp_handle_error(mon, err)) {
>> - return;
>> + return;
>
> Was the change to TAB intentional?
Another accident.
>
>> }
>>
>> - monitor_printf(mon, "Alarm clock method in use: '%s'\n",
>> clock->clock_name);
>> - if (clock->has_next_deadline) {
>> - monitor_printf(mon, "Next alarm will fire in %" PRId64 "
>> nanoseconds\n",
>> - clock->next_deadline);
>> + for (tail = info_list; tail; tail = tail->next) {
>> + info = tail->value;
>> + monitor_printf(mon, "%s", info->filename);
>> + if (info->has_bootindex) {
>> + monitor_printf(mon, " %" PRId64, info->bootindex);
>> + }
>> + monitor_printf(mon, "\n");
>> }
>
> More TABs.
Will expand them all.
>> Writing a debugging aid returning unstructured text
>> ---------------------------------------------------
> ...
>> -The ``HumanReadableText`` struct is intended to be used for all
>> -commands, under the ``x-`` name prefix that are returning unstructured
>> -text targeted at humans. It should never be used for commands outside
>> -the ``x-`` name prefix, as those should be using structured QAPI types.
>> +The ``HumanReadableText`` struct is defined in qapi/common.json as a
>> +struct with a string member. It is intended to be used for all
>> +commands that are returning unstructured text targeted at
>> +humans. These should all have feature 'unstable'. Note that the
>> +feature's documentation states why the command is unstable. WE
>
> We
Will fix.
>> +commonly use a ``x-`` command name prefix to make lack of stability
>> +obvious to human users.
>>
>
> Cleanups are trivial enough that I'm fine with you making them before
> including:
>
> Reviewed-by: Eric Blake <ebl...@redhat.com>
Thanks!
