[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 01/20] monitor: Drop broken, unused asynchronous
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 01/20] monitor: Drop broken, unused asynchronous command interface |
Date: |
Tue, 26 May 2015 11:00:37 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
Eric Blake <address@hidden> 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 <address@hidden>
>> ---
>> 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 <address@hidden>
Thanks!
- Re: [Qemu-devel] [PATCH 06/20] monitor: Use traditional command interface for HMP device_add, (continued)
- [Qemu-devel] [PATCH 05/20] monitor: Use traditional command interface for HMP drive_del, Markus Armbruster, 2015/05/22
- [Qemu-devel] [PATCH 14/20] monitor: Rename handle_user_command() to handle_hmp_command(), Markus Armbruster, 2015/05/22
- [Qemu-devel] [PATCH 07/20] monitor: Use trad. command interface for HMP pcie_aer_inject_error, Markus Armbruster, 2015/05/22
- [Qemu-devel] [PATCH 01/20] monitor: Drop broken, unused asynchronous command interface, Markus Armbruster, 2015/05/22
- [Qemu-devel] [PATCH 09/20] monitor: Propagate errors through qmp_check_client_args(), Markus Armbruster, 2015/05/22
- [Qemu-devel] [PATCH 18/20] monitor: Turn int command_mode into bool in_command_mode, Markus Armbruster, 2015/05/22
- [Qemu-devel] [PATCH 19/20] monitor: Rename monitor_ctrl_mode() to monitor_is_qmp(), Markus Armbruster, 2015/05/22
- [Qemu-devel] [PATCH 13/20] monitor: Limit QError use to command handlers, Markus Armbruster, 2015/05/22