| Message ID | 1461801715-24307-9-git-send-email-eblake@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Eric Blake <eblake@redhat.com> writes: > Having to manually call out the set of expected arguments in > qmp-commands.hx, in addition to what is already in *.json, > is tedious and prone to error. The only reason we use > .args_type is for checking if there is any excess arguments > or incorrectly typed arguments during qmp_check_client_args(), > but a strict QMP Input visitor also does that checking. We also use it for completion. > Simplify things so that .args_type is only needed for the > few remaining commands that don't have a QAPI description > (namely, device_add, netdev_add, qmp_capabilities) or which > use 'gen':false (query-qmp-schema). > > There was one case where the generated marshal code has to be > beefed up: for a command that has no (or empty) 'data' in the > .json file, we were just ignoring the passed-in QDict; now we > need to validate that there are no extra arguments. > > Generated code changes like: > > |@@ -1206,7 +1206,10 @@ void qmp_marshal_cont(QDict *args, QObje > | { > | Error *err = NULL; > | > |- (void)args; > |+ if (args && qdict_size(args)) { > |+ error_setg(errp, "Command %s does not take arguments", "cont"); > |+ return; > |+ } > | > | qmp_cont(&err); > | error_propagate(errp, err); > > QMP behavior for no-arg commands changes from: > > {"execute":"query-version","arguments":{"a":1}} > {"error": {"class": "GenericError", "desc": "Invalid parameter 'a'"}} > > to: > > {"execute":"query-version","arguments":{"a":1}} > {"error": {"class": "GenericError", "desc": "Command 'query-version' does not take arguments"}} > > and for commands with at least one (possibly-optional) argument, > the output changes from: > > {"execute":"query-command-line-options","arguments":{"a":1}} > {"error": {"class": "GenericError", "desc": "Invalid parameter 'a'"}} > > to: > > {"execute":"query-command-line-options","arguments":{"a":1}} > {"error": {"class": "GenericError", "desc": "QMP input object member 'a' is unexpected"}} This error message becomes rather generic. Tolerable. Can we restore the old message without trouble? > Signed-off-by: Eric Blake <eblake@redhat.com> > > --- > v15: new patch > --- > scripts/qapi-commands.py | 8 ++- > monitor.c | 4 ++ > qmp-commands.hx | 148 +---------------------------------------------- > 3 files changed, 12 insertions(+), 148 deletions(-) > > diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py > index 04549fa..beb34c5 100644 > --- a/scripts/qapi-commands.py > +++ b/scripts/qapi-commands.py > @@ -136,8 +136,12 @@ def gen_marshal(name, arg_type, ret_type): if arg_type and arg_type.members: visit the parameter struct > else: > ret += mcgen(''' > > - (void)args; > -''') > + if (args && qdict_size(args)) { > + error_setg(errp, "Command '%%s' does not take arguments", "%(name)s"); > + return; > + } > +''', > + name=name) > > ret += gen_call(name, arg_type, ret_type) > Works for me. Naive question: is the special case "no arguments" really necessary here? Could we visit the empty struct instead? > diff --git a/monitor.c b/monitor.c > index d1c1930..bc8ff5e 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -3783,6 +3783,7 @@ out: > /* > * Client argument checking rules: > * > + * 0. If args_type is NULL, rely on the generated QAPI to do the check > * 1. Client must provide all mandatory arguments > * 2. Each argument provided by the client must be expected > * 3. Each argument provided by the client must have the type expected > @@ -3795,6 +3796,9 @@ static void qmp_check_client_args(const mon_cmd_t *cmd, QDict *client_args, > int flags; > QDict *cmd_args; > > + if (!cmd->args_type) { > + return; > + } > cmd_args = qdict_from_args_type(cmd->args_type); > > flags = 0; Let's review the other uses of args_type, to understand the impact of deleting args_type values in qmp-commands.hx: * monitor_parse_arguments() HMP only, not affected. * monitor_find_completion_by_table() MONITOR_USE_READLINE only, see monitor_init(). But then it parses input in HMP syntax with parse_cmdline(). Crap code. We're good. > diff --git a/qmp-commands.hx b/qmp-commands.hx > index de896a5..49189ce 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -62,7 +62,6 @@ EQMP > > { > .name = "quit", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_quit, > }, > > @@ -83,7 +82,6 @@ EQMP > > { > .name = "eject", > - .args_type = "force:-f,device:B", > .mhandler.cmd_new = qmp_marshal_eject, > }, > > @@ -109,7 +107,6 @@ EQMP > > { > .name = "change", > - .args_type = "device:B,target:F,arg:s?", > .mhandler.cmd_new = qmp_marshal_change, > }, > > @@ -145,7 +142,6 @@ EQMP > > { > .name = "screendump", > - .args_type = "filename:F", > .mhandler.cmd_new = qmp_marshal_screendump, > }, > > @@ -168,7 +164,6 @@ EQMP > > { > .name = "stop", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_stop, > }, > > @@ -189,7 +184,6 @@ EQMP > > { > .name = "cont", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_cont, > }, > > @@ -210,7 +204,6 @@ EQMP > > { > .name = "system_wakeup", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_system_wakeup, > }, > > @@ -231,7 +224,6 @@ EQMP > > { > .name = "system_reset", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_system_reset, > }, > > @@ -252,7 +244,6 @@ EQMP > > { > .name = "system_powerdown", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_system_powerdown, > }, > > @@ -309,7 +300,6 @@ EQMP > > { > .name = "device_del", > - .args_type = "id:s", > .mhandler.cmd_new = qmp_marshal_device_del, > }, > > @@ -337,7 +327,6 @@ EQMP > > { > .name = "send-key", > - .args_type = "keys:q,hold-time:i?", > .mhandler.cmd_new = qmp_marshal_send_key, > }, > > @@ -368,7 +357,6 @@ EQMP > > { > .name = "cpu", > - .args_type = "index:i", > .mhandler.cmd_new = qmp_marshal_cpu, > }, > > @@ -393,7 +381,6 @@ EQMP > > { > .name = "cpu-add", > - .args_type = "id:i", > .mhandler.cmd_new = qmp_marshal_cpu_add, > }, > > @@ -416,7 +403,6 @@ EQMP > > { > .name = "memsave", > - .args_type = "val:l,size:i,filename:s,cpu:i?", > .mhandler.cmd_new = qmp_marshal_memsave, > }, > > @@ -445,7 +431,6 @@ EQMP > > { > .name = "pmemsave", > - .args_type = "val:l,size:i,filename:s", > .mhandler.cmd_new = qmp_marshal_pmemsave, > }, > > @@ -473,7 +458,6 @@ EQMP > > { > .name = "inject-nmi", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_inject_nmi, > }, > > @@ -496,7 +480,6 @@ EQMP > > { > .name = "ringbuf-write", > - .args_type = "device:s,data:s,format:s?", > .mhandler.cmd_new = qmp_marshal_ringbuf_write, > }, > > @@ -525,7 +508,6 @@ EQMP > > { > .name = "ringbuf-read", > - .args_type = "device:s,size:i,format:s?", > .mhandler.cmd_new = qmp_marshal_ringbuf_read, > }, > > @@ -561,7 +543,6 @@ EQMP > > { > .name = "xen-save-devices-state", > - .args_type = "filename:F", > .mhandler.cmd_new = qmp_marshal_xen_save_devices_state, > }, > > @@ -588,7 +569,6 @@ EQMP > > { > .name = "xen-set-global-dirty-log", > - .args_type = "enable:b", > .mhandler.cmd_new = qmp_marshal_xen_set_global_dirty_log, > }, > > @@ -612,7 +592,6 @@ EQMP > > { > .name = "migrate", > - .args_type = "detach:-d,blk:-b,inc:-i,uri:s", > .mhandler.cmd_new = qmp_marshal_migrate, > }, > > @@ -645,7 +624,6 @@ EQMP > > { > .name = "migrate_cancel", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_migrate_cancel, > }, > > @@ -666,7 +644,6 @@ EQMP > > { > .name = "migrate-incoming", > - .args_type = "uri:s", > .mhandler.cmd_new = qmp_marshal_migrate_incoming, > }, > > @@ -694,7 +671,6 @@ Notes: > EQMP > { > .name = "migrate-set-cache-size", > - .args_type = "value:o", > .mhandler.cmd_new = qmp_marshal_migrate_set_cache_size, > }, > > @@ -717,7 +693,6 @@ Example: > EQMP > { > .name = "migrate-start-postcopy", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_migrate_start_postcopy, > }, > > @@ -736,7 +711,6 @@ EQMP > > { > .name = "query-migrate-cache-size", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_migrate_cache_size, > }, > > @@ -758,7 +732,6 @@ EQMP > > { > .name = "migrate_set_speed", > - .args_type = "value:o", > .mhandler.cmd_new = qmp_marshal_migrate_set_speed, > }, > > @@ -781,7 +754,6 @@ EQMP > > { > .name = "migrate_set_downtime", > - .args_type = "value:T", > .mhandler.cmd_new = qmp_marshal_migrate_set_downtime, > }, > > @@ -804,7 +776,6 @@ 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 = "set migration information for remote display", > .mhandler.cmd_new = qmp_marshal_client_migrate_info, > @@ -838,7 +809,6 @@ EQMP > > { > .name = "dump-guest-memory", > - .args_type = "paging:b,protocol:s,detach:b?,begin:i?,end:i?,format:s?", > .params = "-p protocol [-d] [begin] [length] [format]", > .help = "dump guest memory to file", > .mhandler.cmd_new = qmp_marshal_dump_guest_memory, > @@ -879,8 +849,7 @@ EQMP > > { > .name = "query-dump-guest-memory-capability", > - .args_type = "", > - .mhandler.cmd_new = qmp_marshal_query_dump_guest_memory_capability, > + .mhandler.cmd_new = qmp_marshal_query_dump_guest_memory_capability, > }, > > SQMP > @@ -899,7 +868,6 @@ EQMP > > { > .name = "query-dump", > - .args_type = "", > .params = "", > .help = "query background dump status", > .mhandler.cmd_new = qmp_marshal_query_dump, > @@ -924,7 +892,6 @@ EQMP > #if defined TARGET_S390X > { > .name = "dump-skeys", > - .args_type = "filename:F", > .mhandler.cmd_new = qmp_marshal_dump_skeys, > }, > #endif > @@ -979,7 +946,6 @@ EQMP > > { > .name = "netdev_del", > - .args_type = "id:s", > .mhandler.cmd_new = qmp_marshal_netdev_del, > }, > > @@ -1003,7 +969,6 @@ EQMP > > { > .name = "object-add", > - .args_type = "qom-type:s,id:s,props:q?", > .mhandler.cmd_new = qmp_marshal_object_add, > }, > > @@ -1029,7 +994,6 @@ EQMP > > { > .name = "object-del", > - .args_type = "id:s", > .mhandler.cmd_new = qmp_marshal_object_del, > }, > > @@ -1054,7 +1018,6 @@ EQMP > > { > .name = "block_resize", > - .args_type = "device:s?,node-name:s?,size:o", > .mhandler.cmd_new = qmp_marshal_block_resize, > }, > > @@ -1079,7 +1042,6 @@ EQMP > > { > .name = "block-stream", > - .args_type = "device:B,base:s?,speed:o?,backing-file:s?,on-error:s?", > .mhandler.cmd_new = qmp_marshal_block_stream, > }, > > @@ -1122,7 +1084,6 @@ EQMP > > { > .name = "block-commit", > - .args_type = "device:B,base:s?,top:s?,backing-file:s?,speed:o?", > .mhandler.cmd_new = qmp_marshal_block_commit, > }, > > @@ -1185,8 +1146,6 @@ EQMP > > { > .name = "drive-backup", > - .args_type = "sync:s,device:B,target:s,speed:i?,mode:s?,format:s?," > - "bitmap:s?,on-source-error:s?,on-target-error:s?", > .mhandler.cmd_new = qmp_marshal_drive_backup, > }, > > @@ -1239,8 +1198,6 @@ EQMP > > { > .name = "blockdev-backup", > - .args_type = "sync:s,device:B,target:B,speed:i?," > - "on-source-error:s?,on-target-error:s?", > .mhandler.cmd_new = qmp_marshal_blockdev_backup, > }, > > @@ -1280,33 +1237,27 @@ EQMP > > { > .name = "block-job-set-speed", > - .args_type = "device:B,speed:o", > .mhandler.cmd_new = qmp_marshal_block_job_set_speed, > }, > > { > .name = "block-job-cancel", > - .args_type = "device:B,force:b?", > .mhandler.cmd_new = qmp_marshal_block_job_cancel, > }, > { > .name = "block-job-pause", > - .args_type = "device:B", > .mhandler.cmd_new = qmp_marshal_block_job_pause, > }, > { > .name = "block-job-resume", > - .args_type = "device:B", > .mhandler.cmd_new = qmp_marshal_block_job_resume, > }, > { > .name = "block-job-complete", > - .args_type = "device:B", > .mhandler.cmd_new = qmp_marshal_block_job_complete, > }, > { > .name = "transaction", > - .args_type = "actions:q,properties:q?", > .mhandler.cmd_new = qmp_marshal_transaction, > }, > > @@ -1400,7 +1351,6 @@ EQMP > > { > .name = "block-dirty-bitmap-add", > - .args_type = "node:B,name:s,granularity:i?", > .mhandler.cmd_new = qmp_marshal_block_dirty_bitmap_add, > }, > > @@ -1428,7 +1378,6 @@ EQMP > > { > .name = "block-dirty-bitmap-remove", > - .args_type = "node:B,name:s", > .mhandler.cmd_new = qmp_marshal_block_dirty_bitmap_remove, > }, > > @@ -1456,7 +1405,6 @@ EQMP > > { > .name = "block-dirty-bitmap-clear", > - .args_type = "node:B,name:s", > .mhandler.cmd_new = qmp_marshal_block_dirty_bitmap_clear, > }, > > @@ -1485,7 +1433,6 @@ EQMP > > { > .name = "blockdev-snapshot-sync", > - .args_type = "device:s?,node-name:s?,snapshot-file:s,snapshot-node-name:s?,format:s?,mode:s?", > .mhandler.cmd_new = qmp_marshal_blockdev_snapshot_sync, > }, > > @@ -1521,7 +1468,6 @@ EQMP > > { > .name = "blockdev-snapshot", > - .args_type = "node:s,overlay:s", > .mhandler.cmd_new = qmp_marshal_blockdev_snapshot, > }, > > @@ -1559,7 +1505,6 @@ EQMP > > { > .name = "blockdev-snapshot-internal-sync", > - .args_type = "device:B,name:s", > .mhandler.cmd_new = qmp_marshal_blockdev_snapshot_internal_sync, > }, > > @@ -1588,7 +1533,6 @@ EQMP > > { > .name = "blockdev-snapshot-delete-internal-sync", > - .args_type = "device:B,id:s?,name:s?", > .mhandler.cmd_new = > qmp_marshal_blockdev_snapshot_delete_internal_sync, > }, > @@ -1629,11 +1573,6 @@ EQMP > > { > .name = "drive-mirror", > - .args_type = "sync:s,device:B,target:s,speed:i?,mode:s?,format:s?," > - "node-name:s?,replaces:s?," > - "on-source-error:s?,on-target-error:s?," > - "unmap:b?," > - "granularity:i?,buf-size:i?", > .mhandler.cmd_new = qmp_marshal_drive_mirror, > }, > > @@ -1693,9 +1632,6 @@ EQMP > > { > .name = "blockdev-mirror", > - .args_type = "sync:s,device:B,target:B,replaces:s?,speed:i?," > - "on-source-error:s?,on-target-error:s?," > - "granularity:i?,buf-size:i?", > .mhandler.cmd_new = qmp_marshal_blockdev_mirror, > }, > > @@ -1741,7 +1677,6 @@ Example: > EQMP > { > .name = "change-backing-file", > - .args_type = "device:s,image-node-name:s,backing-file:s", > .mhandler.cmd_new = qmp_marshal_change_backing_file, > }, > > @@ -1780,7 +1715,6 @@ EQMP > > { > .name = "balloon", > - .args_type = "value:M", > .mhandler.cmd_new = qmp_marshal_balloon, > }, > > @@ -1803,7 +1737,6 @@ EQMP > > { > .name = "set_link", > - .args_type = "name:s,up:b", > .mhandler.cmd_new = qmp_marshal_set_link, > }, > > @@ -1827,7 +1760,6 @@ EQMP > > { > .name = "getfd", > - .args_type = "fdname:s", > .params = "getfd name", > .help = "receive a file descriptor via SCM rights and assign it a name", > .mhandler.cmd_new = qmp_marshal_getfd, > @@ -1860,7 +1792,6 @@ EQMP > > { > .name = "closefd", > - .args_type = "fdname:s", > .params = "closefd name", > .help = "close a file descriptor previously passed via SCM rights", > .mhandler.cmd_new = qmp_marshal_closefd, > @@ -1885,7 +1816,6 @@ EQMP > > { > .name = "add-fd", > - .args_type = "fdset-id:i?,opaque:s?", > .params = "add-fd fdset-id opaque", > .help = "Add a file descriptor, that was passed via SCM rights, to an fd set", > .mhandler.cmd_new = qmp_marshal_add_fd, > @@ -1924,7 +1854,6 @@ EQMP > > { > .name = "remove-fd", > - .args_type = "fdset-id:i,fd:i?", > .params = "remove-fd fdset-id fd", > .help = "Remove a file descriptor from an fd set", > .mhandler.cmd_new = qmp_marshal_remove_fd, > @@ -1957,7 +1886,6 @@ EQMP > > { > .name = "query-fdsets", > - .args_type = "", > .help = "Return information describing all fd sets", > .mhandler.cmd_new = qmp_marshal_query_fdsets, > }, > @@ -2007,7 +1935,6 @@ EQMP > > { > .name = "block_passwd", > - .args_type = "device:s?,node-name:s?,password:s", > .mhandler.cmd_new = qmp_marshal_block_passwd, > }, > > @@ -2033,7 +1960,6 @@ EQMP > > { > .name = "block_set_io_throttle", > - .args_type = "device:B,bps:l,bps_rd:l,bps_wr:l,iops:l,iops_rd:l,iops_wr:l,bps_max:l?,bps_rd_max:l?,bps_wr_max:l?,iops_max:l?,iops_rd_max:l?,iops_wr_max:l?,bps_max_length:l?,bps_rd_max_length:l?,bps_wr_max_length:l?,iops_max_length:l?,iops_rd_max_length:l?,iops_wr_max_length:l?,iops_size:l?,group:s?", > .mhandler.cmd_new = qmp_marshal_block_set_io_throttle, > }, > > @@ -2090,7 +2016,6 @@ EQMP > > { > .name = "set_password", > - .args_type = "protocol:s,password:s,connected:s?", > .mhandler.cmd_new = qmp_marshal_set_password, > }, > > @@ -2116,7 +2041,6 @@ EQMP > > { > .name = "expire_password", > - .args_type = "protocol:s,time:s", > .mhandler.cmd_new = qmp_marshal_expire_password, > }, > > @@ -2141,7 +2065,6 @@ EQMP > > { > .name = "add_client", > - .args_type = "protocol:s,fdname:s,skipauth:b?,tls:b?", > .mhandler.cmd_new = qmp_marshal_add_client, > }, > > @@ -2192,7 +2115,6 @@ EQMP > > { > .name = "human-monitor-command", > - .args_type = "command-line:s,cpu-index:i?", > .mhandler.cmd_new = qmp_marshal_human_monitor_command, > }, > > @@ -2271,7 +2193,6 @@ EQMP > > { > .name = "query-version", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_version, > }, > > @@ -2308,7 +2229,6 @@ EQMP > > { > .name = "query-commands", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_commands, > }, > > @@ -2345,7 +2265,6 @@ EQMP > > { > .name = "query-events", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_events, > }, > > @@ -2362,7 +2281,7 @@ EQMP > > { > .name = "query-qmp-schema", > - .args_type = "", > + .args_type = "", > .mhandler.cmd_new = qmp_query_qmp_schema, > }, Spurious hunk. > > @@ -2407,7 +2326,6 @@ EQMP > > { > .name = "query-chardev", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_chardev, > }, > > @@ -2448,7 +2366,6 @@ EQMP > > { > .name = "query-chardev-backends", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_chardev_backends, > }, > > @@ -2632,7 +2549,6 @@ EQMP > > { > .name = "query-block", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_block, > }, > > @@ -2829,7 +2745,6 @@ EQMP > > { > .name = "query-blockstats", > - .args_type = "query-nodes:b?", > .mhandler.cmd_new = qmp_marshal_query_blockstats, > }, > > @@ -2884,7 +2799,6 @@ EQMP > > { > .name = "query-cpus", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_cpus, > }, > > @@ -2923,7 +2837,6 @@ EQMP > > { > .name = "query-iothreads", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_iothreads, > }, > > @@ -3140,7 +3053,6 @@ EQMP > > { > .name = "query-pci", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_pci, > }, > > @@ -3164,7 +3076,6 @@ EQMP > > { > .name = "query-kvm", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_kvm, > }, > > @@ -3204,7 +3115,6 @@ EQMP > > { > .name = "query-status", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_status, > }, > > @@ -3248,7 +3158,6 @@ EQMP > > { > .name = "query-mice", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_mice, > }, > > @@ -3311,12 +3220,10 @@ EQMP > > { > .name = "query-vnc", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_vnc, > }, > { > .name = "query-vnc-servers", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_vnc_servers, > }, > > @@ -3393,7 +3300,6 @@ EQMP > #if defined(CONFIG_SPICE) > { > .name = "query-spice", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_spice, > }, > #endif > @@ -3417,7 +3323,6 @@ EQMP > > { > .name = "query-name", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_name, > }, > > @@ -3440,7 +3345,6 @@ EQMP > > { > .name = "query-uuid", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_uuid, > }, > > @@ -3489,7 +3393,6 @@ EQMP > > { > .name = "query-command-line-options", > - .args_type = "option:s?", > .mhandler.cmd_new = qmp_marshal_query_command_line_options, > }, > > @@ -3667,7 +3570,6 @@ EQMP > > { > .name = "query-migrate", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_migrate, > }, > > @@ -3696,7 +3598,6 @@ EQMP > > { > .name = "migrate-set-capabilities", > - .args_type = "capabilities:q", > .params = "capability:s,state:b", > .mhandler.cmd_new = qmp_marshal_migrate_set_capabilities, > }, > @@ -3734,7 +3635,6 @@ EQMP > > { > .name = "query-migrate-capabilities", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_migrate_capabilities, > }, > > @@ -3763,8 +3663,6 @@ EQMP > > { > .name = "migrate-set-parameters", > - .args_type = > - "compress-level:i?,compress-threads:i?,decompress-threads:i?,x-cpu-throttle-initial:i?,x-cpu-throttle-increment:i?", > .mhandler.cmd_new = qmp_marshal_migrate_set_parameters, > }, > SQMP > @@ -3801,7 +3699,6 @@ EQMP > > { > .name = "query-migrate-parameters", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_migrate_parameters, > }, > > @@ -3829,88 +3726,73 @@ EQMP > > { > .name = "query-balloon", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_balloon, > }, > > { > .name = "query-block-jobs", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_block_jobs, > }, > > { > .name = "qom-list", > - .args_type = "path:s", > .mhandler.cmd_new = qmp_marshal_qom_list, > }, > > { > .name = "qom-set", > - .args_type = "path:s,property:s,value:q", > .mhandler.cmd_new = qmp_marshal_qom_set, > }, > > { > .name = "qom-get", > - .args_type = "path:s,property:s", > .mhandler.cmd_new = qmp_marshal_qom_get, > }, > > { > .name = "nbd-server-start", > - .args_type = "addr:q,tls-creds:s?", > .mhandler.cmd_new = qmp_marshal_nbd_server_start, > }, > { > .name = "nbd-server-add", > - .args_type = "device:B,writable:b?", > .mhandler.cmd_new = qmp_marshal_nbd_server_add, > }, > { > .name = "nbd-server-stop", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_nbd_server_stop, > }, > > { > .name = "change-vnc-password", > - .args_type = "password:s", > .mhandler.cmd_new = qmp_marshal_change_vnc_password, > }, > { > .name = "qom-list-types", > - .args_type = "implements:s?,abstract:b?", > .mhandler.cmd_new = qmp_marshal_qom_list_types, > }, > > { > .name = "device-list-properties", > - .args_type = "typename:s", > .mhandler.cmd_new = qmp_marshal_device_list_properties, > }, > > { > .name = "query-machines", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_machines, > }, > > { > .name = "query-cpu-definitions", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_cpu_definitions, > }, > > { > .name = "query-target", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_target, > }, > > { > .name = "query-tpm", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_tpm, > }, > > @@ -3944,7 +3826,6 @@ EQMP > > { > .name = "query-tpm-models", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_tpm_models, > }, > > @@ -3965,7 +3846,6 @@ EQMP > > { > .name = "query-tpm-types", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_tpm_types, > }, > > @@ -3986,7 +3866,6 @@ EQMP > > { > .name = "chardev-add", > - .args_type = "id:s,backend:q", > .mhandler.cmd_new = qmp_marshal_chardev_add, > }, > > @@ -4023,7 +3902,6 @@ EQMP > > { > .name = "chardev-remove", > - .args_type = "id:s", > .mhandler.cmd_new = qmp_marshal_chardev_remove, > }, > > @@ -4046,7 +3924,6 @@ Example: > EQMP > { > .name = "query-rx-filter", > - .args_type = "name:s?", > .mhandler.cmd_new = qmp_marshal_query_rx_filter, > }, > > @@ -4112,7 +3989,6 @@ EQMP > > { > .name = "blockdev-add", > - .args_type = "options:q", > .mhandler.cmd_new = qmp_marshal_blockdev_add, > }, > > @@ -4171,7 +4047,6 @@ EQMP > > { > .name = "x-blockdev-del", > - .args_type = "id:s?,node-name:s?", > .mhandler.cmd_new = qmp_marshal_x_blockdev_del, > }, > > @@ -4228,7 +4103,6 @@ EQMP > > { > .name = "blockdev-open-tray", > - .args_type = "device:s,force:b?", > .mhandler.cmd_new = qmp_marshal_blockdev_open_tray, > }, > > @@ -4276,7 +4150,6 @@ EQMP > > { > .name = "blockdev-close-tray", > - .args_type = "device:s", > .mhandler.cmd_new = qmp_marshal_blockdev_close_tray, > }, > > @@ -4311,7 +4184,6 @@ EQMP > > { > .name = "x-blockdev-remove-medium", > - .args_type = "device:s", > .mhandler.cmd_new = qmp_marshal_x_blockdev_remove_medium, > }, > > @@ -4359,7 +4231,6 @@ EQMP > > { > .name = "x-blockdev-insert-medium", > - .args_type = "device:s,node-name:s", > .mhandler.cmd_new = qmp_marshal_x_blockdev_insert_medium, > }, > > @@ -4399,7 +4270,6 @@ EQMP > > { > .name = "query-named-block-nodes", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_named_block_nodes, > }, > > @@ -4461,7 +4331,6 @@ EQMP > > { > .name = "blockdev-change-medium", > - .args_type = "device:B,filename:F,format:s?,read-only-mode:s?", > .mhandler.cmd_new = qmp_marshal_blockdev_change_medium, > }, > > @@ -4514,7 +4383,6 @@ EQMP > > { > .name = "query-memdev", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_memdev, > }, > > @@ -4552,7 +4420,6 @@ EQMP > > { > .name = "query-memory-devices", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_memory_devices, > }, > > @@ -4579,7 +4446,6 @@ EQMP > > { > .name = "query-acpi-ospm-status", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_acpi_ospm_status, > }, > > @@ -4602,7 +4468,6 @@ EQMP > #if defined TARGET_I386 > { > .name = "rtc-reset-reinjection", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_rtc_reset_reinjection, > }, > #endif > @@ -4623,7 +4488,6 @@ EQMP > > { > .name = "trace-event-get-state", > - .args_type = "name:s", > .mhandler.cmd_new = qmp_marshal_trace_event_get_state, > }, > > @@ -4641,7 +4505,6 @@ EQMP > > { > .name = "trace-event-set-state", > - .args_type = "name:s,enable:b,ignore-unavailable:b?", > .mhandler.cmd_new = qmp_marshal_trace_event_set_state, > }, > > @@ -4659,7 +4522,6 @@ EQMP > > { > .name = "input-send-event", > - .args_type = "console:i?,events:q", > .mhandler.cmd_new = qmp_marshal_input_send_event, > }, > > @@ -4725,7 +4587,6 @@ EQMP > > { > .name = "block-set-write-threshold", > - .args_type = "node-name:s,write-threshold:l", > .mhandler.cmd_new = qmp_marshal_block_set_write_threshold, > }, > > @@ -4753,7 +4614,6 @@ EQMP > > { > .name = "query-rocker", > - .args_type = "name:s", > .mhandler.cmd_new = qmp_marshal_query_rocker, > }, > > @@ -4774,7 +4634,6 @@ EQMP > > { > .name = "query-rocker-ports", > - .args_type = "name:s", > .mhandler.cmd_new = qmp_marshal_query_rocker_ports, > }, > > @@ -4799,7 +4658,6 @@ EQMP > > { > .name = "query-rocker-of-dpa-flows", > - .args_type = "name:s,tbl-id:i?", > .mhandler.cmd_new = qmp_marshal_query_rocker_of_dpa_flows, > }, > > @@ -4828,7 +4686,6 @@ EQMP > > { > .name = "query-rocker-of-dpa-groups", > - .args_type = "name:s,type:i?", > .mhandler.cmd_new = qmp_marshal_query_rocker_of_dpa_groups, > }, > > @@ -4859,7 +4716,6 @@ EQMP > #if defined TARGET_ARM > { > .name = "query-gic-capabilities", > - .args_type = "", > .mhandler.cmd_new = qmp_marshal_query_gic_capabilities, > }, > #endif Entries defining anything beyond .name and .mhandler.cmd_new: * device_add, qmp_capabilities Not QAPIfied, need everything. * netdev_add, query-qmp-schema Need .args_type because of 'gen': false. * client_migrate_info, dump-guest-memory, query-dump, getfd, closefd, add-fd, remove-fd, query-fdsets, migrate-set-capabilities Define .params and/or .help. Does this make any sense? A comment explaining which members need to be set would be nice. Have you checked Marc-André's work for overlap? Cc'ing him.
On Thu, Apr 28, 2016 at 4:09 PM, Markus Armbruster <armbru@redhat.com> wrote: > > * device_add, qmp_capabilities > > Not QAPIfied, need everything. Some related commits from https://github.com/elmarco/qemu/commits/qapi https://github.com/elmarco/qemu/commit/e6828a0a926e20d3ee91ea956152b610ca30d2b8 https://github.com/elmarco/qemu/commit/ba871db1c8d0edd0cbe7be716952f72c9df519a2 > > * netdev_add, query-qmp-schema > > Need .args_type because of 'gen': false. > > * client_migrate_info, dump-guest-memory, query-dump, getfd, closefd, > add-fd, remove-fd, query-fdsets, migrate-set-capabilities > > Define .params and/or .help. Does this make any sense? > > A comment explaining which members need to be set would be nice. > > Have you checked Marc-André's work for overlap? Cc'ing him. yeah, it's conflicting with my work. Unfortunately, since I have been asked to wait since a long time, I am no longer on top of things, nor what I need to wait for. I have the feeling we really want qmp_dispatch() in monitor.c: https://github.com/elmarco/qemu/commit/af4158993088ee2bbf9087c440990880d7943eb1. Eric, I have been waiting for your series to settle for getting back to qapi, but it keeps growing ;) I hope you are familiar with the work I did in the branch so we don't duplicate efforts.
On 04/28/2016 08:09 AM, Markus Armbruster wrote: > Eric Blake <eblake@redhat.com> writes: > >> Having to manually call out the set of expected arguments in >> qmp-commands.hx, in addition to what is already in *.json, >> is tedious and prone to error. The only reason we use >> .args_type is for checking if there is any excess arguments >> or incorrectly typed arguments during qmp_check_client_args(), >> but a strict QMP Input visitor also does that checking. > > We also use it for completion. Does scripts/qmp/qmp-shell do completion? It's a script, is it parsing the .hx file? I know we do completion for HMP, but this is QMP that I'm tweaking, and other than qmp-shell, I don't know of any qemu code that wants to do QMP completion. We have query-qmp-schema that could be used to provide completion, if needed. >> and for commands with at least one (possibly-optional) argument, >> the output changes from: >> >> {"execute":"query-command-line-options","arguments":{"a":1}} >> {"error": {"class": "GenericError", "desc": "Invalid parameter 'a'"}} >> QERR_INVALID_PARAMETER >> to: >> >> {"execute":"query-command-line-options","arguments":{"a":1}} >> {"error": {"class": "GenericError", "desc": "QMP input object member 'a' is unexpected"}} > > This error message becomes rather generic. Tolerable. Can we restore > the old message without trouble? QERR_QMP_EXTRA_MEMBER Yes, it's quite easy to switch between the two error message strings by tweaking the choice of message in qmp_input_check_struct() (at the end of the series; it's in qmp_input_pop() at this point of the series). >> >> - (void)args; >> -''') >> + if (args && qdict_size(args)) { >> + error_setg(errp, "Command '%%s' does not take arguments", "%(name)s"); >> + return; >> + } >> +''', >> + name=name) >> >> ret += gen_call(name, arg_type, ret_type) >> > > Works for me. > > Naive question: is the special case "no arguments" really necessary > here? Could we visit the empty struct instead? If we had an empty struct visitor around. But right now the generator special-cases ':empty' so that it has no generated functions. >> @@ -2362,7 +2281,7 @@ EQMP >> >> { >> .name = "query-qmp-schema", >> - .args_type = "", >> + .args_type = "", >> .mhandler.cmd_new = qmp_query_qmp_schema, >> }, > > Spurious hunk. Whoops. I first deleted it, then realized this was one of the few places that doesn't use qmp_marshal_ so I had to restore it, but restored it with the wrong whitespace. > > Entries defining anything beyond .name and .mhandler.cmd_new: > > * device_add, qmp_capabilities > > Not QAPIfied, need everything. > > * netdev_add, query-qmp-schema > > Need .args_type because of 'gen': false. > > * client_migrate_info, dump-guest-memory, query-dump, getfd, closefd, > add-fd, remove-fd, query-fdsets, migrate-set-capabilities > > Define .params and/or .help. Does this make any sense? > > A comment explaining which members need to be set would be nice. > > Have you checked Marc-André's work for overlap? Cc'ing him. Marc-André QAPIfies device_add and qmp_capabilities, then completely eliminates qmp-commands.hx. This probably duplicates some of the work in his queue, but should be fine in the short term. As for whether any of the other fields are needed or useful, I didn't check (but suspect that no, they are not needed, again because this is QMP not HMP and that only HMP 'info cmd' cares).
Marc-André Lureau <marcandre.lureau@gmail.com> writes: > On Thu, Apr 28, 2016 at 4:09 PM, Markus Armbruster <armbru@redhat.com> wrote: >> >> * device_add, qmp_capabilities >> >> Not QAPIfied, need everything. > > Some related commits from https://github.com/elmarco/qemu/commits/qapi > > > https://github.com/elmarco/qemu/commit/e6828a0a926e20d3ee91ea956152b610ca30d2b8 > > https://github.com/elmarco/qemu/commit/ba871db1c8d0edd0cbe7be716952f72c9df519a2 > >> >> * netdev_add, query-qmp-schema >> >> Need .args_type because of 'gen': false. >> >> * client_migrate_info, dump-guest-memory, query-dump, getfd, closefd, >> add-fd, remove-fd, query-fdsets, migrate-set-capabilities >> >> Define .params and/or .help. Does this make any sense? >> >> A comment explaining which members need to be set would be nice. >> >> Have you checked Marc-André's work for overlap? Cc'ing him. > > yeah, it's conflicting with my work. Unfortunately, since I have been > asked to wait since a long time, I am no longer on top of things, nor > what I need to wait for. I have the feeling we really want > qmp_dispatch() in monitor.c: > https://github.com/elmarco/qemu/commit/af4158993088ee2bbf9087c440990880d7943eb1. > > Eric, I have been waiting for your series to settle for getting back > to qapi, but it keeps growing ;) I hope you are familiar with the work > I did in the branch so we don't duplicate efforts. I'd very much prefer to give precedence to Marc-André's solution. I think this patch is in this series just because the previous patch notes a redundancy, and promises its cleanup. What about this: * We shelve this patch for now, and s/a later patch/a future patch/ in the previous patch's commit message. * We continue to flush Eric's QAPI queue. If we come to a point where we really want this patch, we insert it, and apologize to Marc-André. But hopefully we come to a point where Marc-André's work fits first! We'll refer to this patch when reviewing his solution, to make sure we're not losing anything. Okay?
On 04/28/2016 12:00 PM, Markus Armbruster wrote: >>> Have you checked Marc-André's work for overlap? Cc'ing him. >> >> yeah, it's conflicting with my work. Unfortunately, since I have been >> asked to wait since a long time, I am no longer on top of things, nor >> what I need to wait for. I have the feeling we really want >> qmp_dispatch() in monitor.c: >> https://github.com/elmarco/qemu/commit/af4158993088ee2bbf9087c440990880d7943eb1. >> >> Eric, I have been waiting for your series to settle for getting back >> to qapi, but it keeps growing ;) I hope you are familiar with the work >> I did in the branch so we don't duplicate efforts. > > I'd very much prefer to give precedence to Marc-André's solution. > > I think this patch is in this series just because the previous patch > notes a redundancy, and promises its cleanup. What about this: > > * We shelve this patch for now, and s/a later patch/a future patch/ in > the previous patch's commit message. > > * We continue to flush Eric's QAPI queue. If we come to a point where > we really want this patch, we insert it, and apologize to Marc-André. > But hopefully we come to a point where Marc-André's work fits first! > We'll refer to this patch when reviewing his solution, to make sure > we're not losing anything. > > Okay? Sounds good to me :)
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 04549fa..beb34c5 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -136,8 +136,12 @@ def gen_marshal(name, arg_type, ret_type): else: ret += mcgen(''' - (void)args; -''') + if (args && qdict_size(args)) { + error_setg(errp, "Command '%%s' does not take arguments", "%(name)s"); + return; + } +''', + name=name) ret += gen_call(name, arg_type, ret_type) diff --git a/monitor.c b/monitor.c index d1c1930..bc8ff5e 100644 --- a/monitor.c +++ b/monitor.c @@ -3783,6 +3783,7 @@ out: /* * Client argument checking rules: * + * 0. If args_type is NULL, rely on the generated QAPI to do the check * 1. Client must provide all mandatory arguments * 2. Each argument provided by the client must be expected * 3. Each argument provided by the client must have the type expected @@ -3795,6 +3796,9 @@ static void qmp_check_client_args(const mon_cmd_t *cmd, QDict *client_args, int flags; QDict *cmd_args; + if (!cmd->args_type) { + return; + } cmd_args = qdict_from_args_type(cmd->args_type); flags = 0; diff --git a/qmp-commands.hx b/qmp-commands.hx index de896a5..49189ce 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -62,7 +62,6 @@ EQMP { .name = "quit", - .args_type = "", .mhandler.cmd_new = qmp_marshal_quit, }, @@ -83,7 +82,6 @@ EQMP { .name = "eject", - .args_type = "force:-f,device:B", .mhandler.cmd_new = qmp_marshal_eject, }, @@ -109,7 +107,6 @@ EQMP { .name = "change", - .args_type = "device:B,target:F,arg:s?", .mhandler.cmd_new = qmp_marshal_change, }, @@ -145,7 +142,6 @@ EQMP { .name = "screendump", - .args_type = "filename:F", .mhandler.cmd_new = qmp_marshal_screendump, }, @@ -168,7 +164,6 @@ EQMP { .name = "stop", - .args_type = "", .mhandler.cmd_new = qmp_marshal_stop, }, @@ -189,7 +184,6 @@ EQMP { .name = "cont", - .args_type = "", .mhandler.cmd_new = qmp_marshal_cont, }, @@ -210,7 +204,6 @@ EQMP { .name = "system_wakeup", - .args_type = "", .mhandler.cmd_new = qmp_marshal_system_wakeup, }, @@ -231,7 +224,6 @@ EQMP { .name = "system_reset", - .args_type = "", .mhandler.cmd_new = qmp_marshal_system_reset, }, @@ -252,7 +244,6 @@ EQMP { .name = "system_powerdown", - .args_type = "", .mhandler.cmd_new = qmp_marshal_system_powerdown, }, @@ -309,7 +300,6 @@ EQMP { .name = "device_del", - .args_type = "id:s", .mhandler.cmd_new = qmp_marshal_device_del, }, @@ -337,7 +327,6 @@ EQMP { .name = "send-key", - .args_type = "keys:q,hold-time:i?", .mhandler.cmd_new = qmp_marshal_send_key, }, @@ -368,7 +357,6 @@ EQMP { .name = "cpu", - .args_type = "index:i", .mhandler.cmd_new = qmp_marshal_cpu, }, @@ -393,7 +381,6 @@ EQMP { .name = "cpu-add", - .args_type = "id:i", .mhandler.cmd_new = qmp_marshal_cpu_add, }, @@ -416,7 +403,6 @@ EQMP { .name = "memsave", - .args_type = "val:l,size:i,filename:s,cpu:i?", .mhandler.cmd_new = qmp_marshal_memsave, }, @@ -445,7 +431,6 @@ EQMP { .name = "pmemsave", - .args_type = "val:l,size:i,filename:s", .mhandler.cmd_new = qmp_marshal_pmemsave, }, @@ -473,7 +458,6 @@ EQMP { .name = "inject-nmi", - .args_type = "", .mhandler.cmd_new = qmp_marshal_inject_nmi, }, @@ -496,7 +480,6 @@ EQMP { .name = "ringbuf-write", - .args_type = "device:s,data:s,format:s?", .mhandler.cmd_new = qmp_marshal_ringbuf_write, }, @@ -525,7 +508,6 @@ EQMP { .name = "ringbuf-read", - .args_type = "device:s,size:i,format:s?", .mhandler.cmd_new = qmp_marshal_ringbuf_read, }, @@ -561,7 +543,6 @@ EQMP { .name = "xen-save-devices-state", - .args_type = "filename:F", .mhandler.cmd_new = qmp_marshal_xen_save_devices_state, }, @@ -588,7 +569,6 @@ EQMP { .name = "xen-set-global-dirty-log", - .args_type = "enable:b", .mhandler.cmd_new = qmp_marshal_xen_set_global_dirty_log, }, @@ -612,7 +592,6 @@ EQMP { .name = "migrate", - .args_type = "detach:-d,blk:-b,inc:-i,uri:s", .mhandler.cmd_new = qmp_marshal_migrate, }, @@ -645,7 +624,6 @@ EQMP { .name = "migrate_cancel", - .args_type = "", .mhandler.cmd_new = qmp_marshal_migrate_cancel, }, @@ -666,7 +644,6 @@ EQMP { .name = "migrate-incoming", - .args_type = "uri:s", .mhandler.cmd_new = qmp_marshal_migrate_incoming, }, @@ -694,7 +671,6 @@ Notes: EQMP { .name = "migrate-set-cache-size", - .args_type = "value:o", .mhandler.cmd_new = qmp_marshal_migrate_set_cache_size, }, @@ -717,7 +693,6 @@ Example: EQMP { .name = "migrate-start-postcopy", - .args_type = "", .mhandler.cmd_new = qmp_marshal_migrate_start_postcopy, }, @@ -736,7 +711,6 @@ EQMP { .name = "query-migrate-cache-size", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_migrate_cache_size, }, @@ -758,7 +732,6 @@ EQMP { .name = "migrate_set_speed", - .args_type = "value:o", .mhandler.cmd_new = qmp_marshal_migrate_set_speed, }, @@ -781,7 +754,6 @@ EQMP { .name = "migrate_set_downtime", - .args_type = "value:T", .mhandler.cmd_new = qmp_marshal_migrate_set_downtime, }, @@ -804,7 +776,6 @@ 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 = "set migration information for remote display", .mhandler.cmd_new = qmp_marshal_client_migrate_info, @@ -838,7 +809,6 @@ EQMP { .name = "dump-guest-memory", - .args_type = "paging:b,protocol:s,detach:b?,begin:i?,end:i?,format:s?", .params = "-p protocol [-d] [begin] [length] [format]", .help = "dump guest memory to file", .mhandler.cmd_new = qmp_marshal_dump_guest_memory, @@ -879,8 +849,7 @@ EQMP { .name = "query-dump-guest-memory-capability", - .args_type = "", - .mhandler.cmd_new = qmp_marshal_query_dump_guest_memory_capability, + .mhandler.cmd_new = qmp_marshal_query_dump_guest_memory_capability, }, SQMP @@ -899,7 +868,6 @@ EQMP { .name = "query-dump", - .args_type = "", .params = "", .help = "query background dump status", .mhandler.cmd_new = qmp_marshal_query_dump, @@ -924,7 +892,6 @@ EQMP #if defined TARGET_S390X { .name = "dump-skeys", - .args_type = "filename:F", .mhandler.cmd_new = qmp_marshal_dump_skeys, }, #endif @@ -979,7 +946,6 @@ EQMP { .name = "netdev_del", - .args_type = "id:s", .mhandler.cmd_new = qmp_marshal_netdev_del, }, @@ -1003,7 +969,6 @@ EQMP { .name = "object-add", - .args_type = "qom-type:s,id:s,props:q?", .mhandler.cmd_new = qmp_marshal_object_add, }, @@ -1029,7 +994,6 @@ EQMP { .name = "object-del", - .args_type = "id:s", .mhandler.cmd_new = qmp_marshal_object_del, }, @@ -1054,7 +1018,6 @@ EQMP { .name = "block_resize", - .args_type = "device:s?,node-name:s?,size:o", .mhandler.cmd_new = qmp_marshal_block_resize, }, @@ -1079,7 +1042,6 @@ EQMP { .name = "block-stream", - .args_type = "device:B,base:s?,speed:o?,backing-file:s?,on-error:s?", .mhandler.cmd_new = qmp_marshal_block_stream, }, @@ -1122,7 +1084,6 @@ EQMP { .name = "block-commit", - .args_type = "device:B,base:s?,top:s?,backing-file:s?,speed:o?", .mhandler.cmd_new = qmp_marshal_block_commit, }, @@ -1185,8 +1146,6 @@ EQMP { .name = "drive-backup", - .args_type = "sync:s,device:B,target:s,speed:i?,mode:s?,format:s?," - "bitmap:s?,on-source-error:s?,on-target-error:s?", .mhandler.cmd_new = qmp_marshal_drive_backup, }, @@ -1239,8 +1198,6 @@ EQMP { .name = "blockdev-backup", - .args_type = "sync:s,device:B,target:B,speed:i?," - "on-source-error:s?,on-target-error:s?", .mhandler.cmd_new = qmp_marshal_blockdev_backup, }, @@ -1280,33 +1237,27 @@ EQMP { .name = "block-job-set-speed", - .args_type = "device:B,speed:o", .mhandler.cmd_new = qmp_marshal_block_job_set_speed, }, { .name = "block-job-cancel", - .args_type = "device:B,force:b?", .mhandler.cmd_new = qmp_marshal_block_job_cancel, }, { .name = "block-job-pause", - .args_type = "device:B", .mhandler.cmd_new = qmp_marshal_block_job_pause, }, { .name = "block-job-resume", - .args_type = "device:B", .mhandler.cmd_new = qmp_marshal_block_job_resume, }, { .name = "block-job-complete", - .args_type = "device:B", .mhandler.cmd_new = qmp_marshal_block_job_complete, }, { .name = "transaction", - .args_type = "actions:q,properties:q?", .mhandler.cmd_new = qmp_marshal_transaction, }, @@ -1400,7 +1351,6 @@ EQMP { .name = "block-dirty-bitmap-add", - .args_type = "node:B,name:s,granularity:i?", .mhandler.cmd_new = qmp_marshal_block_dirty_bitmap_add, }, @@ -1428,7 +1378,6 @@ EQMP { .name = "block-dirty-bitmap-remove", - .args_type = "node:B,name:s", .mhandler.cmd_new = qmp_marshal_block_dirty_bitmap_remove, }, @@ -1456,7 +1405,6 @@ EQMP { .name = "block-dirty-bitmap-clear", - .args_type = "node:B,name:s", .mhandler.cmd_new = qmp_marshal_block_dirty_bitmap_clear, }, @@ -1485,7 +1433,6 @@ EQMP { .name = "blockdev-snapshot-sync", - .args_type = "device:s?,node-name:s?,snapshot-file:s,snapshot-node-name:s?,format:s?,mode:s?", .mhandler.cmd_new = qmp_marshal_blockdev_snapshot_sync, }, @@ -1521,7 +1468,6 @@ EQMP { .name = "blockdev-snapshot", - .args_type = "node:s,overlay:s", .mhandler.cmd_new = qmp_marshal_blockdev_snapshot, }, @@ -1559,7 +1505,6 @@ EQMP { .name = "blockdev-snapshot-internal-sync", - .args_type = "device:B,name:s", .mhandler.cmd_new = qmp_marshal_blockdev_snapshot_internal_sync, }, @@ -1588,7 +1533,6 @@ EQMP { .name = "blockdev-snapshot-delete-internal-sync", - .args_type = "device:B,id:s?,name:s?", .mhandler.cmd_new = qmp_marshal_blockdev_snapshot_delete_internal_sync, }, @@ -1629,11 +1573,6 @@ EQMP { .name = "drive-mirror", - .args_type = "sync:s,device:B,target:s,speed:i?,mode:s?,format:s?," - "node-name:s?,replaces:s?," - "on-source-error:s?,on-target-error:s?," - "unmap:b?," - "granularity:i?,buf-size:i?", .mhandler.cmd_new = qmp_marshal_drive_mirror, }, @@ -1693,9 +1632,6 @@ EQMP { .name = "blockdev-mirror", - .args_type = "sync:s,device:B,target:B,replaces:s?,speed:i?," - "on-source-error:s?,on-target-error:s?," - "granularity:i?,buf-size:i?", .mhandler.cmd_new = qmp_marshal_blockdev_mirror, }, @@ -1741,7 +1677,6 @@ Example: EQMP { .name = "change-backing-file", - .args_type = "device:s,image-node-name:s,backing-file:s", .mhandler.cmd_new = qmp_marshal_change_backing_file, }, @@ -1780,7 +1715,6 @@ EQMP { .name = "balloon", - .args_type = "value:M", .mhandler.cmd_new = qmp_marshal_balloon, }, @@ -1803,7 +1737,6 @@ EQMP { .name = "set_link", - .args_type = "name:s,up:b", .mhandler.cmd_new = qmp_marshal_set_link, }, @@ -1827,7 +1760,6 @@ EQMP { .name = "getfd", - .args_type = "fdname:s", .params = "getfd name", .help = "receive a file descriptor via SCM rights and assign it a name", .mhandler.cmd_new = qmp_marshal_getfd, @@ -1860,7 +1792,6 @@ EQMP { .name = "closefd", - .args_type = "fdname:s", .params = "closefd name", .help = "close a file descriptor previously passed via SCM rights", .mhandler.cmd_new = qmp_marshal_closefd, @@ -1885,7 +1816,6 @@ EQMP { .name = "add-fd", - .args_type = "fdset-id:i?,opaque:s?", .params = "add-fd fdset-id opaque", .help = "Add a file descriptor, that was passed via SCM rights, to an fd set", .mhandler.cmd_new = qmp_marshal_add_fd, @@ -1924,7 +1854,6 @@ EQMP { .name = "remove-fd", - .args_type = "fdset-id:i,fd:i?", .params = "remove-fd fdset-id fd", .help = "Remove a file descriptor from an fd set", .mhandler.cmd_new = qmp_marshal_remove_fd, @@ -1957,7 +1886,6 @@ EQMP { .name = "query-fdsets", - .args_type = "", .help = "Return information describing all fd sets", .mhandler.cmd_new = qmp_marshal_query_fdsets, }, @@ -2007,7 +1935,6 @@ EQMP { .name = "block_passwd", - .args_type = "device:s?,node-name:s?,password:s", .mhandler.cmd_new = qmp_marshal_block_passwd, }, @@ -2033,7 +1960,6 @@ EQMP { .name = "block_set_io_throttle", - .args_type = "device:B,bps:l,bps_rd:l,bps_wr:l,iops:l,iops_rd:l,iops_wr:l,bps_max:l?,bps_rd_max:l?,bps_wr_max:l?,iops_max:l?,iops_rd_max:l?,iops_wr_max:l?,bps_max_length:l?,bps_rd_max_length:l?,bps_wr_max_length:l?,iops_max_length:l?,iops_rd_max_length:l?,iops_wr_max_length:l?,iops_size:l?,group:s?", .mhandler.cmd_new = qmp_marshal_block_set_io_throttle, }, @@ -2090,7 +2016,6 @@ EQMP { .name = "set_password", - .args_type = "protocol:s,password:s,connected:s?", .mhandler.cmd_new = qmp_marshal_set_password, }, @@ -2116,7 +2041,6 @@ EQMP { .name = "expire_password", - .args_type = "protocol:s,time:s", .mhandler.cmd_new = qmp_marshal_expire_password, }, @@ -2141,7 +2065,6 @@ EQMP { .name = "add_client", - .args_type = "protocol:s,fdname:s,skipauth:b?,tls:b?", .mhandler.cmd_new = qmp_marshal_add_client, }, @@ -2192,7 +2115,6 @@ EQMP { .name = "human-monitor-command", - .args_type = "command-line:s,cpu-index:i?", .mhandler.cmd_new = qmp_marshal_human_monitor_command, }, @@ -2271,7 +2193,6 @@ EQMP { .name = "query-version", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_version, }, @@ -2308,7 +2229,6 @@ EQMP { .name = "query-commands", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_commands, }, @@ -2345,7 +2265,6 @@ EQMP { .name = "query-events", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_events, }, @@ -2362,7 +2281,7 @@ EQMP { .name = "query-qmp-schema", - .args_type = "", + .args_type = "", .mhandler.cmd_new = qmp_query_qmp_schema, }, @@ -2407,7 +2326,6 @@ EQMP { .name = "query-chardev", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_chardev, }, @@ -2448,7 +2366,6 @@ EQMP { .name = "query-chardev-backends", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_chardev_backends, }, @@ -2632,7 +2549,6 @@ EQMP { .name = "query-block", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_block, }, @@ -2829,7 +2745,6 @@ EQMP { .name = "query-blockstats", - .args_type = "query-nodes:b?", .mhandler.cmd_new = qmp_marshal_query_blockstats, }, @@ -2884,7 +2799,6 @@ EQMP { .name = "query-cpus", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_cpus, }, @@ -2923,7 +2837,6 @@ EQMP { .name = "query-iothreads", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_iothreads, }, @@ -3140,7 +3053,6 @@ EQMP { .name = "query-pci", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_pci, }, @@ -3164,7 +3076,6 @@ EQMP { .name = "query-kvm", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_kvm, }, @@ -3204,7 +3115,6 @@ EQMP { .name = "query-status", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_status, }, @@ -3248,7 +3158,6 @@ EQMP { .name = "query-mice", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_mice, }, @@ -3311,12 +3220,10 @@ EQMP { .name = "query-vnc", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_vnc, }, { .name = "query-vnc-servers", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_vnc_servers, }, @@ -3393,7 +3300,6 @@ EQMP #if defined(CONFIG_SPICE) { .name = "query-spice", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_spice, }, #endif @@ -3417,7 +3323,6 @@ EQMP { .name = "query-name", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_name, }, @@ -3440,7 +3345,6 @@ EQMP { .name = "query-uuid", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_uuid, }, @@ -3489,7 +3393,6 @@ EQMP { .name = "query-command-line-options", - .args_type = "option:s?", .mhandler.cmd_new = qmp_marshal_query_command_line_options, }, @@ -3667,7 +3570,6 @@ EQMP { .name = "query-migrate", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_migrate, }, @@ -3696,7 +3598,6 @@ EQMP { .name = "migrate-set-capabilities", - .args_type = "capabilities:q", .params = "capability:s,state:b", .mhandler.cmd_new = qmp_marshal_migrate_set_capabilities, }, @@ -3734,7 +3635,6 @@ EQMP { .name = "query-migrate-capabilities", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_migrate_capabilities, }, @@ -3763,8 +3663,6 @@ EQMP { .name = "migrate-set-parameters", - .args_type = - "compress-level:i?,compress-threads:i?,decompress-threads:i?,x-cpu-throttle-initial:i?,x-cpu-throttle-increment:i?", .mhandler.cmd_new = qmp_marshal_migrate_set_parameters, }, SQMP @@ -3801,7 +3699,6 @@ EQMP { .name = "query-migrate-parameters", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_migrate_parameters, }, @@ -3829,88 +3726,73 @@ EQMP { .name = "query-balloon", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_balloon, }, { .name = "query-block-jobs", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_block_jobs, }, { .name = "qom-list", - .args_type = "path:s", .mhandler.cmd_new = qmp_marshal_qom_list, }, { .name = "qom-set", - .args_type = "path:s,property:s,value:q", .mhandler.cmd_new = qmp_marshal_qom_set, }, { .name = "qom-get", - .args_type = "path:s,property:s", .mhandler.cmd_new = qmp_marshal_qom_get, }, { .name = "nbd-server-start", - .args_type = "addr:q,tls-creds:s?", .mhandler.cmd_new = qmp_marshal_nbd_server_start, }, { .name = "nbd-server-add", - .args_type = "device:B,writable:b?", .mhandler.cmd_new = qmp_marshal_nbd_server_add, }, { .name = "nbd-server-stop", - .args_type = "", .mhandler.cmd_new = qmp_marshal_nbd_server_stop, }, { .name = "change-vnc-password", - .args_type = "password:s", .mhandler.cmd_new = qmp_marshal_change_vnc_password, }, { .name = "qom-list-types", - .args_type = "implements:s?,abstract:b?", .mhandler.cmd_new = qmp_marshal_qom_list_types, }, { .name = "device-list-properties", - .args_type = "typename:s", .mhandler.cmd_new = qmp_marshal_device_list_properties, }, { .name = "query-machines", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_machines, }, { .name = "query-cpu-definitions", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_cpu_definitions, }, { .name = "query-target", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_target, }, { .name = "query-tpm", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_tpm, }, @@ -3944,7 +3826,6 @@ EQMP { .name = "query-tpm-models", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_tpm_models, }, @@ -3965,7 +3846,6 @@ EQMP { .name = "query-tpm-types", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_tpm_types, }, @@ -3986,7 +3866,6 @@ EQMP { .name = "chardev-add", - .args_type = "id:s,backend:q", .mhandler.cmd_new = qmp_marshal_chardev_add, }, @@ -4023,7 +3902,6 @@ EQMP { .name = "chardev-remove", - .args_type = "id:s", .mhandler.cmd_new = qmp_marshal_chardev_remove, }, @@ -4046,7 +3924,6 @@ Example: EQMP { .name = "query-rx-filter", - .args_type = "name:s?", .mhandler.cmd_new = qmp_marshal_query_rx_filter, }, @@ -4112,7 +3989,6 @@ EQMP { .name = "blockdev-add", - .args_type = "options:q", .mhandler.cmd_new = qmp_marshal_blockdev_add, }, @@ -4171,7 +4047,6 @@ EQMP { .name = "x-blockdev-del", - .args_type = "id:s?,node-name:s?", .mhandler.cmd_new = qmp_marshal_x_blockdev_del, }, @@ -4228,7 +4103,6 @@ EQMP { .name = "blockdev-open-tray", - .args_type = "device:s,force:b?", .mhandler.cmd_new = qmp_marshal_blockdev_open_tray, }, @@ -4276,7 +4150,6 @@ EQMP { .name = "blockdev-close-tray", - .args_type = "device:s", .mhandler.cmd_new = qmp_marshal_blockdev_close_tray, }, @@ -4311,7 +4184,6 @@ EQMP { .name = "x-blockdev-remove-medium", - .args_type = "device:s", .mhandler.cmd_new = qmp_marshal_x_blockdev_remove_medium, }, @@ -4359,7 +4231,6 @@ EQMP { .name = "x-blockdev-insert-medium", - .args_type = "device:s,node-name:s", .mhandler.cmd_new = qmp_marshal_x_blockdev_insert_medium, }, @@ -4399,7 +4270,6 @@ EQMP { .name = "query-named-block-nodes", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_named_block_nodes, }, @@ -4461,7 +4331,6 @@ EQMP { .name = "blockdev-change-medium", - .args_type = "device:B,filename:F,format:s?,read-only-mode:s?", .mhandler.cmd_new = qmp_marshal_blockdev_change_medium, }, @@ -4514,7 +4383,6 @@ EQMP { .name = "query-memdev", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_memdev, }, @@ -4552,7 +4420,6 @@ EQMP { .name = "query-memory-devices", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_memory_devices, }, @@ -4579,7 +4446,6 @@ EQMP { .name = "query-acpi-ospm-status", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_acpi_ospm_status, }, @@ -4602,7 +4468,6 @@ EQMP #if defined TARGET_I386 { .name = "rtc-reset-reinjection", - .args_type = "", .mhandler.cmd_new = qmp_marshal_rtc_reset_reinjection, }, #endif @@ -4623,7 +4488,6 @@ EQMP { .name = "trace-event-get-state", - .args_type = "name:s", .mhandler.cmd_new = qmp_marshal_trace_event_get_state, }, @@ -4641,7 +4505,6 @@ EQMP { .name = "trace-event-set-state", - .args_type = "name:s,enable:b,ignore-unavailable:b?", .mhandler.cmd_new = qmp_marshal_trace_event_set_state, }, @@ -4659,7 +4522,6 @@ EQMP { .name = "input-send-event", - .args_type = "console:i?,events:q", .mhandler.cmd_new = qmp_marshal_input_send_event, }, @@ -4725,7 +4587,6 @@ EQMP { .name = "block-set-write-threshold", - .args_type = "node-name:s,write-threshold:l", .mhandler.cmd_new = qmp_marshal_block_set_write_threshold, }, @@ -4753,7 +4614,6 @@ EQMP { .name = "query-rocker", - .args_type = "name:s", .mhandler.cmd_new = qmp_marshal_query_rocker, }, @@ -4774,7 +4634,6 @@ EQMP { .name = "query-rocker-ports", - .args_type = "name:s", .mhandler.cmd_new = qmp_marshal_query_rocker_ports, }, @@ -4799,7 +4658,6 @@ EQMP { .name = "query-rocker-of-dpa-flows", - .args_type = "name:s,tbl-id:i?", .mhandler.cmd_new = qmp_marshal_query_rocker_of_dpa_flows, }, @@ -4828,7 +4686,6 @@ EQMP { .name = "query-rocker-of-dpa-groups", - .args_type = "name:s,type:i?", .mhandler.cmd_new = qmp_marshal_query_rocker_of_dpa_groups, }, @@ -4859,7 +4716,6 @@ EQMP #if defined TARGET_ARM { .name = "query-gic-capabilities", - .args_type = "", .mhandler.cmd_new = qmp_marshal_query_gic_capabilities, }, #endif
Having to manually call out the set of expected arguments in qmp-commands.hx, in addition to what is already in *.json, is tedious and prone to error. The only reason we use .args_type is for checking if there is any excess arguments or incorrectly typed arguments during qmp_check_client_args(), but a strict QMP Input visitor also does that checking. Simplify things so that .args_type is only needed for the few remaining commands that don't have a QAPI description (namely, device_add, netdev_add, qmp_capabilities) or which use 'gen':false (query-qmp-schema). There was one case where the generated marshal code has to be beefed up: for a command that has no (or empty) 'data' in the .json file, we were just ignoring the passed-in QDict; now we need to validate that there are no extra arguments. Generated code changes like: |@@ -1206,7 +1206,10 @@ void qmp_marshal_cont(QDict *args, QObje | { | Error *err = NULL; | |- (void)args; |+ if (args && qdict_size(args)) { |+ error_setg(errp, "Command %s does not take arguments", "cont"); |+ return; |+ } | | qmp_cont(&err); | error_propagate(errp, err); QMP behavior for no-arg commands changes from: {"execute":"query-version","arguments":{"a":1}} {"error": {"class": "GenericError", "desc": "Invalid parameter 'a'"}} to: {"execute":"query-version","arguments":{"a":1}} {"error": {"class": "GenericError", "desc": "Command 'query-version' does not take arguments"}} and for commands with at least one (possibly-optional) argument, the output changes from: {"execute":"query-command-line-options","arguments":{"a":1}} {"error": {"class": "GenericError", "desc": "Invalid parameter 'a'"}} to: {"execute":"query-command-line-options","arguments":{"a":1}} {"error": {"class": "GenericError", "desc": "QMP input object member 'a' is unexpected"}} Signed-off-by: Eric Blake <eblake@redhat.com> --- v15: new patch --- scripts/qapi-commands.py | 8 ++- monitor.c | 4 ++ qmp-commands.hx | 148 +---------------------------------------------- 3 files changed, 12 insertions(+), 148 deletions(-)