[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 24/28] qapi: Enforce command naming rules
From: |
Eric Blake |
Subject: |
Re: [PATCH 24/28] qapi: Enforce command naming rules |
Date: |
Tue, 23 Mar 2021 10:23:51 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 |
On 3/23/21 4:40 AM, Markus Armbruster wrote:
> Command names should be lower-case. Enforce this. Fix the fixable
> offenders (all in tests/), and add the remainder to pragma
> command-name-exceptions.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> +++ b/qapi/pragma.json
> @@ -4,6 +4,24 @@
> # add to them!
> { 'pragma': {
> # Commands allowed to return a non-dictionary:
> + 'command-name-exceptions': [
> + 'add_client',
> + 'block_passwd',
> + 'block_resize',
> + 'block_set_io_throttle',
> + 'client_migrate_info',
> + 'device_add',
> + 'device_del',
> + 'expire_password',
> + 'migrate_cancel',
> + 'netdev_add',
> + 'netdev_del',
> + 'qmp_capabilities',
> + 'set_link',
> + 'set_password',
> + 'system_powerdown',
> + 'system_reset',
> + 'system_wakeup' ],
Outside the scope of this patch, do we have any intentions on adding
alias commands or deprecating old spellings in favor of new ones?
None of these have a capital letter...
qmp_capabilities is probably the hardest one to get rid of, since you
can't send any other commands until that one is complete (that is, you
can't introspect if a replacement exists; if we add a new spelling, all
you can do is try both spellings until one works, but that is extra
traffic). The rest can be suitably probed via introspection.
> +++ b/tests/unit/test-qmp-cmds.c
> @@ -13,7 +13,7 @@
>
> static QmpCommandList qmp_commands;
>
> -UserDefThree *qmp_TestCmdReturnDefThree(Error **errp)
> +UserDefThree *qmp_test_cmd_return_def_three(Error **errp)
...oh, we had a test command with capitals....
> +++ b/scripts/qapi/expr.py
> @@ -70,8 +70,9 @@ def check_defn_name_str(name, info, meta):
> if meta == 'event':
> check_name_upper(name, info, meta)
> elif meta == 'command':
> - check_name_lower(name, info, meta,
> - permit_upper=True, permit_underscore=True)
> + check_name_lower(
> + name, info, meta,
> + permit_underscore=name in info.pragma.command_name_exceptions)
...and earlier in the series, I had asked why you wanted
permit_upper=True here. So it is now obvious that it was just for the
tests and that you deferred fixing the tests until now. If you don't
want to refactor the series, then it's at least worth a tweak to that
commit message to call it out. At any rate, I'm glad to see the
permit_upper=True gone!
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
- [PATCH 06/28] tests/qapi-schema: Tweak to demonstrate buggy member name check, (continued)
[PATCH 05/28] tests/qapi-schema: Drop TODO comment on simple unions, Markus Armbruster, 2021/03/23
[PATCH 04/28] tests/qapi-schema: Belatedly update comment on alternate clash, Markus Armbruster, 2021/03/23
[PATCH 24/28] qapi: Enforce command naming rules, Markus Armbruster, 2021/03/23
- Re: [PATCH 24/28] qapi: Enforce command naming rules,
Eric Blake <=
[PATCH 25/28] tests/qapi-schema: Switch member name clash test to struct, Markus Armbruster, 2021/03/23
[PATCH 16/28] qapi: Factor out QAPISchemaParser._check_pragma_list_of_str(), Markus Armbruster, 2021/03/23
[PATCH 27/28] qapi: Enforce enum member naming rules, Markus Armbruster, 2021/03/23
[PATCH 23/28] qapi: Enforce feature naming rules, Markus Armbruster, 2021/03/23
[PATCH 21/28] tests-qmp-cmds: Drop unused and incorrect qmp_TestIfCmd(), Markus Armbruster, 2021/03/23