All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: Eric Blake <eblake@redhat.com>
Cc: qemu-devel@nongnu.org, "Michael Roth" <mdroth@linux.vnet.ibm.com>,
	"Luiz Capitulino" <lcapitulino@redhat.com>,
	"Marc-André Lureau" <mlureau@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v15 08/23] monitor: Let generated code validate arguments
Date: Thu, 28 Apr 2016 16:09:06 +0200	[thread overview]
Message-ID: <87d1p9n7ul.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <1461801715-24307-9-git-send-email-eblake@redhat.com> (Eric Blake's message of "Wed, 27 Apr 2016 18:01:40 -0600")

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.

  reply	other threads:[~2016-04-28 14:09 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-28  0:01 [Qemu-devel] [PATCH v15 00/23] qapi visitor cleanups (post-introspection cleanups subset E) Eric Blake
2016-04-28  0:01 ` [Qemu-devel] [PATCH v15 01/23] qapi-visit: Add visitor.type classification Eric Blake
2016-04-28  0:01 ` [Qemu-devel] [PATCH v15 02/23] qapi: Guarantee NULL obj on input visitor callback error Eric Blake
2016-04-28 12:24   ` Markus Armbruster
2016-04-28 13:00     ` Eric Blake
2016-04-28 15:41       ` Eric Blake
2016-04-28 16:02   ` [Qemu-devel] [PATCH v15 02A/23] fixup! " Eric Blake
2016-04-28  0:01 ` [Qemu-devel] [PATCH v15 03/23] qmp: Drop dead command->type Eric Blake
2016-04-28  0:01 ` [Qemu-devel] [PATCH v15 04/23] qmp-input: Clean up stack handling Eric Blake
2016-04-28  0:01 ` [Qemu-devel] [PATCH v15 05/23] qapi: Use strict QMP input visitor in more places Eric Blake
2016-04-28 13:06   ` Markus Armbruster
2016-04-28 14:28     ` Eric Blake
2016-04-28  0:01 ` [Qemu-devel] [PATCH v15 06/23] qmp-input: Don't consume input when checking has_member Eric Blake
2016-04-28  0:01 ` [Qemu-devel] [PATCH v15 07/23] qapi-commands: Wrap argument visit in visit_start_struct Eric Blake
2016-04-28  0:01 ` [Qemu-devel] [PATCH v15 08/23] monitor: Let generated code validate arguments Eric Blake
2016-04-28 14:09   ` Markus Armbruster [this message]
2016-04-28 14:39     ` Marc-André Lureau
2016-04-28 18:00       ` Markus Armbruster
2016-04-28 18:58         ` Eric Blake
2016-04-28 14:47     ` Eric Blake
2016-04-28  0:01 ` [Qemu-devel] [PATCH v15 09/23] qom: Wrap prop visit in visit_start_struct Eric Blake
2016-04-28 14:46   ` Markus Armbruster
2016-04-28 15:14     ` Eric Blake
2016-04-28  0:01 ` [Qemu-devel] [PATCH v15 10/23] qmp-input: Require struct push to visit members of top dict Eric Blake
2016-04-28 15:00   ` Markus Armbruster
2016-04-28 15:04     ` Eric Blake
2016-04-28  0:01 ` [Qemu-devel] [PATCH v15 11/23] qmp-input: Refactor when list is advanced Eric Blake
2016-04-28 15:19   ` Markus Armbruster
2016-04-28  0:01 ` [Qemu-devel] [PATCH v15 12/23] qapi: Document visitor interfaces, add assertions Eric Blake
2016-04-28 16:34   ` Markus Armbruster
2016-04-28 19:02     ` Eric Blake
2016-04-28  0:01 ` [Qemu-devel] [PATCH v15 13/23] tests: Add check-qnull Eric Blake
2016-04-28  0:01 ` [Qemu-devel] [PATCH v15 14/23] qapi: Add visit_type_null() visitor Eric Blake
2016-04-28 16:40   ` Markus Armbruster
2016-04-28  0:01 ` [Qemu-devel] [PATCH v15 15/23] qmp: Support explicit null during visits Eric Blake
2016-04-28 16:50   ` Markus Armbruster
2016-04-28 19:07     ` Eric Blake
2016-04-28  0:01 ` [Qemu-devel] [PATCH v15 16/23] spapr_drc: Expose 'null' in qom-get when there is no fdt Eric Blake
2016-04-28  0:01 ` [Qemu-devel] [PATCH v15 17/23] qmp: Add qmp_output_visitor_reset() Eric Blake
2016-04-28  0:01 ` [Qemu-devel] [PATCH v15 18/23] qmp: Tighten output visitor rules Eric Blake
2016-04-28  0:01 ` [Qemu-devel] [PATCH v15 19/23] qapi: Split visit_end_struct() into pieces Eric Blake
2016-04-28  0:01 ` [Qemu-devel] [PATCH v15 20/23] tests/string-input-visitor: Add negative integer tests Eric Blake
2016-04-28  0:01 ` [Qemu-devel] [PATCH v15 21/23] qapi: Fix string input visitor handling of invalid list Eric Blake
2016-04-28 17:18   ` Markus Armbruster
2016-04-28  0:01 ` [Qemu-devel] [PATCH v15 22/23] qapi: Simplify semantics of visit_next_list() Eric Blake
2016-04-28 15:44   ` Eric Blake
2016-04-28  0:01 ` [Qemu-devel] [PATCH v15 23/23] qapi: Change visit_type_FOO() to no longer return partial objects Eric Blake
2016-04-28 17:42   ` Markus Armbruster
2016-04-28 18:03 ` [Qemu-devel] [PATCH v15 00/23] qapi visitor cleanups (post-introspection cleanups subset E) Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87d1p9n7ul.fsf@dusky.pond.sub.org \
    --to=armbru@redhat.com \
    --cc=eblake@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=mlureau@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.