monitor: Wean core off QError, and other cleanups
v2 cover monitor: Wean core off QError, and other cleanups
Command handlers still use QError. Left for another day.
v2:
* Trivially rebased
* PATCH 01: Drop another async remnant [Eric]
* PATCH 01+02+18: Improve commit messages
* PATCH 03+04: client_migrate_info still hasn't been implemented VNC, de-document [Eric, Gerd]
* PATCH 16+19: Don't inline monitor_ctrl_mode() into monitor_init() [Eric]
* PATCH 20: Use false instead of 0 [Eric]
Markus Armbruster (20):
monitor: Drop broken, unused asynchronous command interface
monitor: Clean up after previous commit
monitor: Improve and document client_migrate_info protocol error
monitor: Convert client_migrate_info to QAPI
monitor: Use traditional command interface for HMP drive_del
monitor: Use traditional command interface for HMP device_add
monitor: Use trad. command interface for HMP pcie_aer_inject_error
monitor: Drop unused "new" HMP command interface
monitor: Propagate errors through qmp_check_client_args()
monitor: Propagate errors through qmp_check_input_obj()
monitor: Wean monitor_protocol_emitter() off mon->error
monitor: Inline monitor_has_error() into its only caller
monitor: Limit QError use to command handlers
monitor: Rename handle_user_command() to handle_hmp_command()
monitor: Rename monitor_control_read(), monitor_control_event()
monitor: Unbox Monitor member mc and rename to qmp
monitor: Drop do_qmp_capabilities()'s superfluous QMP check
monitor: Turn int command_mode into bool in_command_mode
monitor: Rename monitor_ctrl_mode() to monitor_is_qmp()
monitor: Change return type of monitor_cur_is_qmp() to bool
blockdev.c | 9 +- hmp-commands.hx | 20 +-- hmp.c | 23 +++ hmp.h | 2 + hw/pci/pci-stub.c | 14 +- hw/pci/pcie_aer.c | 39 ++--- include/monitor/monitor.h | 7 +- include/sysemu/blockdev.h | 2 +- include/sysemu/sysemu.h | 4 +- monitor.c | 380 ++++++++++++++++------------------------------ qapi-schema.json | 19 +++ qmp-commands.hx | 16 +- stubs/mon-is-qmp.c | 4 +- 13 files changed, 222 insertions(+), 317 deletions(-)
Signature · 1 line
1.9.3
PATCH v2 1/20 monitor: Drop broken, unused asynchronous command interface
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.
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 commit 3b5704b. Run QEMU with spice:
$ qemu-system-x86_64 -nodefaults -S -spice port=5900,disable-ticketing -qmp stdio
{"QMP": {"version": {"qemu": {"micro": 94, "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", "id": "i0" }
{ "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": {}, "id": "i0"}
{"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 <[email protected]>
Reviewed-by Eric Blake <[email protected]>
--- include/monitor/monitor.h | 5 ---- monitor.c | 70 ++--------------------------------------------- 2 files changed, 2 insertions(+), 73 deletions(-)
include/monitor/monitor.h +0 -5
@@ -16,9 +16,6 @@ extern Monitor *default_mon;#define MONITOR_USE_CONTROL 0x04#define MONITOR_USE_PRETTY 0x08 -/* flags for monitor commands */ -#define MONITOR_CMD_ASYNC 0x0001 -int monitor_cur_is_qmp (void );void monitor_init (CharDriverState *chr,int flags);
@@ -43,8 +40,6 @@ void monitor_flush(Monitor *mon);int monitor_set_cpu (int cpu_index);int monitor_get_cpu_index (void ); -typedef void (MonitorCompletion )(void *opaque,QObject *ret_data); -void monitor_set_error (Monitor *mon,QError *qerror);void monitor_read_command (Monitor *mon,int show_prompt); int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
monitor.c +2 -68
113 lines not on this page · raw message
Signature · 1 line
1.9.3
PATCH v2 2/20 monitor: Clean up after previous commit
In reply to Markus ArmbrusterInline qmp_call_cmd() along with its helper handler_audit() into its only caller handle_qmp_command(), and simplify the result.
Signed-off-by Markus Armbruster <[email protected]>
Reviewed-by Eric Blake <[email protected]>
--- monitor.c | 40 +++++++++++----------------------------- 1 file changed, 11 insertions(+), 29 deletions(-)
monitor.c +11 -29
@@ -4045,18 +4045,6 @@ void monitor_set_error(Monitor *mon, QError *qerror) } } -static void handler_audit (Monitor *mon,const mon_cmd_t *cmd,int ret) -{ -if (ret && !monitor_has_error (mon)) { -/* -* If it returns failure, it must have passed on error. -* -* Action: Report an internal error to the client if in QMP. -*/ -qerror_report (QERR_UNDEFINED_ERROR ); - } -} -static void handle_user_command (Monitor *mon,const char *cmdline) {QDict *qdict;
@@ -5013,28 +5001,17 @@ static QDict *qmp_check_input_obj(QObject *input_obj)return input_dict; } -static void qmp_call_cmd (Monitor *mon,const mon_cmd_t *cmd, -const QDict *params) -{ -int ret; -QObject *data =NULL ; - - ret = cmd->mhandler.cmd_new (mon, params, &data); -handler_audit (mon, cmd, ret); -monitor_protocol_emitter (mon, data); -qobject_decref (data); -} -static void handle_qmp_command (JSONMessageParser *parser,QList *tokens) {int err; -QObject *obj; +QObject *obj, *data;QDict *input, *args;const mon_cmd_t *cmd;const char *cmd_name;Monitor *mon = cur_mon; args = input =NULL ; + data =NULL ; obj =json_parser_parse (tokens,NULL );if (!obj) {
@@ -5077,12 +5054,17 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens) goto err_out; } -qmp_call_cmd (mon, cmd, args); - goto out; +if (cmd->mhandler.cmd_new (mon, args, &data)) { +/* Command failed... */ +if (!monitor_has_error (mon)) { +/* ... without setting an error, so make one up */ +qerror_report (QERR_UNDEFINED_ERROR ); + } + } err_out: -monitor_protocol_emitter (mon,NULL ); -out: +monitor_protocol_emitter (mon, data); +qobject_decref (data);QDECREF (input);QDECREF (args); }
Signature · 1 line
1.9.3
PATCH v2 3/20 monitor: Improve and document client_migrate_info protocol error
In reply to Markus ArmbrusterProtocol must be spice, vnc isn't implemented. Fix up documentation.
Attempts to use vnc or any other unknown protocol yield the misleading error message "Invalid parameter 'protocol'". Improve it to "Parameter 'protocol' expects spice".
Cc Gerd Hoffmann <[email protected]>
Signed-off-by Markus Armbruster <[email protected]>
Reviewed-by Eric Blake <[email protected]>
--- hmp-commands.hx | 8 ++++---- monitor.c | 2 +- qmp-commands.hx | 14 +++++++------- 3 files changed, 12 insertions(+), 12 deletions(-)
hmp-commands.hx +4 -4
@@ -1011,7 +1011,7 @@ ETEXI .name = "client_migrate_info", .args_type = "protocol:s,hostname:s,port:i?,tls-port:i?,cert-subject:s?", .params = "protocol hostname port tls-port cert-subject", - .help = "send migration info to spice/vnc client", + .help = "set migration information for remote display", .user_print = monitor_user_noop, .mhandler.cmd_new = client_migrate_info, },
@@ -1019,9 +1019,9 @@ ETEXI STEXI @item client_migrate_info @var{protocol} @var{hostname} @var{port} @var{tls-port} @var{cert-subject} @findex client_migrate_info -Set the spice/vnc connection info for the migration target. The spice/vnc -server will ask the spice/vnc client to automatically reconnect using the -new parameters (if specified) once the vm migration finished successfully. +Set migration information for remote display. This makes the server +ask the client to automatically reconnect using the new parameters +once migration finished successfully. Only implemented for SPICE. ETEXI {
monitor.c +1 -1
@@ -1063,7 +1063,7 @@ static int client_migrate_info(Monitor *mon, const QDict *qdict,return 0 ; } -qerror_report (QERR_INVALID_PARAMETER ,"protocol" ); +qerror_report (QERR_INVALID_PARAMETER_VALUE ,"protocol" ,"spice" );return -1 ; }
qmp-commands.hx +7 -7
@@ -784,23 +784,23 @@ EQMP .name = "client_migrate_info", .args_type = "protocol:s,hostname:s,port:i?,tls-port:i?,cert-subject:s?", .params = "protocol hostname port tls-port cert-subject", - .help = "send migration info to spice/vnc client", + .help = "set migration information for remote display", .mhandler.cmd_new = client_migrate_info, }, SQMP client_migrate_info ------------------- +------------------- -Set the spice/vnc connection info for the migration target. The spice/vnc -server will ask the spice/vnc client to automatically reconnect using the -new parameters (if specified) once the vm migration finished successfully. +Set migration information for remote display. This makes the server +ask the client to automatically reconnect using the new parameters +once migration finished successfully. Only implemented for SPICE. Arguments: -- "protocol": protocol: "spice" or "vnc" (json-string) +- "protocol": must be "spice" (json-string) - "hostname": migration target hostname (json-string) -- "port": spice/vnc tcp port for plaintext channels (json-int, optional) +- "port": spice tcp port for plaintext channels (json-int, optional) - "tls-port": spice tcp port for tls-secured channels (json-int, optional) - "cert-subject": server certificate subject (json-string, optional)
Signature · 1 line
1.9.3
Re: [Qemu-devel] [PATCH v2 03/20] monitor: Improve and document client_migrate_info protocol error
Quoted 5 lines · Markus Armbruster
Protocol must be spice, vnc isn't implemented. Fix up documentation.
Attempts to use vnc or any other unknown protocol yield the misleading error message "Invalid parameter 'protocol'". Improve it to "Parameter 'protocol' expects spice".
Reviewed-by. Gerd Hoffmann <[email protected]>
PATCH v2 4/20 monitor: Convert client_migrate_info to QAPI
In reply to Markus ArmbrusterSigned-off-by Markus Armbruster <[email protected]>
Reviewed-by Eric Blake <[email protected]>
--- hmp-commands.hx | 3 +-- hmp.c | 17 +++++++++++++++++ hmp.h | 1 + monitor.c | 42 ++++++++++++++++++------------------------ qapi-schema.json | 19 +++++++++++++++++++ qmp-commands.hx | 2 +- 6 files changed, 57 insertions(+), 27 deletions(-)
hmp-commands.hx +1 -2
@@ -1012,8 +1012,7 @@ ETEXI .args_type = "protocol:s,hostname:s,port:i?,tls-port:i?,cert-subject:s?", .params = "protocol hostname port tls-port cert-subject", .help = "set migration information for remote display", - .user_print = monitor_user_noop, - .mhandler.cmd_new = client_migrate_info, + .mhandler.cmd = hmp_client_migrate_info, }, STEXI
hmp.c +17 -0
@@ -1250,6 +1250,23 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) } } +void hmp_client_migrate_info (Monitor *mon,const QDict *qdict) +{ +Error *err =NULL ; +const char *protocol =qdict_get_str (qdict,"protocol" ); +const char *hostname =qdict_get_str (qdict,"hostname" ); +bool has_port =qdict_haskey (qdict,"port" ); +int port =qdict_get_try_int (qdict,"port" ,-1 ); +bool has_tls_port =qdict_haskey (qdict,"tls-port" ); +int tls_port =qdict_get_try_int (qdict,"tls-port" ,-1 ); +const char *cert_subject =qdict_get_try_str (qdict,"cert-subject" ); + +qmp_client_migrate_info (protocol, hostname, + has_port, port, has_tls_port, tls_port, + !!cert_subject, cert_subject, &err); +hmp_handle_error (mon, &err); +} +void hmp_set_password (Monitor *mon,const QDict *qdict) {const char *protocol =qdict_get_str (qdict,"protocol" );
hmp.h +1 -0
@@ -67,6 +67,7 @@ void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict);void hmp_migrate_set_capability (Monitor *mon,const QDict *qdict);void hmp_migrate_set_parameter (Monitor *mon,const QDict *qdict);void hmp_migrate_set_cache_size (Monitor *mon,const QDict *qdict); +void hmp_client_migrate_info (Monitor *mon,const QDict *qdict);void hmp_set_password (Monitor *mon,const QDict *qdict);void hmp_expire_password (Monitor *mon,const QDict *qdict);void hmp_eject (Monitor *mon,const QDict *qdict);
monitor.c +18 -24
57 lines not on this page · raw message
qapi-schema.json +19 -0
@@ -638,6 +638,25 @@ 'returns': 'MigrationParameters' } ## +# @client_migrate_info +# +# Set migration information for remote display. This makes the server +# ask the client to automatically reconnect using the new parameters +# once migration finished successfully. Only implemented for SPICE. +# +# @protocol: must be "spice" +# @hostname: migration target hostname +# @port: #optional spice tcp port for plaintext channels +# @tls-port: #optional spice tcp port for tls-secured channels +# @cert-subject: #optional server certificate subject +# +# Since: 0.14.0 +## +{ 'command': 'client_migrate_info', + 'data': { 'protocol': 'str', 'hostname': 'str', '*port': 'int', + '*tls-port': 'int', '*cert-subject': 'str' } } + +## # @MouseInfo: # # Information about a mouse device.
qmp-commands.hx +1 -1
@@ -785,7 +785,7 @@ EQMP .args_type = "protocol:s,hostname:s,port:i?,tls-port:i?,cert-subject:s?", .params = "protocol hostname port tls-port cert-subject", .help = "set migration information for remote display", - .mhandler.cmd_new = client_migrate_info, + .mhandler.cmd_new = qmp_marshal_input_client_migrate_info, }, SQMP
Signature · 1 line
1.9.3
Re: [Qemu-devel] [PATCH v2 04/20] monitor: Convert client_migrate_info to QAPI
Quoted 115 lines · On Tue, 26 May 2015 17:20:39 +0200 Markus Armbruster <[email protected]> wrote:
Signed-off-by Markus Armbruster <[email protected]>Reviewed-by Eric Blake <[email protected]>--- hmp-commands.hx | 3 +-- hmp.c | 17 +++++++++++++++++ hmp.h | 1 + monitor.c | 42 ++++++++++++++++++------------------------ qapi-schema.json | 19 +++++++++++++++++++ qmp-commands.hx | 2 +- 6 files changed, 57 insertions(+), 27 deletions(-)hmp-commands.hx +1
-2@@ -1012,8 +1012,7 @@ ETEXI .args_type = "protocol:s,hostname:s,port:i?,tls-port:i?,cert-subject:s?", .params = "protocol hostname port tls-port cert-subject", .help = "set migration information for remote display", - .user_print = monitor_user_noop, - .mhandler.cmd_new = client_migrate_info, + .mhandler.cmd = hmp_client_migrate_info, }, STEXIhmp.c +17
-0@@ -1250,6 +1250,23 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) } } +void hmp_client_migrate_info (Monitor *mon,const QDict *qdict) +{ +Error *err =NULL ; +const char *protocol =qdict_get_str (qdict,"protocol" ); +const char *hostname =qdict_get_str (qdict,"hostname" ); +bool has_port =qdict_haskey (qdict,"port" ); +int port =qdict_get_try_int (qdict,"port" ,-1 ); +bool has_tls_port =qdict_haskey (qdict,"tls-port" ); +int tls_port =qdict_get_try_int (qdict,"tls-port" ,-1 ); +const char *cert_subject =qdict_get_try_str (qdict,"cert-subject" ); + +qmp_client_migrate_info (protocol, hostname, + has_port, port, has_tls_port, tls_port, + !!cert_subject, cert_subject, &err); +hmp_handle_error (mon, &err); +} +void hmp_set_password (Monitor *mon,const QDict *qdict) {const char *protocol =qdict_get_str (qdict,"protocol" );hmp.h +1
-0@@ -67,6 +67,7 @@ void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict);void hmp_migrate_set_capability (Monitor *mon,const QDict *qdict);void hmp_migrate_set_parameter (Monitor *mon,const QDict *qdict);void hmp_migrate_set_cache_size (Monitor *mon,const QDict *qdict); +void hmp_client_migrate_info (Monitor *mon,const QDict *qdict);void hmp_set_password (Monitor *mon,const QDict *qdict);void hmp_expire_password (Monitor *mon,const QDict *qdict);void hmp_eject (Monitor *mon,const QDict *qdict);monitor.c +15
-21@@ -1032,39 +1032,33 @@ static void hmp_info_trace_events(Monitor *mon, const QDict *qdict)qapi_free_TraceEventInfoList (events); } -static int client_migrate_info (Monitor *mon,const QDict *qdict, -QObject **ret_data) +void qmp_client_migrate_info (const char *protocol,const char *hostname, +bool has_port,int64_t port, +bool has_tls_port,int64_t tls_port, +bool has_cert_subject,const char *cert_subject, +Error **errp) { -const char *protocol =qdict_get_str (qdict,"protocol" ); -const char *hostname =qdict_get_str (qdict,"hostname" ); -const char *subject =qdict_get_try_str (qdict,"cert-subject" ); -int port =qdict_get_try_int (qdict,"port" ,-1 ); -int tls_port =qdict_get_try_int (qdict,"tls-port" ,-1 ); -Error *err =NULL ; -int ret; -if (strcmp (protocol,"spice" ) ==0 ) { -if (!qemu_using_spice (&err)) { -qerror_report_err (err); -error_free (err); -return -1 ; +if (!qemu_using_spice (errp)) { +return ; } -if (port ==-1 && tls_port ==-1 ) { -qerror_report (QERR_MISSING_PARAMETER ,"port/tls-port" ); -return -1 ; +if (!has_port && !has_tls_port) { +error_set (errp,QERR_MISSING_PARAMETER ,"port/tls-port" ); +return ; } - ret =qemu_spice_migrate_info (hostname, port, tls_port, subject); -if (ret !=0 ) { -qerror_report (QERR_UNDEFINED_ERROR ); -return -1 ; +if (qemu_spice_migrate_info (hostname, + has_port ? port :-1 , + has_tls_port ? tls_port :-1 , + cert_subject)) { +error_set (errp,QERR_UNDEFINED_ERROR );
monitor.c static void hmp_info_trace_events(Monitor *mon, const QDict *qdict) · line 1062
Quoted 10 lines
monitor.c +3
-3+return ; } -return 0 ; +return ; } -qerror_report (QERR_INVALID_PARAMETER_VALUE ,"protocol" ,"spice" ); -return -1 ; +error_set (errp,QERR_INVALID_PARAMETER_VALUE ,"protocol" ,"spice" ); }
Why not use error_setg() instead of error_set()?
Quoted 45 lines
monitor.c +0
-0static void hmp_logfile (Monitor *mon,const QDict *qdict)qapi-schema.json +19
-0@@ -638,6 +638,25 @@ 'returns': 'MigrationParameters' } ## +# @client_migrate_info +# +# Set migration information for remote display. This makes the server +# ask the client to automatically reconnect using the new parameters +# once migration finished successfully. Only implemented for SPICE. +# +# @protocol: must be "spice" +# @hostname: migration target hostname +# @port: #optional spice tcp port for plaintext channels +# @tls-port: #optional spice tcp port for tls-secured channels +# @cert-subject: #optional server certificate subject +# +# Since: 0.14.0 +## +{ 'command': 'client_migrate_info', + 'data': { 'protocol': 'str', 'hostname': 'str', '*port': 'int', + '*tls-port': 'int', '*cert-subject': 'str' } } + +## # @MouseInfo: # # Information about a mouse device.qmp-commands.hx +1
-1@@ -785,7 +785,7 @@ EQMP .args_type = "protocol:s,hostname:s,port:i?,tls-port:i?,cert-subject:s?", .params = "protocol hostname port tls-port cert-subject", .help = "set migration information for remote display", - .mhandler.cmd_new = client_migrate_info, + .mhandler.cmd_new = qmp_marshal_input_client_migrate_info, }, SQMP
Re: [Qemu-devel] [PATCH v2 04/20] monitor: Convert client_migrate_info to QAPI
Quoted 5 lines · Luiz Capitulino
Quoted 2 lines · On Tue, 26 May 2015 17:20:39 +0200 Markus Armbruster <[email protected]> wrote:
Signed-off-by Markus Armbruster <[email protected]>Reviewed-by Eric Blake <[email protected]>
[...]
Quoted 55 lines
Quoted 55 lines
index 8170309..38ff972 100644
monitor.c +17
-22@@ -1032,39 +1032,33 @@ static void hmp_info_trace_events(Monitor *mon, const QDict *qdict)qapi_free_TraceEventInfoList (events); } -static int client_migrate_info (Monitor *mon,const QDict *qdict, -QObject **ret_data) +void qmp_client_migrate_info (const char *protocol,const char *hostname, +bool has_port,int64_t port, +bool has_tls_port,int64_t tls_port, +bool has_cert_subject,const char *cert_subject, +Error **errp) { -const char *protocol =qdict_get_str (qdict,"protocol" ); -const char *hostname =qdict_get_str (qdict,"hostname" ); -const char *subject =qdict_get_try_str (qdict,"cert-subject" ); -int port =qdict_get_try_int (qdict,"port" ,-1 ); -int tls_port =qdict_get_try_int (qdict,"tls-port" ,-1 ); -Error *err =NULL ; -int ret; -if (strcmp (protocol,"spice" ) ==0 ) { -if (!qemu_using_spice (&err)) { -qerror_report_err (err); -error_free (err); -return -1 ; +if (!qemu_using_spice (errp)) { +return ; } -if (port ==-1 && tls_port ==-1 ) { -qerror_report (QERR_MISSING_PARAMETER ,"port/tls-port" ); -return -1 ; +if (!has_port && !has_tls_port) { +error_set (errp,QERR_MISSING_PARAMETER ,"port/tls-port" ); +return ; } - ret =qemu_spice_migrate_info (hostname, port, tls_port, subject); -if (ret !=0 ) { -qerror_report (QERR_UNDEFINED_ERROR ); -return -1 ; +if (qemu_spice_migrate_info (hostname, + has_port ? port :-1 , + has_tls_port ? tls_port :-1 , + cert_subject)) { +error_set (errp,QERR_UNDEFINED_ERROR ); +return ; } -return 0 ; +return ; }
Quoted 6 lines
monitor.c static void hmp_info_trace_events(Monitor *mon, const QDict *qdict) · line 1062
Quoted 4 lines
monitor.c +1
-2-qerror_report (QERR_INVALID_PARAMETER_VALUE ,"protocol" ,"spice" ); -return -1 ; +error_set (errp,QERR_INVALID_PARAMETER_VALUE ,"protocol" ,"spice" ); }Why not use
error_setg()instead oferror_set()?
Just to keep this patch more obvious.
I agree the QERR_ macros need to go, but that'll take a separate series, and only after I managed to empty out the rest of qerror.h.
[...]
PATCH v2 5/20 monitor: Use traditional command interface for HMP drive_del
In reply to Markus ArmbrusterAll QMP commands use the "new" handler interface (mhandler.cmd_new). Most HMP commands still use the traditional interface (mhandler.cmd), but a few use the "new" one. Complicates handle_user_command() for no gain, so I'm converting these to the traditional interface.
For drive_del, that's easy: hmp_drive_del() sheds its unused last parameter, and its return value, which the caller ignored anyway.
Signed-off-by Markus Armbruster <[email protected]>
Reviewed-by Eric Blake <[email protected]>
--- blockdev.c | 9 ++++----- hmp-commands.hx | 3 +-- include/sysemu/blockdev.h | 2 +- 3 files changed, 6 insertions(+), 8 deletions(-)
blockdev.c +4 -5
@@ -2113,7 +2113,7 @@ void qmp_block_dirty_bitmap_clear(const char *node, const char *name,aio_context_release (aio_context); } -int hmp_drive_del (Monitor *mon,const QDict *qdict,QObject **ret_data) +void hmp_drive_del (Monitor *mon,const QDict *qdict) {const char *id =qdict_get_str (qdict,"id" );BlockBackend *blk;
@@ -2124,14 +2124,14 @@ int hmp_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data) blk =blk_by_name (id);if (!blk) {error_report ("Device '%s' not found" , id); -return -1 ; +return ; } bs =blk_bs (blk);if (!blk_legacy_dinfo (blk)) {error_report ("Deleting device added with blockdev-add" " is not supported" ); -return -1 ; +return ; } aio_context =bdrv_get_aio_context (bs);
@@ -2140,7 +2140,7 @@ int hmp_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)if (bdrv_op_is_blocked (bs,BLOCK_OP_TYPE_DRIVE_DEL , &local_err)) {error_report_err (local_err);aio_context_release (aio_context); -return -1 ; +return ; }/* quiesce block driver; prevent further io */
@@ -2163,7 +2163,6 @@ int hmp_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data) }aio_context_release (aio_context); -return 0 ; } void qmp_block_resize(bool has_device, const char *device,
hmp-commands.hx +1 -2
@@ -178,8 +178,7 @@ ETEXI .args_type = "id:B", .params = "device", .help = "remove host block device", - .user_print = monitor_user_noop, - .mhandler.cmd_new = hmp_drive_del, + .mhandler.cmd = hmp_drive_del, }, STEXI
include/sysemu/blockdev.h +1 -1
@@ -66,5 +66,5 @@ DriveInfo *drive_new(QemuOpts *arg, BlockInterfaceType block_default_type);void qmp_change_blockdev (const char *device,const char *filename,const char *format,Error **errp);void hmp_commit (Monitor *mon,const QDict *qdict); -int hmp_drive_del (Monitor *mon,const QDict *qdict,QObject **ret_data); +void hmp_drive_del (Monitor *mon,const QDict *qdict);#endif
Signature · 1 line
1.9.3
PATCH v2 6/20 monitor: Use traditional command interface for HMP device_add
In reply to Markus ArmbrusterAll QMP commands use the "new" handler interface (mhandler.cmd_new). Most HMP commands still use the traditional interface (mhandler.cmd), but a few use the "new" one. Complicates handle_user_command() for no gain, so I'm converting these to the traditional interface.
For device_add, that's easy: just wrap the obvious hmp_device_add() around do_device_add().
monitor_user_noop() is now unused, drop it.
Signed-off-by Markus Armbruster <[email protected]>
Reviewed-by Eric Blake <[email protected]>
--- hmp-commands.hx | 3 +-- hmp.c | 6 ++++++ hmp.h | 1 + monitor.c | 2 -- 4 files changed, 8 insertions(+), 4 deletions(-)
hmp-commands.hx +1 -2
@@ -653,8 +653,7 @@ ETEXI .args_type = "device:O", .params = "driver[,prop=value][,...]", .help = "add device, like -device on the command line", - .user_print = monitor_user_noop, - .mhandler.cmd_new = do_device_add, + .mhandler.cmd = hmp_device_add, .command_completion = device_add_completion, },
hmp.c +6 -0
@@ -22,6 +22,7 @@#include "qmp-commands.h" #include "qemu/sockets.h" #include "monitor/monitor.h" +#include "monitor/qdev.h" #include "qapi/opts-visitor.h" #include "qapi/string-output-visitor.h" #include "qapi-visit.h"
@@ -1499,6 +1500,11 @@ void hmp_migrate(Monitor *mon, const QDict *qdict) } } +void hmp_device_add (Monitor *mon,const QDict *qdict) +{ +do_device_add (mon, qdict,NULL ); +} +void hmp_device_del (Monitor *mon,const QDict *qdict) {const char *id =qdict_get_str (qdict,"id" );
hmp.h +1 -0
@@ -80,6 +80,7 @@ void hmp_block_job_pause(Monitor *mon, const QDict *qdict);void hmp_block_job_resume (Monitor *mon,const QDict *qdict);void hmp_block_job_complete (Monitor *mon,const QDict *qdict);void hmp_migrate (Monitor *mon,const QDict *qdict); +void hmp_device_add (Monitor *mon,const QDict *qdict);void hmp_device_del (Monitor *mon,const QDict *qdict);void hmp_dump_guest_memory (Monitor *mon,const QDict *qdict);void hmp_netdev_add (Monitor *mon,const QDict *qdict);
monitor.c +0 -2
@@ -378,8 +378,6 @@ static int GCC_FMT_ATTR(2, 3) monitor_fprintf(FILE *stream,return 0 ; } -static void monitor_user_noop (Monitor *mon,const QObject *data) { } -static inline int handler_is_qobject (const mon_cmd_t *cmd) {return cmd->user_print !=NULL ;
Signature · 1 line
1.9.3
PATCH v2 7/20 monitor: Use trad. command interface for HMP pcie_aer_inject_error
In reply to Markus ArmbrusterAll QMP commands use the "new" handler interface (mhandler.cmd_new). Most HMP commands still use the traditional interface (mhandler.cmd), but a few use the "new" one. Complicates handle_user_command() for no gain, so I'm converting these to the traditional interface.
pcie_aer_inject_error's implementation is split into the hmp_pcie_aer_inject_error() and pcie_aer_inject_error_print(). The former is a peculiar crossbreed between HMP and QMP handler. On success, it works like a QMP handler: store QDict through ret_data parameter, return 0. Printing the QDict is left to pcie_aer_inject_error_print(). On failure, it works more like an HMP
handler: print error to monitor, return negative number.
To convert to the traditional interface, turn pcie_aer_inject_error_print() into a command handler wrapping around hmp_pcie_aer_inject_error(). By convention, this command handler should be called hmp_pcie_aer_inject_error(), so rename the existing hmp_pcie_aer_inject_error() to do_pcie_aer_inject_error().
Signed-off-by Markus Armbruster <[email protected]>
Reviewed-by Eric Blake <[email protected]>
--- hmp-commands.hx | 3 +-- hw/pci/pci-stub.c | 14 +------------- hw/pci/pcie_aer.c | 39 ++++++++++++++++++++++----------------- include/sysemu/sysemu.h | 4 +--- 4 files changed, 25 insertions(+), 35 deletions(-)
hmp-commands.hx +1 -2
@@ -1183,8 +1183,7 @@ ETEXI "<error_status> = error string or 32bit\n\t\t\t" "<tlb header> = 32bit x 4\n\t\t\t" "<tlb header prefix> = 32bit x 4", - .user_print = pcie_aer_inject_error_print, - .mhandler.cmd_new = hmp_pcie_aer_inject_error, + .mhandler.cmd = hmp_pcie_aer_inject_error, }, STEXI
hw/pci/pci-stub.c +1 -13
@@ -29,19 +29,7 @@ PciInfoList *qmp_query_pci(Error **errp)return NULL ; } -static void pci_error_message (Monitor *mon) +void hmp_pcie_aer_inject_error (Monitor *mon,const QDict *qdict) {monitor_printf (mon,"PCI devices not supported\n" ); } - -int hmp_pcie_aer_inject_error (Monitor *mon, -const QDict *qdict,QObject **ret_data) -{ -pci_error_message (mon); -return -ENOSYS ; -} - -void pcie_aer_inject_error_print (Monitor *mon,const QObject *data) -{ -pci_error_message (mon); -}
hw/pci/pcie_aer.c +22 -17
@@ -815,21 +815,6 @@ const VMStateDescription vmstate_pcie_aer_log = { } }; -void pcie_aer_inject_error_print (Monitor *mon,const QObject *data) -{ -QDict *qdict; -int devfn; -assert (qobject_type (data) ==QTYPE_QDICT ); - qdict =qobject_to_qdict (data); - - devfn = (int )qdict_get_int (qdict,"devfn" ); -monitor_printf (mon,"OK id: %s root bus: %s, bus: %x devfn: %x.%x\n" , -qdict_get_str (qdict,"id" ), -qdict_get_str (qdict,"root_bus" ), - (int )qdict_get_int (qdict,"bus" ), -PCI_SLOT (devfn),PCI_FUNC (devfn)); -} - typedef struct PCIEAERErrorName { const char *name; uint32_t val;
@@ -962,8 +947,8 @@ static int pcie_aer_parse_error_string(const char *error_name,return -EINVAL ; } -int hmp_pcie_aer_inject_error (Monitor *mon, -const QDict *qdict,QObject **ret_data) +static int do_pcie_aer_inject_error (Monitor *mon, +const QDict *qdict,QObject **ret_data) {const char *id =qdict_get_str (qdict,"id" );const char *error_name;
@@ -1035,3 +1020,23 @@ int hmp_pcie_aer_inject_error(Monitor *mon,return 0 ; } + +void hmp_pcie_aer_inject_error (Monitor *mon,const QDict *qdict) +{ +QObject *data; +int devfn; + +if (do_pcie_aer_inject_error (mon, qdict, &data) <0 ) { +return ; + } + +assert (qobject_type (data) ==QTYPE_QDICT ); + qdict =qobject_to_qdict (data); + + devfn = (int )qdict_get_int (qdict,"devfn" ); +monitor_printf (mon,"OK id: %s root bus: %s, bus: %x devfn: %x.%x\n" , +qdict_get_str (qdict,"id" ), +qdict_get_str (qdict,"root_bus" ), + (int )qdict_get_int (qdict,"bus" ), +PCI_SLOT (devfn),PCI_FUNC (devfn)); +}
include/sysemu/sysemu.h +1 -3
@@ -161,9 +161,7 @@ extern unsigned int nb_prom_envs;void hmp_drive_add (Monitor *mon,const QDict *qdict);/* pcie aer error injection */ -void pcie_aer_inject_error_print (Monitor *mon,const QObject *data); -int hmp_pcie_aer_inject_error (Monitor *mon, -const QDict *qdict,QObject **ret_data); +void hmp_pcie_aer_inject_error (Monitor *mon,const QDict *qdict);/* serial ports */
Signature · 1 line
1.9.3
PATCH v2 8/20 monitor: Drop unused "new" HMP command interface
In reply to Markus ArmbrusterSigned-off-by Markus Armbruster <[email protected]>
Reviewed-by Eric Blake <[email protected]>
--- monitor.c | 22 +--------------------- 1 file changed, 1 insertion(+), 21 deletions(-)
monitor.c +1 -21
@@ -123,7 +123,6 @@ typedef struct mon_cmd_t {const char *args_type;const char *params;const char *help; -void (*user_print)(Monitor *mon,const QObject *data);union {void (*cmd)(Monitor *mon,const QDict *qdict);int (*cmd_new)(Monitor *mon,const QDict *params,QObject **ret_data);
@@ -378,11 +377,6 @@ static int GCC_FMT_ATTR(2, 3) monitor_fprintf(FILE *stream,return 0 ; } -static inline int handler_is_qobject (const mon_cmd_t *cmd) -{ -return cmd->user_print !=NULL ; -} -static inline int monitor_has_error (const Monitor *mon) {return mon->error !=NULL ;
@@ -4045,24 +4039,10 @@ static void handle_user_command(Monitor *mon, const char *cmdline) qdict =qdict_new (); cmd =monitor_parse_command (mon, cmdline,0 , mon->cmd_table, qdict); -if (!cmd) - goto out; - -if (handler_is_qobject (cmd)) { -QObject *data =NULL ; - -/* XXX: ignores the error code */ - cmd->mhandler.cmd_new (mon, qdict, &data); -assert (!monitor_has_error (mon)); -if (data) { - cmd->user_print (mon, data); -qobject_decref (data); - } - }else { +if (cmd) { cmd->mhandler.cmd (mon, qdict); } -out:QDECREF (qdict); }
Signature · 1 line
1.9.3
PATCH v2 9/20 monitor: Propagate errors through qmp_check_client_args()
In reply to Markus ArmbrusterSigned-off-by Markus Armbruster <[email protected]>
Reviewed-by Eric Blake <[email protected]>
--- monitor.c | 65 ++++++++++++++++++++++++++++++++------------------------------- 1 file changed, 33 insertions(+), 32 deletions(-)
monitor.c +33 -32
@@ -4736,8 +4736,9 @@ static bool invalid_qmp_mode(const Monitor *mon, const mon_cmd_t *cmd)* the QMP_ACCEPT_UNKNOWNS flag is set, then the * checking is skipped for it. */ -static int check_client_args_type (const QDict *client_args, -const QDict *cmd_args,int flags) +static void check_client_args_type (const QDict *client_args, +const QDict *cmd_args,int flags, +Error **errp) {const QDictEntry *ent;
@@ -4754,8 +4755,8 @@ static int check_client_args_type(const QDict *client_args,continue ; }/* client arg doesn't exist */ -qerror_report (QERR_INVALID_PARAMETER , client_arg_name); -return -1 ; +error_set (errp,QERR_INVALID_PARAMETER , client_arg_name); +return ; } arg_type =qobject_to_qstring (obj);
@@ -4767,9 +4768,9 @@ static int check_client_args_type(const QDict *client_args,case 'B' :case 's' :if (qobject_type (client_arg) !=QTYPE_QSTRING ) { -qerror_report (QERR_INVALID_PARAMETER_TYPE , client_arg_name, -"string" ); -return -1 ; +error_set (errp,QERR_INVALID_PARAMETER_TYPE , + client_arg_name,"string" ); +return ; }break ;case 'i' :
@@ -4777,25 +4778,25 @@ static int check_client_args_type(const QDict *client_args,case 'M' :case 'o' :if (qobject_type (client_arg) !=QTYPE_QINT ) { -qerror_report (QERR_INVALID_PARAMETER_TYPE , client_arg_name, -"int" ); -return -1 ; +error_set (errp,QERR_INVALID_PARAMETER_TYPE , + client_arg_name,"int" ); +return ; }break ;case 'T' :if (qobject_type (client_arg) !=QTYPE_QINT &&qobject_type (client_arg) !=QTYPE_QFLOAT ) { -qerror_report (QERR_INVALID_PARAMETER_TYPE , client_arg_name, -"number" ); -return -1 ; +error_set (errp,QERR_INVALID_PARAMETER_TYPE , + client_arg_name,"number" ); +return ; }break ;case 'b' :case '-' :if (qobject_type (client_arg) !=QTYPE_QBOOL ) { -qerror_report (QERR_INVALID_PARAMETER_TYPE , client_arg_name, -"bool" ); -return -1 ; +error_set (errp,QERR_INVALID_PARAMETER_TYPE , + client_arg_name,"bool" ); +return ; }break ;case 'O' :
@@ -4814,16 +4815,15 @@ static int check_client_args_type(const QDict *client_args,abort (); } } - -return 0 ; }/* * - Check if the client has passed all mandatory args * - Set special flags for argument validation */ -static int check_mandatory_args (const QDict *cmd_args, -const QDict *client_args,int *flags) +static void check_mandatory_args (const QDict *cmd_args, +const QDict *client_args,int *flags, +Error **errp) {const QDictEntry *ent;
@@ -4838,12 +4838,10 @@ static int check_mandatory_args(const QDict *cmd_args, }else if (qstring_get_str (type)[0 ] !='-' &&qstring_get_str (type)[1 ] !='?' && !qdict_haskey (client_args, cmd_arg_name)) { -qerror_report (QERR_MISSING_PARAMETER , cmd_arg_name); -return -1 ; +error_set (errp,QERR_MISSING_PARAMETER , cmd_arg_name); +return ; } } - -return 0 ; }static QDict *qdict_from_args_type (const char *args_type)
@@ -4899,24 +4897,26 @@ out:* 3. Each argument provided by the client must have the type expected * by the command */ -static int qmp_check_client_args (const mon_cmd_t *cmd,QDict *client_args) +static void qmp_check_client_args (const mon_cmd_t *cmd,QDict *client_args, +Error **errp) { -int flags, err; +Error *err =NULL ; +int flags;QDict *cmd_args; cmd_args =qdict_from_args_type (cmd->args_type); flags =0 ; - err =check_mandatory_args (cmd_args, client_args, &flags); +check_mandatory_args (cmd_args, client_args, &flags, &err);if (err) { goto out; } - err =check_client_args_type (client_args, cmd_args, flags); +check_client_args_type (client_args, cmd_args, flags, &err); out: +error_propagate (errp, err);QDECREF (cmd_args); -return err; }/*
@@ -4975,7 +4975,7 @@ static QDict *qmp_check_input_obj(QObject *input_obj)static void handle_qmp_command (JSONMessageParser *parser,QList *tokens) { -int err; +Error *local_err =NULL ;QObject *obj, *data;QDict *input, *args;const mon_cmd_t *cmd;
@@ -5021,8 +5021,9 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)QINCREF (args); } - err =qmp_check_client_args (cmd, args); -if (err <0 ) { +qmp_check_client_args (cmd, args, &local_err); +if (local_err) { +qerror_report_err (local_err); goto err_out; }
Signature · 1 line
1.9.3
Re: [Qemu-devel] [PATCH v2 09/20] monitor: Propagate errors through qmp_check_client_args()
Quoted 103 lines · On Tue, 26 May 2015 17:20:44 +0200 Markus Armbruster <[email protected]> wrote:
Signed-off-by Markus Armbruster <[email protected]>Reviewed-by Eric Blake <[email protected]>--- monitor.c | 65 ++++++++++++++++++++++++++++++++------------------------------- 1 file changed, 33 insertions(+), 32 deletions(-)monitor.c +20
-20@@ -4736,8 +4736,9 @@ static bool invalid_qmp_mode(const Monitor *mon, const mon_cmd_t *cmd)* the QMP_ACCEPT_UNKNOWNS flag is set, then the * checking is skipped for it. */ -static int check_client_args_type (const QDict *client_args, -const QDict *cmd_args,int flags) +static void check_client_args_type (const QDict *client_args, +const QDict *cmd_args,int flags, +Error **errp) {const QDictEntry *ent;@@ -4754,8 +4755,8 @@ static int check_client_args_type(const QDict *client_args,continue ; }/* client arg doesn't exist */ -qerror_report (QERR_INVALID_PARAMETER , client_arg_name); -return -1 ; +error_set (errp,QERR_INVALID_PARAMETER , client_arg_name); +return ; } arg_type =qobject_to_qstring (obj);@@ -4767,9 +4768,9 @@ static int check_client_args_type(const QDict *client_args,case 'B' :case 's' :if (qobject_type (client_arg) !=QTYPE_QSTRING ) { -qerror_report (QERR_INVALID_PARAMETER_TYPE , client_arg_name, -"string" ); -return -1 ; +error_set (errp,QERR_INVALID_PARAMETER_TYPE , + client_arg_name,"string" ); +return ; }break ;case 'i' :@@ -4777,25 +4778,25 @@ static int check_client_args_type(const QDict *client_args,case 'M' :case 'o' :if (qobject_type (client_arg) !=QTYPE_QINT ) { -qerror_report (QERR_INVALID_PARAMETER_TYPE , client_arg_name, -"int" ); -return -1 ; +error_set (errp,QERR_INVALID_PARAMETER_TYPE , + client_arg_name,"int" ); +return ; }break ;case 'T' :if (qobject_type (client_arg) !=QTYPE_QINT &&qobject_type (client_arg) !=QTYPE_QFLOAT ) { -qerror_report (QERR_INVALID_PARAMETER_TYPE , client_arg_name, -"number" ); -return -1 ; +error_set (errp,QERR_INVALID_PARAMETER_TYPE , + client_arg_name,"number" ); +return ; }break ;case 'b' :case '-' :if (qobject_type (client_arg) !=QTYPE_QBOOL ) { -qerror_report (QERR_INVALID_PARAMETER_TYPE , client_arg_name, -"bool" ); -return -1 ; +error_set (errp,QERR_INVALID_PARAMETER_TYPE , + client_arg_name,"bool" ); +return ; }break ;case 'O' :@@ -4814,16 +4815,15 @@ static int check_client_args_type(const QDict *client_args,abort (); } } - -return 0 ; }/* * - Check if the client has passed all mandatory args * - Set special flags for argument validation */ -static int check_mandatory_args (const QDict *cmd_args, -const QDict *client_args,int *flags) +static void check_mandatory_args (const QDict *cmd_args, +const QDict *client_args,int *flags, +Error **errp) {const QDictEntry *ent;@@ -4838,12 +4838,10 @@ static int check_mandatory_args(const QDict *cmd_args, } else if (qstring_get_str(type)[0] != '-' &&
monitor.c static int check_mandatory_args(const QDict *cmd_args, · line 4846
Quoted 10 lines
monitor.c +2
-4qstring_get_str(type)[1] != '?' && !qdict_haskey(client_args, cmd_arg_name)) { -qerror_report (QERR_MISSING_PARAMETER , cmd_arg_name); -return -1 ; +error_set (errp,QERR_MISSING_PARAMETER , cmd_arg_name); +return ; } } - -return 0 ;
I'd go from qerror_report() to error_setg(), but it's fine if you're saving this for a different series. I won't make this comment on the next patches, as this seems to be your intention.
Quoted 56 lines
monitor.c +11
-8}static QDict *qdict_from_args_type (const char *args_type)@@ -4899,24 +4897,26 @@ out:* 3. Each argument provided by the client must have the type expected * by the command */ -static int qmp_check_client_args (const mon_cmd_t *cmd,QDict *client_args) +static void qmp_check_client_args (const mon_cmd_t *cmd,QDict *client_args, +Error **errp) { -int flags, err; +Error *err =NULL ; +int flags;QDict *cmd_args; cmd_args =qdict_from_args_type (cmd->args_type); flags =0 ; - err =check_mandatory_args (cmd_args, client_args, &flags); +check_mandatory_args (cmd_args, client_args, &flags, &err);if (err) { goto out; } - err =check_client_args_type (client_args, cmd_args, flags); +check_client_args_type (client_args, cmd_args, flags, &err); out: +error_propagate (errp, err);QDECREF (cmd_args); -return err; }/* @@ -4975,7 +4975,7 @@ static QDict *qmp_check_input_obj(QObject *input_obj)static void handle_qmp_command (JSONMessageParser *parser,QList *tokens) { -int err; +Error *local_err =NULL ;QObject *obj, *data;QDict *input, *args;const mon_cmd_t *cmd;@@ -5021,8 +5021,9 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)QINCREF (args); } - err =qmp_check_client_args (cmd, args); -if (err <0 ) { +qmp_check_client_args (cmd, args, &local_err); +if (local_err) { +qerror_report_err (local_err); goto err_out; }
PATCH v2 10/20 monitor: Propagate errors through qmp_check_input_obj()
In reply to Markus ArmbrusterSigned-off-by Markus Armbruster <[email protected]>
Reviewed-by Eric Blake <[email protected]>
--- monitor.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-)
monitor.c +10 -9
@@ -4929,14 +4929,14 @@ out:* 5. If the "id" key exists, it can be anything (ie. json-value) * 6. Any argument not listed above is considered invalid */ -static QDict *qmp_check_input_obj (QObject *input_obj) +static QDict *qmp_check_input_obj (QObject *input_obj,Error **errp) {const QDictEntry *ent;int has_exec_key =0 ;QDict *input_dict;if (qobject_type (input_obj) !=QTYPE_QDICT ) { -qerror_report (QERR_QMP_BAD_INPUT_OBJECT ,"object" ); +error_set (errp,QERR_QMP_BAD_INPUT_OBJECT ,"object" );return NULL ; }
@@ -4948,25 +4948,25 @@ static QDict *qmp_check_input_obj(QObject *input_obj)if (!strcmp (arg_name,"execute" )) {if (qobject_type (arg_obj) !=QTYPE_QSTRING ) { -qerror_report (QERR_QMP_BAD_INPUT_OBJECT_MEMBER ,"execute" , -"string" ); +error_set (errp,QERR_QMP_BAD_INPUT_OBJECT_MEMBER , +"execute" ,"string" );return NULL ; } has_exec_key =1 ; }else if (!strcmp (arg_name,"arguments" )) {if (qobject_type (arg_obj) !=QTYPE_QDICT ) { -qerror_report (QERR_QMP_BAD_INPUT_OBJECT_MEMBER ,"arguments" , -"object" ); +error_set (errp,QERR_QMP_BAD_INPUT_OBJECT_MEMBER , +"arguments" ,"object" );return NULL ; } }else { -qerror_report (QERR_QMP_EXTRA_MEMBER , arg_name); +error_set (errp,QERR_QMP_EXTRA_MEMBER , arg_name);return NULL ; } }if (!has_exec_key) { -qerror_report (QERR_QMP_BAD_INPUT_OBJECT ,"execute" ); +error_set (errp,QERR_QMP_BAD_INPUT_OBJECT ,"execute" );return NULL ; }
@@ -4992,8 +4992,9 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens) goto err_out; } - input =qmp_check_input_obj (obj); + input =qmp_check_input_obj (obj, &local_err);if (!input) { +qerror_report_err (local_err);qobject_decref (obj); goto err_out; }
Signature · 1 line
1.9.3
PATCH v2 11/20 monitor: Wean monitor_protocol_emitter() off mon->error
In reply to Markus ArmbrusterMove mon->error handling to its caller handle_qmp_command().
Signed-off-by Markus Armbruster <[email protected]>
Reviewed-by Eric Blake <[email protected]>
--- monitor.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)
monitor.c +7 -6
@@ -407,13 +407,14 @@ static QDict *build_qmp_error_dict(const QError *err)return qobject_to_qdict (obj); } -static void monitor_protocol_emitter (Monitor *mon,QObject *data) +static void monitor_protocol_emitter (Monitor *mon,QObject *data, +QError *err) {QDict *qmp;trace_monitor_protocol_emitter (mon); -if (!monitor_has_error (mon)) { +if (!err) {/* success response */ qmp =qdict_new ();if (data) {
@@ -425,9 +426,7 @@ static void monitor_protocol_emitter(Monitor *mon, QObject *data) } }else {/* error response */ - qmp =build_qmp_error_dict (mon->error); -QDECREF (mon->error); - mon->error =NULL ; + qmp =build_qmp_error_dict (err); }if (mon->mc->id) {
@@ -5037,8 +5036,10 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens) } err_out: -monitor_protocol_emitter (mon, data); +monitor_protocol_emitter (mon, data, mon->error);qobject_decref (data); +QDECREF (mon->error); + mon->error =NULL ;QDECREF (input);QDECREF (args); }
Signature · 1 line
1.9.3
PATCH v2 12/20 monitor: Inline monitor_has_error() into its only caller
In reply to Markus ArmbrusterSigned-off-by Markus Armbruster <[email protected]>
Reviewed-by Eric Blake <[email protected]>
--- monitor.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)
monitor.c +1 -6
@@ -377,11 +377,6 @@ static int GCC_FMT_ATTR(2, 3) monitor_fprintf(FILE *stream,return 0 ; } -static inline int monitor_has_error (const Monitor *mon) -{ -return mon->error !=NULL ; -} -static void monitor_json_emitter (Monitor *mon,const QObject *data) {QString *json;
@@ -5029,7 +5024,7 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)if (cmd->mhandler.cmd_new (mon, args, &data)) {/* Command failed... */ -if (!monitor_has_error (mon)) { +if (!mon->error) {/* ... without setting an error, so make one up */ qerror_report (QERR_UNDEFINED_ERROR ); }
Signature · 1 line
1.9.3
PATCH v2 13/20 monitor: Limit QError use to command handlers
In reply to Markus ArmbrusterThe previous commits narrowed use of QError to handle_qmp_command() and its helpers monitor_protocol_emitter(), build_qmp_error_dict(). Narrow it further to just the command handler call: instead of converting Error to QError throughout handle_qmp_command(), convert the QError gotten from the command handler to Error, and switch the helpers from QError to Error.
Signed-off-by Markus Armbruster <[email protected]>
Reviewed-by Eric Blake <[email protected]>
--- monitor.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-)
monitor.c +14 -12
@@ -391,19 +391,19 @@ static void monitor_json_emitter(Monitor *mon, const QObject *data)QDECREF (json); } -static QDict *build_qmp_error_dict (const QError *err) +static QDict *build_qmp_error_dict (Error *err) {QObject *obj; - obj =qobject_from_jsonf ("{ 'error': { 'class': %s, 'desc': %p } }" , - ErrorClass_lookup[err->err_class], -qerror_human (err)); + obj =qobject_from_jsonf ("{ 'error': { 'class': %s, 'desc': %s } }" , + ErrorClass_lookup[error_get_class (err)], +error_get_pretty (err));return qobject_to_qdict (obj); }static void monitor_protocol_emitter (Monitor *mon,QObject *data, -QError *err) +Error *err) {QDict *qmp;
@@ -4982,13 +4982,12 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens) obj =json_parser_parse (tokens,NULL );if (!obj) {// FIXME: should be triggered in json_parser_parse() -qerror_report (QERR_JSON_PARSING ); +error_set (&local_err,QERR_JSON_PARSING ); goto err_out; } input =qmp_check_input_obj (obj, &local_err);if (!input) { -qerror_report_err (local_err);qobject_decref (obj); goto err_out; }
@@ -5000,8 +4999,8 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)trace_handle_qmp_command (mon, cmd_name); cmd =qmp_find_cmd (cmd_name);if (!cmd) { -qerror_report (ERROR_CLASS_COMMAND_NOT_FOUND , -"The command %s has not been found" , cmd_name); +error_set (&local_err,ERROR_CLASS_COMMAND_NOT_FOUND , +"The command %s has not been found" , cmd_name); goto err_out; }if (invalid_qmp_mode (mon, cmd)) {
@@ -5018,7 +5017,6 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)qmp_check_client_args (cmd, args, &local_err);if (local_err) { -qerror_report_err (local_err); goto err_out; }
@@ -5026,12 +5024,16 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)/* Command failed... */ if (!mon->error) {/* ... without setting an error, so make one up */ -qerror_report (QERR_UNDEFINED_ERROR ); +error_set (&local_err,QERR_UNDEFINED_ERROR ); } } +if (mon->error) { +error_set (&local_err, mon->error->err_class,"%s" , + mon->error->err_msg); + } err_out: -monitor_protocol_emitter (mon, data, mon->error); +monitor_protocol_emitter (mon, data, local_err);qobject_decref (data);QDECREF (mon->error); mon->error =NULL ;
Signature · 1 line
1.9.3
Re: [Qemu-devel] [PATCH v2 13/20] monitor: Limit QError use to command handlers
Quoted 82 lines · On Tue, 26 May 2015 17:20:48 +0200 Markus Armbruster <[email protected]> wrote:
The previous commits narrowed use of QError to
handle_qmp_command()and its helpersmonitor_protocol_emitter(),build_qmp_error_dict(). Narrow it further to just the command handler call: instead of converting Error to QError throughouthandle_qmp_command(), convert the QError gotten from the command handler to Error, and switch the helpers from QError to Error.Signed-off-by Markus Armbruster <[email protected]>Reviewed-by Eric Blake <[email protected]>--- monitor.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-)monitor.c +9
-11@@ -391,19 +391,19 @@ static void monitor_json_emitter(Monitor *mon, const QObject *data)QDECREF (json); } -static QDict *build_qmp_error_dict (const QError *err) +static QDict *build_qmp_error_dict (Error *err) {QObject *obj; - obj =qobject_from_jsonf ("{ 'error': { 'class': %s, 'desc': %p } }" , - ErrorClass_lookup[err->err_class], -qerror_human (err)); + obj =qobject_from_jsonf ("{ 'error': { 'class': %s, 'desc': %s } }" , + ErrorClass_lookup[error_get_class (err)], +error_get_pretty (err));return qobject_to_qdict (obj); }static void monitor_protocol_emitter (Monitor *mon,QObject *data, -QError *err) +Error *err) {QDict *qmp;@@ -4982,13 +4982,12 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens) obj =json_parser_parse (tokens,NULL );if (!obj) {// FIXME: should be triggered in json_parser_parse() -qerror_report (QERR_JSON_PARSING ); +error_set (&local_err,QERR_JSON_PARSING ); goto err_out; } input =qmp_check_input_obj (obj, &local_err);if (!input) { -qerror_report_err (local_err);qobject_decref (obj); goto err_out; }@@ -5000,8 +4999,8 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)trace_handle_qmp_command (mon, cmd_name); cmd =qmp_find_cmd (cmd_name);if (!cmd) { -qerror_report (ERROR_CLASS_COMMAND_NOT_FOUND , -"The command %s has not been found" , cmd_name); +error_set (&local_err,ERROR_CLASS_COMMAND_NOT_FOUND , +"The command %s has not been found" , cmd_name); goto err_out; }if (invalid_qmp_mode (mon, cmd)) {@@ -5018,7 +5017,6 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)qmp_check_client_args (cmd, args, &local_err);if (local_err) { -qerror_report_err (local_err); goto err_out; }@@ -5026,12 +5024,16 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)/* Command failed... */ if (!mon->error) {/* ... without setting an error, so make one up */ -qerror_report (QERR_UNDEFINED_ERROR ); +error_set (&local_err,QERR_UNDEFINED_ERROR );
monitor.c static void handle_qmp_command(JSONMessageParser *parser, QList *tokens) · line 5036
Quoted 10 lines
monitor.c +5
-1} } +if (mon->error) { +error_set (&local_err, mon->error->err_class,"%s" , + mon->error->err_msg); + } err_out: -monitor_protocol_emitter (mon, data, mon->error); +monitor_protocol_emitter (mon, data, local_err);
This breaks error reporting from invalid_qmp_mode(). The end result is that every command succeeds in capability negotiation mode and qmp_capabilities never fails (even in command mode).
There are two simple ways to fix it: just propagate mon->error to local_err when invalid_qmp_mode() fails, or change invalid_qmp_mode() to take an Error object (preferable).
Quoted 3 lines
monitor.c +0
-0qobject_decref (data);QDECREF (mon->error); mon->error =NULL ;
Re: [Qemu-devel] [PATCH v2 13/20] monitor: Limit QError use to command handlers
Quoted 92 lines · Luiz Capitulino
Quoted 89 lines · On Tue, 26 May 2015 17:20:48 +0200 Markus Armbruster <[email protected]> wrote:
The previous commits narrowed use of QError to
handle_qmp_command()and its helpersmonitor_protocol_emitter(),build_qmp_error_dict(). Narrow it further to just the command handler call: instead of converting Error to QError throughouthandle_qmp_command(), convert the QError gotten from the command handler to Error, and switch the helpers from QError to Error.Signed-off-by Markus Armbruster <[email protected]>Reviewed-by Eric Blake <[email protected]>--- monitor.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-)monitor.c +13
-11@@ -391,19 +391,19 @@ static void monitor_json_emitter(Monitor *mon, const QObject *data)QDECREF (json); } -static QDict *build_qmp_error_dict (const QError *err) +static QDict *build_qmp_error_dict (Error *err) {QObject *obj; - obj =qobject_from_jsonf ("{ 'error': { 'class': %s, 'desc': %p } }" , - ErrorClass_lookup[err->err_class], -qerror_human (err)); + obj =qobject_from_jsonf ("{ 'error': { 'class': %s, 'desc': %s } }" , + ErrorClass_lookup[error_get_class (err)], +error_get_pretty (err));return qobject_to_qdict (obj); }static void monitor_protocol_emitter (Monitor *mon,QObject *data, -QError *err) +Error *err) {QDict *qmp;@@ -4982,13 +4982,12 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens) obj =json_parser_parse (tokens,NULL );if (!obj) {// FIXME: should be triggered in json_parser_parse() -qerror_report (QERR_JSON_PARSING ); +error_set (&local_err,QERR_JSON_PARSING ); goto err_out; } input =qmp_check_input_obj (obj, &local_err);if (!input) { -qerror_report_err (local_err);qobject_decref (obj); goto err_out; }@@ -5000,8 +4999,8 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)trace_handle_qmp_command (mon, cmd_name); cmd =qmp_find_cmd (cmd_name);if (!cmd) { -qerror_report (ERROR_CLASS_COMMAND_NOT_FOUND , -"The command %s has not been found" , cmd_name); +error_set (&local_err,ERROR_CLASS_COMMAND_NOT_FOUND , +"The command %s has not been found" , cmd_name); goto err_out; }if (invalid_qmp_mode (mon, cmd)) {@@ -5018,7 +5017,6 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)qmp_check_client_args (cmd, args, &local_err);if (local_err) { -qerror_report_err (local_err); goto err_out; }@@ -5026,12 +5024,16 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)/* Command failed... */ if (!mon->error) {/* ... without setting an error, so make one up */ -qerror_report (QERR_UNDEFINED_ERROR ); +error_set (&local_err,QERR_UNDEFINED_ERROR ); } } +if (mon->error) { +error_set (&local_err, mon->error->err_class,"%s" , + mon->error->err_msg); + }
Quoted 7 lines
monitor.c static void handle_qmp_command(JSONMessageParser *parser, QList *tokens) · line 5036
Quoted 3 lines
monitor.c +1
-1err_out: -monitor_protocol_emitter (mon, data, mon->error); +monitor_protocol_emitter (mon, data, local_err);This breaks error reporting from
invalid_qmp_mode(). The end result is that every command succeeds in capability negotiation mode andqmp_capabilitiesnever fails (even in command mode).
Oops.
Quoted 3 lines
There are two simple ways to fix it: just propagate
mon->errortolocal_errwheninvalid_qmp_mode()fails, or changeinvalid_qmp_mode()to take an Error object (preferable).
I'll do the latter. Thanks!
Quoted 3 lines
Quoted 3 lines
monitor.c +0
-0qobject_decref (data);QDECREF (mon->error); mon->error =NULL ;
PATCH v2 14/20 monitor: Rename handle_user_command() to handle_hmp_command()
In reply to Markus ArmbrusterSigned-off-by Markus Armbruster <[email protected]>
Reviewed-by Eric Blake <[email protected]>
--- monitor.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
monitor.c +5 -5
@@ -574,7 +574,7 @@ static int do_qmp_capabilities(Monitor *mon, const QDict *params,return 0 ; } -static void handle_user_command (Monitor *mon,const char *cmdline); +static void handle_hmp_command (Monitor *mon,const char *cmdline);static void monitor_data_init (Monitor *mon) {
@@ -613,7 +613,7 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index, } } -handle_user_command (&hmp, command_line); +handle_hmp_command (&hmp, command_line); cur_mon = old_mon;qemu_mutex_lock (&hmp.out_lock);
@@ -4025,7 +4025,7 @@ void monitor_set_error(Monitor *mon, QError *qerror) } } -static void handle_user_command (Monitor *mon,const char *cmdline) +static void handle_hmp_command (Monitor *mon,const char *cmdline) {QDict *qdict;const mon_cmd_t *cmd;
@@ -5069,7 +5069,7 @@ static void monitor_read(void *opaque, const uint8_t *buf, int size)if (size ==0 || buf[size -1 ] !=0 )monitor_printf (cur_mon,"corrupted command\n" );else -handle_user_command (cur_mon, (char *)buf); +handle_hmp_command (cur_mon, (char *)buf); } cur_mon = old_mon;
@@ -5081,7 +5081,7 @@ static void monitor_command_cb(void *opaque, const char *cmdline,Monitor *mon = opaque;monitor_suspend (mon); -handle_user_command (mon, cmdline); +handle_hmp_command (mon, cmdline);monitor_resume (mon); }
Signature · 1 line
1.9.3
PATCH v2 15/20 monitor: Rename monitor_control_read(), monitor_control_event()
In reply to Markus Armbruster... to monitor_qmp_read(), monitor_qmp_event().
Signed-off-by Markus Armbruster <[email protected]>
Reviewed-by Eric Blake <[email protected]>
--- monitor.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-)
monitor.c +4 -10
@@ -5041,10 +5041,7 @@ err_out:QDECREF (args); } -/** -* monitor_control_read(): Read and handle QMP input -*/ -static void monitor_control_read (void *opaque,const uint8_t *buf,int size) +static void monitor_qmp_read (void *opaque,const uint8_t *buf,int size) {Monitor *old_mon = cur_mon;
@@ -5109,10 +5106,7 @@ static QObject *get_qmp_greeting(void)return qobject_from_jsonf ("{'QMP':{'version': %p,'capabilities': []}}" ,ver); } -/** -* monitor_control_event(): Print QMP gretting -*/ -static void monitor_control_event (void *opaque,int event) +static void monitor_qmp_event (void *opaque,int event) {QObject *data;Monitor *mon = opaque;
@@ -5262,8 +5256,8 @@ void monitor_init(CharDriverState *chr, int flags)if (monitor_ctrl_mode (mon)) { mon->mc =g_malloc0 (sizeof (MonitorControl));/* Control mode requires special handlers */ -qemu_chr_add_handlers (chr, monitor_can_read, monitor_control_read, - monitor_control_event, mon); +qemu_chr_add_handlers (chr, monitor_can_read, monitor_qmp_read, + monitor_qmp_event, mon);qemu_chr_fe_set_echo (chr, true);json_message_parser_init (&mon->mc->parser, handle_qmp_command);
Signature · 1 line
1.9.3
PATCH v2 16/20 monitor: Unbox Monitor member mc and rename to qmp
In reply to Markus ArmbrusterWhile there, rename its type as well, from MonitorControl to MonitorQMP.
Signed-off-by Markus Armbruster <[email protected]>
Reviewed-by Eric Blake <[email protected]>
--- monitor.c | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-)
monitor.c +15 -18
@@ -161,11 +161,11 @@ struct MonFdset {QLIST_ENTRY (MonFdset) next; }; -typedef struct MonitorControl { +typedef struct {QObject *id;JSONMessageParser parser;int command_mode; -}MonitorControl ; +}MonitorQMP ;/* * To prevent flooding clients, events can be throttled. The
@@ -195,7 +195,7 @@ struct Monitor {int mux_out;ReadLineState *rs; -MonitorControl *mc; +MonitorQMP qmp;CPUState *mon_cpu;BlockCompletionFunc *password_completion_cb;void *password_opaque;
@@ -228,7 +228,7 @@ static void monitor_command_cb(void *opaque, const char *cmdline,static inline int qmp_cmd_mode (const Monitor *mon) { -return (mon->mc ? mon->mc->command_mode :0 ); +return mon->qmp.command_mode; }/* Return true if in control mode, false otherwise */
@@ -424,9 +424,9 @@ static void monitor_protocol_emitter(Monitor *mon, QObject *data, qmp =build_qmp_error_dict (err); } -if (mon->mc->id) { -qdict_put_obj (qmp,"id" , mon->mc->id); - mon->mc->id =NULL ; +if (mon->qmp.id) { +qdict_put_obj (qmp,"id" , mon->qmp.id); + mon->qmp.id =NULL ; }monitor_json_emitter (mon,QOBJECT (qmp));
@@ -568,7 +568,7 @@ static int do_qmp_capabilities(Monitor *mon, const QDict *params, {/* Will setup QMP capabilities in the future */ if (monitor_ctrl_mode (mon)) { - mon->mc->command_mode =1 ; + mon->qmp.command_mode =1 ; }return 0 ;
@@ -4992,8 +4992,8 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens) goto err_out; } - mon->mc->id =qdict_get (input,"id" ); -qobject_incref (mon->mc->id); + mon->qmp.id =qdict_get (input,"id" ); +qobject_incref (mon->qmp.id); cmd_name =qdict_get_str (input,"execute" );trace_handle_qmp_command (mon, cmd_name);
@@ -5047,7 +5047,7 @@ static void monitor_qmp_read(void *opaque, const uint8_t *buf, int size) cur_mon = opaque; -json_message_parser_feed (&cur_mon->mc->parser, (const char *) buf, size); +json_message_parser_feed (&cur_mon->qmp.parser, (const char *) buf, size); cur_mon = old_mon; }
@@ -5113,15 +5113,15 @@ static void monitor_qmp_event(void *opaque, int event)switch (event) {case CHR_EVENT_OPENED : - mon->mc->command_mode =0 ; + mon->qmp.command_mode =0 ; data =get_qmp_greeting ();monitor_json_emitter (mon, data);qobject_decref (data); mon_refcount++;break ;case CHR_EVENT_CLOSED : -json_message_parser_destroy (&mon->mc->parser); -json_message_parser_init (&mon->mc->parser, handle_qmp_command); +json_message_parser_destroy (&mon->qmp.parser); +json_message_parser_init (&mon->qmp.parser, handle_qmp_command); mon_refcount--;monitor_fdsets_cleanup ();break ;
@@ -5254,13 +5254,10 @@ void monitor_init(CharDriverState *chr, int flags) }if (monitor_ctrl_mode (mon)) { - mon->mc =g_malloc0 (sizeof (MonitorControl)); -/* Control mode requires special handlers */ qemu_chr_add_handlers (chr, monitor_can_read, monitor_qmp_read, monitor_qmp_event, mon);qemu_chr_fe_set_echo (chr, true); - -json_message_parser_init (&mon->mc->parser, handle_qmp_command); +json_message_parser_init (&mon->qmp.parser, handle_qmp_command); }else {qemu_chr_add_handlers (chr, monitor_can_read, monitor_read, monitor_event, mon);
Signature · 1 line
1.9.3
PATCH v2 17/20 monitor: Drop do_qmp_capabilities()'s superfluous QMP check
In reply to Markus ArmbrusterSuperfluous since commit 30f5041 removed it from HMP.
Signed-off-by Markus Armbruster <[email protected]>
Reviewed-by Eric Blake <[email protected]>
--- monitor.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
monitor.c +1 -5
@@ -566,11 +566,7 @@ static void monitor_qapi_event_init(void)static int do_qmp_capabilities (Monitor *mon,const QDict *params,QObject **ret_data) { -/* Will setup QMP capabilities in the future */ -if (monitor_ctrl_mode (mon)) { - mon->qmp.command_mode =1 ; - } - + mon->qmp.command_mode =1 ;return 0 ; }
Signature · 1 line
1.9.3
PATCH v2 18/20 monitor: Turn int command_mode into bool in_command_mode
In reply to Markus ArmbrusterWhile there, inline the pointless qmp_cmd_mode() wrapper.
Signed-off-by Markus Armbruster <[email protected]>
Reviewed-by Eric Blake <[email protected]>
--- monitor.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-)
monitor.c +12 -11
@@ -164,7 +164,12 @@ struct MonFdset {typedef struct {QObject *id;JSONMessageParser parser; -int command_mode; +/* +* When a client connects, we're in capabilities negotiation mode. +* When command qmp_capabilities succeeds, we go into command +* mode. +*/ +bool in_command_mode;/* are we in command mode? */ }MonitorQMP ;/*
@@ -226,11 +231,6 @@ Monitor *default_mon;static void monitor_command_cb (void *opaque,const char *cmdline,void *readline_opaque); -static inline int qmp_cmd_mode (const Monitor *mon) -{ -return mon->qmp.command_mode; -} -/* Return true if in control mode, false otherwise */ static inline int monitor_ctrl_mode (const Monitor *mon) {
@@ -446,7 +446,7 @@ static void monitor_qapi_event_emit(QAPIEvent event, QObject *data)trace_monitor_protocol_event_emit (event, data); QLIST_FOREACH(mon, &mon_list, entry) { -if (monitor_ctrl_mode (mon) &&qmp_cmd_mode (mon)) { +if (monitor_ctrl_mode (mon) && mon->qmp.in_command_mode) {monitor_json_emitter (mon, data); } }
@@ -566,7 +566,7 @@ static void monitor_qapi_event_init(void)static int do_qmp_capabilities (Monitor *mon,const QDict *params,QObject **ret_data) { - mon->qmp.command_mode =1 ; + mon->qmp.in_command_mode = true;return 0 ; }
@@ -4701,13 +4701,14 @@ static int monitor_can_read(void *opaque)static bool invalid_qmp_mode (const Monitor *mon,const mon_cmd_t *cmd) {bool is_cap = cmd->mhandler.cmd_new == do_qmp_capabilities; -if (is_cap &&qmp_cmd_mode (mon)) { + +if (is_cap && mon->qmp.in_command_mode) {qerror_report (ERROR_CLASS_COMMAND_NOT_FOUND ,"Capabilities negotiation is already complete, command " "'%s' ignored" , cmd->name);return true; } -if (!is_cap && !qmp_cmd_mode (mon)) { +if (!is_cap && !mon->qmp.in_command_mode) {qerror_report (ERROR_CLASS_COMMAND_NOT_FOUND ,"Expecting capabilities negotiation with " "'qmp_capabilities' before command '%s'" , cmd->name);
@@ -5109,7 +5110,7 @@ static void monitor_qmp_event(void *opaque, int event)switch (event) {case CHR_EVENT_OPENED : - mon->qmp.command_mode =0 ; + mon->qmp.in_command_mode = false; data =get_qmp_greeting ();monitor_json_emitter (mon, data);qobject_decref (data);
Signature · 1 line
1.9.3
PATCH v2 19/20 monitor: Rename monitor_ctrl_mode() to monitor_is_qmp()
In reply to Markus Armbruster... and change return type to bool.
Signed-off-by Markus Armbruster <[email protected]>
Reviewed-by Eric Blake <[email protected]>
--- monitor.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
monitor.c +8 -6
@@ -231,8 +231,10 @@ Monitor *default_mon;static void monitor_command_cb (void *opaque,const char *cmdline,void *readline_opaque); -/* Return true if in control mode, false otherwise */ -static inline int monitor_ctrl_mode (const Monitor *mon) +/** +* Is @mon a QMP monitor? +*/ +static inline bool monitor_is_qmp (const Monitor *mon) {return (mon->flags &MONITOR_USE_CONTROL ); }
@@ -240,7 +242,7 @@ static inline int monitor_ctrl_mode(const Monitor *mon)/* Return non-zero iff we have a current monitor, and it is in QMP mode. */ int monitor_cur_is_qmp (void ) { -return cur_mon &&monitor_ctrl_mode (cur_mon); +return cur_mon &&monitor_is_qmp (cur_mon); }void monitor_read_command (Monitor *mon,int show_prompt)
@@ -350,7 +352,7 @@ void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)if (!mon)return ; -if (monitor_ctrl_mode (mon)) { +if (monitor_is_qmp (mon)) {return ; }
@@ -446,7 +448,7 @@ static void monitor_qapi_event_emit(QAPIEvent event, QObject *data)trace_monitor_protocol_event_emit (event, data); QLIST_FOREACH(mon, &mon_list, entry) { -if (monitor_ctrl_mode (mon) && mon->qmp.in_command_mode) { +if (monitor_is_qmp (mon) && mon->qmp.in_command_mode) {monitor_json_emitter (mon, data); } }
@@ -5250,7 +5252,7 @@ void monitor_init(CharDriverState *chr, int flags)monitor_read_command (mon,0 ); } -if (monitor_ctrl_mode (mon)) { +if (monitor_is_qmp (mon)) {qemu_chr_add_handlers (chr, monitor_can_read, monitor_qmp_read, monitor_qmp_event, mon);qemu_chr_fe_set_echo (chr, true);
Signature · 1 line
1.9.3
PATCH v2 20/20 monitor: Change return type of monitor_cur_is_qmp() to bool
In reply to Markus ArmbrusterSigned-off-by Markus Armbruster <[email protected]>
Reviewed-by Eric Blake <[email protected]>
--- include/monitor/monitor.h | 2 +- monitor.c | 6 ++++-- stubs/mon-is-qmp.c | 4 ++-- 3 files changed, 7 insertions(+), 5 deletions(-)
include/monitor/monitor.h +1 -1
@@ -16,7 +16,7 @@ extern Monitor *default_mon;#define MONITOR_USE_CONTROL 0x04#define MONITOR_USE_PRETTY 0x08 -int monitor_cur_is_qmp (void ); +bool monitor_cur_is_qmp (void );void monitor_init (CharDriverState *chr,int flags);
monitor.c +4 -2
@@ -239,8 +239,10 @@ static inline bool monitor_is_qmp(const Monitor *mon)return (mon->flags &MONITOR_USE_CONTROL ); } -/* Return non-zero iff we have a current monitor, and it is in QMP mode. */ -int monitor_cur_is_qmp (void ) +/** +* Is the current monitor, if any, a QMP monitor? +*/ +bool monitor_cur_is_qmp (void ) {return cur_mon &&monitor_is_qmp (cur_mon); }
stubs/mon-is-qmp.c +2 -2
@@ -1,7 +1,7 @@#include "qemu-common.h" #include "monitor/monitor.h" -int monitor_cur_is_qmp (void ) +bool monitor_cur_is_qmp (void ) { -return 0 ; +return false; }
Signature · 1 line
1.9.3
Quoted 1 line · On Tue, 26 May 2015 17:20:35 +0200 Markus Armbruster <[email protected]> wrote:
Command handlers still use QError. Left for another day.
Great work! I've found one bug that has to be addressed before merging. Also, for the error conversions you're doing you're going from qerror_report() to error_set() but I'd recommend going directly to error_setg() as that's our final goal. It's totally fine with me if you're saving this work for a later series, but I think it will you save work if you do it in this series. Your call.
Can you take this through your tree? Feel free to add this once you fix the bug:
Reviewed-by Luiz Capitulino <[email protected]>
Quoted 47 lines
v2:
* Trivially rebased
* PATCH 01: Drop another async remnant [Eric]
* PATCH 01+02+18: Improve commit messages
* PATCH 03+04:client_migrate_infostill hasn't been implemented VNC, de-document [Eric, Gerd]
* PATCH 16+19: Don't inlinemonitor_ctrl_mode()intomonitor_init()[Eric]
* PATCH 20: Use false instead of 0 [Eric]Markus Armbruster (20):
monitor: Drop broken, unused asynchronous command interface
monitor: Clean up after previous commit
monitor: Improve and documentclient_migrate_infoprotocol error
monitor: Convertclient_migrate_infoto QAPI
monitor: Use traditional command interface for HMPdrive_del
monitor: Use traditional command interface for HMPdevice_add
monitor: Use trad. command interface for HMPpcie_aer_inject_error
monitor: Drop unused "new" HMP command interface
monitor: Propagate errors throughqmp_check_client_args()
monitor: Propagate errors throughqmp_check_input_obj()
monitor: Weanmonitor_protocol_emitter()offmon->error
monitor: Inlinemonitor_has_error()into its only caller
monitor: Limit QError use to command handlers
monitor: Renamehandle_user_command()tohandle_hmp_command()
monitor: Renamemonitor_control_read(),monitor_control_event()
monitor: Unbox Monitor member mc and rename to qmp
monitor: Drop do_qmp_capabilities()'s superfluous QMP check
monitor: Turn intcommand_modeinto boolin_command_mode
monitor: Renamemonitor_ctrl_mode()tomonitor_is_qmp()
monitor: Change return type ofmonitor_cur_is_qmp()to boolblockdev.c | 9 +- hmp-commands.hx | 20 +-- hmp.c | 23 +++ hmp.h | 2 + hw/pci/pci-stub.c | 14 +- hw/pci/pcie_aer.c | 39 ++--- include/monitor/monitor.h | 7 +- include/sysemu/blockdev.h | 2 +- include/sysemu/sysemu.h | 4 +- monitor.c | 380 ++++++++++++++++------------------------------ qapi-schema.json | 19 +++ qmp-commands.hx | 16 +- stubs/mon-is-qmp.c | 4 +- 13 files changed, 222 insertions(+), 317 deletions(-)
Quoted 11 lines · Luiz Capitulino
Quoted 1 line · On Tue, 26 May 2015 17:20:35 +0200 Markus Armbruster <[email protected]> wrote:
Command handlers still use QError. Left for another day.
Great work! I've found one bug that has to be addressed before merging. Also, for the error conversions you're doing you're going from
qerror_report()toerror_set()but I'd recommend going directly toerror_setg()as that's our final goal. It's totally fine with me if you're saving this work for a later series, but I think it will you save work if you do it in this series. Your call.
Saving for later, not least because this series has been fully reviewed now.
Quoted 1 line
Can you take this through your tree?
Sure!
Quoted 4 lines
Feel free to add this once
you fix the bug:Reviewed-by Luiz Capitulino <[email protected]>
Thanks!