Luiz Capitulino <lcapitul...@redhat.com> writes:
> On Fri, 29 May 2015 11:56:50 +0200
> Markus Armbruster <arm...@redhat.com> wrote:
>
>> Signed-off-by: Markus Armbruster <arm...@redhat.com>
>> ---
>> monitor.c | 18 ++++++++++--------
>> 1 file changed, 10 insertions(+), 8 deletions(-)
>>
>> diff --git a/monitor.c b/monitor.c
>> index 61ea346..d336b8f 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -4708,19 +4708,20 @@ static int monitor_can_read(void *opaque)
>> return (mon->suspend_cnt == 0) ? 1 : 0;
>> }
>>
>> -static bool invalid_qmp_mode(const Monitor *mon, const mon_cmd_t *cmd)
>> +static bool invalid_qmp_mode(const Monitor *mon, const mon_cmd_t *cmd,
>> + Error **errp)
>> {
>> bool is_cap = cmd->mhandler.cmd_new == do_qmp_capabilities;
>> if (is_cap && qmp_cmd_mode(mon)) {
>> - qerror_report(ERROR_CLASS_COMMAND_NOT_FOUND,
>> - "Capabilities negotiation is already complete,
>> command "
>> - "'%s' ignored", cmd->name);
>> + error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
>> + "Capabilities negotiation is already complete, command "
>> + "'%s' ignored", cmd->name);
>> return true;
>> }
>> if (!is_cap && !qmp_cmd_mode(mon)) {
>> - qerror_report(ERROR_CLASS_COMMAND_NOT_FOUND,
>> - "Expecting capabilities negotiation with "
>> - "'qmp_capabilities' before command '%s'", cmd->name);
>> + error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
>> + "Expecting capabilities negotiation with "
>> + "'qmp_capabilities' before command '%s'", cmd->name);
>> return true;
>> }
>> return false;
>> @@ -5010,7 +5011,8 @@ static void handle_qmp_command(JSONMessageParser
>> *parser, QList *tokens)
>> "The command %s has not been found", cmd_name);
>> goto err_out;
>> }
>> - if (invalid_qmp_mode(mon, cmd)) {
>> + if (invalid_qmp_mode(mon, cmd, &local_err)) {
>> + qerror_report_err(local_err);
>
> I don't think you need to call qerror_report_err() here, and you could
> have changed invalid_qmp_mode() to return void. You can make those changes
> before applying if you choose to do them:
Actually, I have to call it here, because after this patch, we still
have monitor_protocol_emitter() getting the error from mon->error, so we
still have to put it there.
However, I forgot to drop it along with the other calls in PATCH 14!
> Reviewed-by: Luiz Capitulino <lcapitul...@redhat.com>
Thanks!
>> goto err_out;
>> }
>>
