[16/32] tests/qmp-test: Demonstrate QMP errors jumping the queue

When OOB is enabled, out-of-band commands are executed right away,
everything else is queued.  This lets out-of-band commands "jump the
queue".

However, certain errors are always reported right away, and therefore
can jump the queue even when the erroneous input does not request
out-of-band execution.  These errors are pretty unlikely to occur in
production, but it's wrong all the same.  Mark FIXME.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 monitor.c        | 1 +
 tests/qmp-test.c | 7 +++++++
 2 files changed, 8 insertions(+)

On 07/02/2018 11:22 AM, Markus Armbruster wrote:
> When OOB is enabled, out-of-band commands are executed right away,
> everything else is queued.  This lets out-of-band commands "jump the
> queue".
> 
> However, certain errors are always reported right away, and therefore
> can jump the queue even when the erroneous input does not request
> out-of-band execution.  These errors are pretty unlikely to occur in
> production, but it's wrong all the same.  Mark FIXME.

Ouch.

> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   monitor.c        | 1 +
>   tests/qmp-test.c | 7 +++++++
>   2 files changed, 8 insertions(+)
> 

> +++ b/tests/qmp-test.c
> @@ -239,6 +239,13 @@ static void test_qmp_oob(void)
>       unblock_blocked_cmd();
>       recv_cmd_id(qts, "ib-blocks-1");
>       recv_cmd_id(qts, "ib-quick-1");
> +
> +    /* FIXME certain in-band errors overtake slow in-band command */
> +    send_cmd_that_blocks(qts, "blocks-2");
> +    qtest_async_qmp(qts, "{ 'id': 'err-2' }");

Since this has neither 'execute' nor 'exec-oob', we can't state whether 
it is in-band or out-of-band; back-compatibility says it should be 
treated as in-band.

> +    recv_cmd_id(qts, NULL);
> +    unblock_blocked_cmd();
> +    recv_cmd_id(qts, "blocks-2");

Thus, it should have been queued until after blocks-2 completed.

Useful test.
Reviewed-by: Eric Blake <eblake@redhat.com>

On Mon, Jul 02, 2018 at 06:22:02PM +0200, Markus Armbruster wrote:
> When OOB is enabled, out-of-band commands are executed right away,
> everything else is queued.  This lets out-of-band commands "jump the
> queue".
> 
> However, certain errors are always reported right away, and therefore
> can jump the queue even when the erroneous input does not request
> out-of-band execution.  These errors are pretty unlikely to occur in
> production, but it's wrong all the same.  Mark FIXME.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  monitor.c        | 1 +
>  tests/qmp-test.c | 7 +++++++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/monitor.c b/monitor.c
> index 51ba1485ad..28fa9b8d44 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -4338,6 +4338,7 @@ static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens)
>      return;
>  
>  err:
> +    /* FIXME overtakes queued in-band commands, wrong when !qmp_is_oob() */
>      monitor_qmp_respond(mon, NULL, err, NULL);
>      qobject_unref(req);
>  }
> diff --git a/tests/qmp-test.c b/tests/qmp-test.c
> index dc30930201..fe5e5b548a 100644
> --- a/tests/qmp-test.c
> +++ b/tests/qmp-test.c
> @@ -239,6 +239,13 @@ static void test_qmp_oob(void)
>      unblock_blocked_cmd();
>      recv_cmd_id(qts, "ib-blocks-1");
>      recv_cmd_id(qts, "ib-quick-1");
> +
> +    /* FIXME certain in-band errors overtake slow in-band command */
> +    send_cmd_that_blocks(qts, "blocks-2");
> +    qtest_async_qmp(qts, "{ 'id': 'err-2' }");
> +    recv_cmd_id(qts, NULL);

(I thought the "id" will be passed in when reply with the error,
 though in fact it's not)

Thanks for the test case.  I'll work upon it when fixing that up.

Reviewed-by: Peter Xu <peterx@redhat.com>

> +    unblock_blocked_cmd();
> +    recv_cmd_id(qts, "blocks-2");
>      cleanup_blocking_cmd();
>  
>      qtest_quit(qts);
> -- 
> 2.17.1
>

Peter Xu <peterx@redhat.com> writes:

> On Mon, Jul 02, 2018 at 06:22:02PM +0200, Markus Armbruster wrote:
>> When OOB is enabled, out-of-band commands are executed right away,
>> everything else is queued.  This lets out-of-band commands "jump the
>> queue".
>> 
>> However, certain errors are always reported right away, and therefore
>> can jump the queue even when the erroneous input does not request
>> out-of-band execution.  These errors are pretty unlikely to occur in
>> production, but it's wrong all the same.  Mark FIXME.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  monitor.c        | 1 +
>>  tests/qmp-test.c | 7 +++++++
>>  2 files changed, 8 insertions(+)
>> 
>> diff --git a/monitor.c b/monitor.c
>> index 51ba1485ad..28fa9b8d44 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -4338,6 +4338,7 @@ static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens)
>>      return;
>>  
>>  err:
>> +    /* FIXME overtakes queued in-band commands, wrong when !qmp_is_oob() */
>>      monitor_qmp_respond(mon, NULL, err, NULL);
>>      qobject_unref(req);
>>  }
>> diff --git a/tests/qmp-test.c b/tests/qmp-test.c
>> index dc30930201..fe5e5b548a 100644
>> --- a/tests/qmp-test.c
>> +++ b/tests/qmp-test.c
>> @@ -239,6 +239,13 @@ static void test_qmp_oob(void)
>>      unblock_blocked_cmd();
>>      recv_cmd_id(qts, "ib-blocks-1");
>>      recv_cmd_id(qts, "ib-quick-1");
>> +
>> +    /* FIXME certain in-band errors overtake slow in-band command */
>> +    send_cmd_that_blocks(qts, "blocks-2");
>> +    qtest_async_qmp(qts, "{ 'id': 'err-2' }");
>> +    recv_cmd_id(qts, NULL);
>
> (I thought the "id" will be passed in when reply with the error,
>  though in fact it's not)

It will be after the next patch :)

> Thanks for the test case.  I'll work upon it when fixing that up.
>
> Reviewed-by: Peter Xu <peterx@redhat.com>
>
>> +    unblock_blocked_cmd();
>> +    recv_cmd_id(qts, "blocks-2");
>>      cleanup_blocking_cmd();
>>  
>>      qtest_quit(qts);
>> -- 
>> 2.17.1
>>