Re: [Qemu-devel] [PATCH 05/12] monitor: register the qapi generated commands

[Marc-André Lureau]

On Fri, Jun 24, 2016 at 6:31 AM, Eric Blake <address@hidden> wrote:
>> +    qcmd = qmp_find_command(cmd_name);
>
> Is it worth creating a single type that contains both mon_cmd_t and
> QmpCommand information, so that we don't need two similarly-named
> functions to look up two related pieces of information?  Not necessarily
> in this patch.

Well, this is only temporary. Theuse 'qmp_dispatch()' get rid of all that.

>
>> +    if (!qcmd || !cmd) {
>>          error_set(&local_err, ERROR_CLASS_COMMAND_NOT_FOUND,
>>                    "The command %s has not been found", cmd_name);
>>          goto err_out;
>> @@ -3931,7 +3933,7 @@ static void handle_qmp_command(JSONMessageParser 
>> *parser, GQueue *tokens)
>>          goto err_out;
>>      }
>>
>> -    cmd->mhandler.cmd_new(args, &data, &local_err);
>> +    qcmd->fn(args, &data, &local_err);
>>
>>  err_out:
>>      monitor_protocol_emitter(mon, data, local_err);
>> @@ -4000,9 +4002,13 @@ void monitor_resume(Monitor *mon)
>>
>>  static QObject *get_qmp_greeting(void)
>>  {
>> +    QmpCommand *cmd;
>>      QObject *ver = NULL;
>>
>> -    qmp_marshal_query_version(NULL, &ver, NULL);
>> +    cmd = qmp_find_command("query-version");
>> +    assert(cmd && cmd->fn);
>> +    cmd->fn(NULL, &ver, NULL);
>> +
>>      return qobject_from_jsonf("{'QMP':{'version': %p,'capabilities': 
>> []}}",ver);
>
> Worth fixing the missing space after ',' while touching near here?

fine by me ;)

>
>>  }
>>
>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>> index ee88e48..95c1e7d 100644
>> --- a/qmp-commands.hx
>> +++ b/qmp-commands.hx
>> @@ -63,7 +63,6 @@ EQMP
>>      {
>>          .name       = "quit",
>>          .args_type  = "",
>> -        .mhandler.cmd_new = qmp_marshal_quit,
>
> At one point, I posted an RFC patch for removing .args_type on most QMP
> command listings in this file. Maybe you still do that later in your
> series, but as my work definitely conflicts with yours, and your series
> is older, I don't mind getting through your series first.

> Overall, I like the general idea of automating things rather than having
> to duplicate information in qmp-commands.hx.

Yeah, I basically get rid of qmp-commands.hx in my series (and the doc
follow up in https://github.com/elmarco/qemu/commits/qapi-doc)




-- 
Marc-André Lureau