Message ID | 1432893420-18687-12-git-send-email-armbru@redhat.com |
---|---|
State | New |
Headers | show |
On Fri, 29 May 2015 11:56:50 +0200 Markus Armbruster <armbru@redhat.com> wrote: > Signed-off-by: Markus Armbruster <armbru@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: Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com> > goto err_out; > } >
On 05/29/2015 03:56 AM, Markus Armbruster wrote: > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > monitor.c | 18 ++++++++++-------- > 1 file changed, 10 insertions(+), 8 deletions(-) Reviewed-by: Eric Blake <eblake@redhat.com>
Luiz Capitulino <lcapitulino@redhat.com> writes: > On Fri, 29 May 2015 11:56:50 +0200 > Markus Armbruster <armbru@redhat.com> wrote: > >> Signed-off-by: Markus Armbruster <armbru@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 <lcapitulino@redhat.com> Thanks! >> goto err_out; >> } >>
On Fri, 29 May 2015 16:19:28 +0200 Markus Armbruster <armbru@redhat.com> wrote: > Luiz Capitulino <lcapitulino@redhat.com> writes: > > > On Fri, 29 May 2015 11:56:50 +0200 > > Markus Armbruster <armbru@redhat.com> wrote: > > > >> Signed-off-by: Markus Armbruster <armbru@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. You're right! > However, I forgot to drop it along with the other calls in PATCH 14! You don't have to respin because of that, feel free to fix it when applying. > > > Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com> > > Thanks! > > >> goto err_out; > >> } > >> >
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); goto err_out; }
Signed-off-by: Markus Armbruster <armbru@redhat.com> --- monitor.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)