Re: [Qemu-devel] [PATCH 17/32] qmp: Don't let malformed in-band commands jump the queue

[Markus Armbruster <armbru@redhat.com>]

Eric Blake <eblake@redhat.com> writes:

> On 07/02/2018 11:22 AM, Markus Armbruster wrote:
>> handle_qmp_command() reports certain errors right away.  This is wrong
>> when OOB is enabled, because the errors can "jump the queue" then, as
>> the previous commit demonstrates.
>>
>> To fix, we need to delay errors until dispatch.  Do that for semantic
>> errors, mostly by reverting ill-advised parts of commit cf869d53172
>> "qmp: support out-of-band (oob) execution".  Bonus: doesn't run
>> qmp_dispatch_check_obj() twice, once in handle_qmp_command(), and
>> again in do_qmp_dispatch().  That's also due to commit cf869d53172.
>>
>> The next commit will fix queue jumping for syntax errors.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   include/qapi/qmp/dispatch.h |  2 -
>>   monitor.c                   | 79 +++++++++----------------------------
>>   qapi/qmp-dispatch.c         | 12 +++++-
>>   tests/qmp-test.c            |  4 +-
>>   4 files changed, 30 insertions(+), 67 deletions(-)
>>
>
> Reviewed-by: Eric Blake <eblake@redhat.com
>
> Fixing bug and reducing code size. I'm glad I made oob experimental in
> 2.12, because I obviously didn't review it as closely in your absence
> for that release as you have done now (and changing it to be

In all fairness, it took me a while to see this.  The first clue was
"hmm, why does qmp_dispatch_check_obj() gets called in two places?"
Pulling that thread got me to this bug and more.

> non-experimental early in the release cycle has also been good for
> letting us chase down these bugs in the original implementation).

Running into that regression just in time was lucky :)