Eric Blake <ebl...@redhat.com> writes:
> On 05/22/2015 05:36 AM, Markus Armbruster wrote:
>> The asynchronous monitor command interface goes back to commit 940cc30
>> (Jan 2010). Added a third case to command execution. The hope back
>> then according to the commit message was that all commands get
>> converted to the asynchronous interface, killing off the other two
>> cases. Didn't happen.
>>
>> The initial asynchronous commands balloon and info balloon were
>> converted back to synchronous long ago (commit 96637bc and d72f32),
>> with commit messages calling the asynchronous interface "not fully
>> working" and "deprecated". The only other user went away in commit
>> 3b5704b.
>
> The history is helpful; thanks.
>
>>
>> New code generally uses synchronous commands and asynchronous events.
>>
>> What exactly is still "not fully working" with asynchronous commands?
>> Well, here's a bug that defeats actual asynchronous use pretty
>> reliably: the reply's ID is wrong (and has always been wrong) unless
>> you use the command synchronously! To reproduce, we need an
>> asynchronous command, so we have to go back before the commit 3b5704b.
>> Run QEMU with spice:
>>
>> $ qemu-system-x86_64 -nodefaults -S -spice
>> port=5900,disable-ticketing -qmp stdio
>> {"QMP": {"version": {"qemu": {"micro": 50, "minor": 2, "major":
>> 2}, "package": ""}, "capabilities": []}}
>>
>> Connect a spice client in another terminal:
>>
>> $ remote-viewer spice://localhost:5900
>>
>> Set up a migration destination dummy in a third terminal:
>>
>> $ socat TCP-LISTEN:12345 STDIO
>>
>> Now paste the following into the QMP monitor:
>>
>> { "execute": "qmp_capabilities" }
>
> If you stick and "id":"i0" here...
>
>> { "execute": "client_migrate_info", "id": "i1", "arguments": {
>> "protocol": "spice", "hostname": "localhost", "port": 12345 } }
>> { "execute": "query-kvm", "id": "i2" }
>>
>> Produces two replies immediately, one to qmp_capabilities, and one to
>> query-kvm:
>>
>> {"return": {}}
>
> ...it should show up here, for even less ambiguity about the problems
> with the async response.
Can do.
>> {"return": {"enabled": false, "present": true}, "id": "i2"}
>>
>> Both are correct. Two lines of debug output from libspice-server not
>> shown.
>>
>> Now EOF socat's standard input to make it close the connection. This
>> makes the asynchronous client_migrate_info complete. It replies:
>>
>> {"return": {}}
>>
>> Bug: "id": "i1" is missing. Two lines of debug output from
>> libspice-server not shown. Cherry on top: storage for the missing ID
>> is leaked.
>>
>> Get rid of this stuff before somebody hurts himself with it.
>>
>> Signed-off-by: Markus Armbruster <arm...@redhat.com>
>> ---
>> include/monitor/monitor.h | 5 ----
>> monitor.c | 68
>> ++---------------------------------------------
>> 2 files changed, 2 insertions(+), 71 deletions(-)
>
> Certainly takes an axe to it!
>
> Reviewed-by: Eric Blake <ebl...@redhat.com>
Thanks!
