From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: armbru@redhat.com
Subject: [Qemu-devel] [PATCH v11 00/28] qapi member collision, alternate layout (post-introspection cleanups, subset D)
Date: Tue, 10 Nov 2015 23:51:02 -0700 [thread overview]
Message-ID: <1447224690-9743-1-git-send-email-eblake@redhat.com> (raw)
No pending prerequisites; based on qemu.git master
Also available as a tag at this location:
git fetch git://repo.or.cz/qemu/ericb.git qapi-cleanupv11d
and will soon be part of my branch with the rest of the v5 series, at:
http://repo.or.cz/qemu/ericb.git/shortlog/refs/heads/qapi
v11 notes:
First half of v10 subset C has been applied, so consolidate the last
half of that along with all of subset D (which was at v9) into one
group. Address list reviews, in particular, add a new patch 21 that
makes alternate layouts a lot nicer by making qtype_code a builtin
qapi type; and new patches 18-19 that try to reduce confusion on
the use of camel_to_upper() in c_enum_const().
Probably too late to get these into 2.5, in which case 17/28 will need
tweaks to call out 2.6.
backport diff (1-20 against v10c, 21-28 against v9d)
001/28:[0001] [FC] 'qapi: Track simple union tag in object.local_members'
002/28:[----] [--] 'qapi-types: Consolidate gen_struct() and gen_union()'
003/28:[----] [--] 'qapi-types: Simplify gen_struct_field[s]'
004/28:[----] [--] 'qapi: Drop obsolete tag value collision assertions'
005/28:[----] [--] 'qapi: Simplify QAPISchemaObjectTypeMember.check()'
006/28:[0001] [FC] 'qapi: Clean up after previous commit'
007/28:[----] [--] 'qapi: Fix up commit 7618b91's clash sanity checking change'
008/28:[----] [--] 'qapi: Eliminate QAPISchemaObjectType.check() variable members'
009/28:[----] [--] 'qapi: Factor out QAPISchemaObjectTypeMember.check_clash()'
010/28:[0002] [FC] 'qapi: Simplify QAPISchemaObjectTypeVariants.check()'
011/28:[0007] [FC] 'qapi: Check for qapi collisions of flat union branches'
012/28:[0005] [FC] 'qapi: Factor out QAPISchemaObjectType.check_clash()'
013/28:[0005] [FC] 'qapi: Hoist tag collision check to Variants.check()'
014/28:[----] [--] 'qapi: Remove outdated tests related to QMP/branch collisions'
015/28:[0025] [FC] 'qapi: Track owner of each object member'
016/28:[0013] [FC] 'qapi: Detect collisions in C member names'
017/28:[0003] [FC] 'cpu: Convert CpuInfo into flat union'
018/28:[down] 'qerror: more error_setg() usage'
019/28:[down] 'qapi: Change munging of CamelCase enum values'
020/28:[0017] [FC] 'qapi: Forbid case-insensitive clashes'
021/28:[down] 'qapi: Convert qtype_code into qapi enum type'
022/28:[0090] [FC] 'qapi: Simplify visiting of alternate types'
023/28:[0022] [FC] 'qapi: Fix alternates that accept 'number' but not 'int''
024/28:[0015] [FC] 'qapi: Add positive tests to qapi-schema-test'
025/28:[----] [-C] 'qapi: Simplify visits of optional fields'
026/28:[----] [--] 'qapi: Move duplicate member checks to schema check()'
027/28:[0002] [FC] 'qapi: Move duplicate enum value checks to schema check()'
028/28:[0009] [FC] 'qapi: Detect base class loops'
v10 notes:
https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg01249.html
Split several patches, redo the middle patches from Markus to be
back in the order they were first posted, some fallout change to
my patches due to the nicer pattern of minimizing conditionals
inside .check(), by instead calling .check_clash() as needed.
Change data->err magic in tests to instead use a new helper
error_free_or_abort(). Add a patch that would prevent qapi
case-insensitive clashes.
I am redoing my subset boundaries slightly: patches 23-27 of
v9 (updating the alternate layout) will be delayed to subset D,
and 2 other patches previously posted in subset D are now here
(turning qapi clash checking into actual error messages), so
the subject line of this cover letter is slightly different.
Hopefully, we are converging on something that will be ready
for a pull request, especially for the earlier patches of this
subset.
v9 notes:
https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg00652.html
https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg06999.html
More patches added, and several reorganized. Lots of new patches
from Markus, although not in the order originally proposed.
The first 8 patches are fairly straightforward, and could probably
be taken as-is. Patch 9 is a rewrite of v8 4/17, but in the opposite
direction (document that no sorting is done, rather than attempting
to sort), so it may need further fine-tuning. Patches 12-21
represents a fusion of Markus' and my attempts to rewrite v5 7/17
into a more-reviewable set of patches, and caused further churn
later in the series.
Patch 23 still uses tag_member.type == None; I ran out of time to
work on Markus' idea of providing an instance of QAPISchemaBuiltinType
to fill the role for 'qtype_code' without being exposed through .json
files and without breaking the invariant of a valid member.type after
check(), and wanted to get the rest of the series started under revew.
So I may need a followup patch or even a v10 of the later half of
this series after exploring that idea more.
v8 notes:
https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg06674.html
Minor changes when rebasing to latest, improved commit messages in
a few places, plus the addition of a few new patches. Markus started
reviewing v7, but hadn't gotten very far.
v7 notes:
https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg04112.html
Patches 1-3 of the previous round have moved to subset B; patch 7
is gone, and a couple of new patches are present in this round. The
latest version of subset B made it much easier to reason about
collision detection (namely, tag values can't collide with QMP values
thanks to a named rather than anonymous union in the C type), and I
like how things turned out. I suspect the hardest part of the review
will be patches 5/14 and 7/14, although none of this has really
had much review in any earlier versions.
v6 notes:
https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg01980.html
Add some patches and rebase onto work on subset B. Rearrange some
patches from v5 (this set includes 17-20, 23, 25-27). Backport diff
gets a bit confused by one patch title changing.
Not much direct review comments, although some of the changes here
are updated based on comments made on other patches in the v5 series.
Subset D (and more?) will come later.
In v5:
https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg05410.html
I _did_ rearrange patches to try and group related features:
1-2: Groundwork cleanups
3-5: Add more test cases
6-16: Front-end cleanups
17-18: Introspection output cleanups
19-20: 'alternate' type cleanups
21-29: qapi visitor cleanups
30-45: qapi-ify netdev_add
46: add qapi shorthand for flat unions
Lots of fixes based on additional testing, and rebased to
track other changes that happened in the meantime. The series
is huge; I can split off smaller portions as requested.
In v4:
https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg02580.html
add some more clean up patches
rebase to Markus' recent work
pull in part of Zoltán's work to make netdev_add a flat union,
further enhancing it to be introspectible
I might be able to rearrange some of these patches, or separate
it into smaller independent series, if requested; but I'm
posting now to get review started.
In v3:
https://lists.gnu.org/archive/html/qemu-devel/2015-08/msg02059.html
redo cleanup of dealloc of partial struct
add patches to make all visit_type_*() avoid leaks on failure
add patches to allow boxed command arguments and events
In v2:
https://lists.gnu.org/archive/html/qemu-devel/2015-08/msg00900.html
rebase to Markus' v3 series
rework how comments are emitted for fields inherited from base
additional patches added for deleting colliding 'void *data'
documentation updates to match code changes
v1 was here:
https://lists.gnu.org/archive/html/qemu-devel/2015-07/msg05266.html
https://lists.gnu.org/archive/html/qemu-devel/2015-07/msg05325.html
Eric Blake (22):
qapi: Track simple union tag in object.local_members
qapi-types: Consolidate gen_struct() and gen_union()
qapi-types: Simplify gen_struct_field[s]
qapi: Clean up after previous commit
qapi: Check for qapi collisions of flat union branches
qapi: Factor out QAPISchemaObjectType.check_clash()
qapi: Hoist tag collision check to Variants.check()
qapi: Remove outdated tests related to QMP/branch collisions
qapi: Track owner of each object member
qapi: Detect collisions in C member names
cpu: Convert CpuInfo into flat union
qerror: more error_setg() usage
qapi: Change munging of CamelCase enum values
qapi: Forbid case-insensitive clashes
qapi: Convert qtype_code into qapi enum type
qapi: Simplify visiting of alternate types
qapi: Fix alternates that accept 'number' but not 'int'
qapi: Add positive tests to qapi-schema-test
qapi: Simplify visits of optional fields
qapi: Move duplicate member checks to schema check()
qapi: Move duplicate enum value checks to schema check()
qapi: Detect base class loops
Markus Armbruster (6):
qapi: Drop obsolete tag value collision assertions
qapi: Simplify QAPISchemaObjectTypeMember.check()
qapi: Fix up commit 7618b91's clash sanity checking change
qapi: Eliminate QAPISchemaObjectType.check() variable members
qapi: Factor out QAPISchemaObjectTypeMember.check_clash()
qapi: Simplify QAPISchemaObjectTypeVariants.check()
backends/rng-egd.c | 2 +-
balloon.c | 4 +-
block.c | 5 +-
block/qapi.c | 4 +-
blockdev-nbd.c | 2 +-
blockdev.c | 24 +--
cpus.c | 31 +--
docs/qapi-code-gen.txt | 7 +-
docs/writing-qmp-commands.txt | 20 +-
hmp.c | 36 ++--
hw/i386/pc.c | 2 +-
hw/input/hid.c | 4 +-
hw/input/ps2.c | 4 +-
hw/input/virtio-input-hid.c | 4 +-
hw/net/rocker/rocker.c | 6 +-
hw/net/rocker/rocker_of_dpa.c | 12 +-
include/hw/qdev-core.h | 2 +-
include/qapi/error.h | 6 +-
include/qapi/qmp/qobject.h | 19 +-
include/qapi/visitor-impl.h | 8 +-
include/qapi/visitor.h | 19 +-
include/ui/qemu-spice.h | 2 +-
monitor.c | 8 +-
net/net.c | 4 +-
qapi-schema.json | 120 +++++++++--
qapi/opts-visitor.c | 2 +-
qapi/qapi-visit-core.c | 10 +-
qapi/qmp-dispatch.c | 2 +-
qapi/qmp-input-visitor.c | 11 +-
qapi/string-input-visitor.c | 3 +-
qdev-monitor.c | 4 +-
qmp-commands.hx | 4 +
qmp.c | 8 +-
qobject/qdict.c | 2 +-
qom/object.c | 8 +-
scripts/qapi-types.py | 115 +++-------
scripts/qapi-visit.py | 34 ++-
scripts/qapi.py | 234 ++++++++++++---------
tests/Makefile | 7 +-
tests/qapi-schema/alternate-clash.err | 2 +-
tests/qapi-schema/alternate-empty.out | 3 +-
tests/qapi-schema/args-case-clash.err | 1 +
...{union-clash-type.exit => args-case-clash.exit} | 0
tests/qapi-schema/args-case-clash.json | 4 +
.../{union-clash-type.out => args-case-clash.out} | 0
tests/qapi-schema/args-name-clash.err | 1 +
tests/qapi-schema/args-name-clash.exit | 2 +-
tests/qapi-schema/args-name-clash.json | 5 +-
tests/qapi-schema/args-name-clash.out | 6 -
tests/qapi-schema/base-cycle-direct.err | 1 +
...nion-bad-branch.exit => base-cycle-direct.exit} | 0
tests/qapi-schema/base-cycle-direct.json | 2 +
...{union-bad-branch.out => base-cycle-direct.out} | 0
tests/qapi-schema/base-cycle-indirect.err | 1 +
...on-clash-type.exit => base-cycle-indirect.exit} | 0
tests/qapi-schema/base-cycle-indirect.json | 3 +
...nion-clash-type.out => base-cycle-indirect.out} | 0
tests/qapi-schema/comments.out | 2 +
tests/qapi-schema/empty.out | 2 +
tests/qapi-schema/enum-clash-member.err | 2 +-
tests/qapi-schema/enum-max-member.err | 2 +-
tests/qapi-schema/event-case.out | 2 +
tests/qapi-schema/flat-union-clash-branch.err | 0
tests/qapi-schema/flat-union-clash-branch.exit | 1 -
tests/qapi-schema/flat-union-clash-branch.json | 18 --
tests/qapi-schema/flat-union-clash-branch.out | 14 --
tests/qapi-schema/flat-union-clash-member.err | 2 +-
tests/qapi-schema/flat-union-clash-type.err | 1 -
tests/qapi-schema/flat-union-clash-type.json | 14 --
tests/qapi-schema/flat-union-empty.out | 2 +
tests/qapi-schema/ident-with-escape.out | 2 +
tests/qapi-schema/include-relpath.out | 2 +
tests/qapi-schema/include-repetition.out | 2 +
tests/qapi-schema/include-simple.out | 2 +
tests/qapi-schema/indented-expr.out | 2 +
tests/qapi-schema/qapi-schema-test.json | 16 ++
tests/qapi-schema/qapi-schema-test.out | 38 +++-
tests/qapi-schema/struct-base-clash-deep.err | 2 +-
tests/qapi-schema/struct-base-clash.err | 2 +-
tests/qapi-schema/union-bad-branch.err | 1 -
tests/qapi-schema/union-bad-branch.json | 8 -
tests/qapi-schema/union-clash-branches.err | 2 +-
tests/qapi-schema/union-clash-branches.json | 2 +-
tests/qapi-schema/union-clash-data.out | 3 +
tests/qapi-schema/union-clash-type.err | 1 -
tests/qapi-schema/union-clash-type.json | 9 -
tests/qapi-schema/union-empty.out | 3 +
tests/qapi-schema/union-max.err | 2 +-
tests/test-qmp-input-visitor.c | 29 ++-
tests/test-qmp-output-visitor.c | 4 +-
ui/cocoa.m | 4 +-
ui/gtk.c | 4 +-
ui/input-legacy.c | 4 +-
ui/input.c | 2 +-
ui/sdl.c | 4 +-
ui/sdl2.c | 4 +-
ui/spice-input.c | 4 +-
ui/vnc.c | 4 +-
util/error.c | 6 +-
99 files changed, 577 insertions(+), 483 deletions(-)
create mode 100644 tests/qapi-schema/args-case-clash.err
rename tests/qapi-schema/{union-clash-type.exit => args-case-clash.exit} (100%)
create mode 100644 tests/qapi-schema/args-case-clash.json
rename tests/qapi-schema/{union-clash-type.out => args-case-clash.out} (100%)
create mode 100644 tests/qapi-schema/base-cycle-direct.err
rename tests/qapi-schema/{union-bad-branch.exit => base-cycle-direct.exit} (100%)
create mode 100644 tests/qapi-schema/base-cycle-direct.json
rename tests/qapi-schema/{union-bad-branch.out => base-cycle-direct.out} (100%)
create mode 100644 tests/qapi-schema/base-cycle-indirect.err
rename tests/qapi-schema/{flat-union-clash-type.exit => base-cycle-indirect.exit} (100%)
create mode 100644 tests/qapi-schema/base-cycle-indirect.json
rename tests/qapi-schema/{flat-union-clash-type.out => base-cycle-indirect.out} (100%)
delete mode 100644 tests/qapi-schema/flat-union-clash-branch.err
delete mode 100644 tests/qapi-schema/flat-union-clash-branch.exit
delete mode 100644 tests/qapi-schema/flat-union-clash-branch.json
delete mode 100644 tests/qapi-schema/flat-union-clash-branch.out
delete mode 100644 tests/qapi-schema/flat-union-clash-type.err
delete mode 100644 tests/qapi-schema/flat-union-clash-type.json
delete mode 100644 tests/qapi-schema/union-bad-branch.err
delete mode 100644 tests/qapi-schema/union-bad-branch.json
delete mode 100644 tests/qapi-schema/union-clash-type.err
delete mode 100644 tests/qapi-schema/union-clash-type.json
--
2.4.3
next reply other threads:[~2015-11-11 6:51 UTC|newest]
Thread overview: 72+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-11 6:51 Eric Blake [this message]
2015-11-11 6:51 ` [Qemu-devel] [PATCH v11 01/28] qapi: Track simple union tag in object.local_members Eric Blake
2015-11-11 6:51 ` [Qemu-devel] [PATCH v11 02/28] qapi-types: Consolidate gen_struct() and gen_union() Eric Blake
2015-11-11 6:51 ` [Qemu-devel] [PATCH v11 03/28] qapi-types: Simplify gen_struct_field[s] Eric Blake
2015-11-11 6:51 ` [Qemu-devel] [PATCH v11 04/28] qapi: Drop obsolete tag value collision assertions Eric Blake
2015-11-11 6:51 ` [Qemu-devel] [PATCH v11 05/28] qapi: Simplify QAPISchemaObjectTypeMember.check() Eric Blake
2015-11-11 6:51 ` [Qemu-devel] [PATCH v11 06/28] qapi: Clean up after previous commit Eric Blake
2015-11-11 6:51 ` [Qemu-devel] [PATCH v11 07/28] qapi: Fix up commit 7618b91's clash sanity checking change Eric Blake
2015-11-11 6:51 ` [Qemu-devel] [PATCH v11 08/28] qapi: Eliminate QAPISchemaObjectType.check() variable members Eric Blake
2015-11-11 6:51 ` [Qemu-devel] [PATCH v11 09/28] qapi: Factor out QAPISchemaObjectTypeMember.check_clash() Eric Blake
2015-11-11 6:51 ` [Qemu-devel] [PATCH v11 10/28] qapi: Simplify QAPISchemaObjectTypeVariants.check() Eric Blake
2015-11-11 6:51 ` [Qemu-devel] [PATCH v11 11/28] qapi: Check for qapi collisions of flat union branches Eric Blake
2015-11-11 13:42 ` Markus Armbruster
2015-11-11 15:49 ` Eric Blake
2015-11-11 6:51 ` [Qemu-devel] [PATCH v11 12/28] qapi: Factor out QAPISchemaObjectType.check_clash() Eric Blake
2015-11-11 6:51 ` [Qemu-devel] [PATCH v11 13/28] qapi: Hoist tag collision check to Variants.check() Eric Blake
2015-11-11 13:56 ` Markus Armbruster
2015-11-11 16:11 ` Eric Blake
2015-11-11 17:03 ` Markus Armbruster
2015-11-11 6:51 ` [Qemu-devel] [PATCH v11 14/28] qapi: Remove outdated tests related to QMP/branch collisions Eric Blake
2015-11-11 6:51 ` [Qemu-devel] [PATCH v11 15/28] qapi: Track owner of each object member Eric Blake
2015-11-11 6:51 ` [Qemu-devel] [PATCH v11 16/28] qapi: Detect collisions in C member names Eric Blake
2015-11-11 6:51 ` [Qemu-devel] [PATCH v11 17/28] cpu: Convert CpuInfo into flat union Eric Blake
2015-11-11 14:13 ` Markus Armbruster
2015-11-11 6:51 ` [Qemu-devel] [PATCH v11 18/28] qerror: more error_setg() usage Eric Blake
2015-11-11 13:26 ` Andreas Färber
2015-11-11 14:21 ` Markus Armbruster
2015-11-11 14:23 ` Andreas Färber
2015-11-11 15:51 ` Eric Blake
2015-11-11 16:19 ` Eric Blake
2015-11-11 17:31 ` Markus Armbruster
2015-11-11 17:44 ` Eric Blake
2015-11-11 6:51 ` [Qemu-devel] [PATCH v11 19/28] qapi: Change munging of CamelCase enum values Eric Blake
2015-11-11 13:29 ` Andreas Färber
2015-11-11 14:50 ` Markus Armbruster
2015-11-11 16:03 ` Eric Blake
2015-11-11 17:11 ` Markus Armbruster
2015-11-12 8:34 ` Gerd Hoffmann
2015-11-12 11:16 ` Markus Armbruster
2015-11-12 8:29 ` Gerd Hoffmann
2015-11-11 16:06 ` Eric Blake
2015-11-13 17:46 ` Eric Blake
2015-11-13 18:13 ` Markus Armbruster
2015-11-13 21:37 ` Eric Blake
2015-11-16 14:30 ` Markus Armbruster
2015-11-11 6:51 ` [Qemu-devel] [PATCH v11 20/28] qapi: Forbid case-insensitive clashes Eric Blake
2015-11-11 14:53 ` Markus Armbruster
2015-11-13 5:32 ` Eric Blake
2015-11-11 6:51 ` [Qemu-devel] [PATCH v11 21/28] qapi: Convert qtype_code into qapi enum type Eric Blake
2015-11-11 16:42 ` Markus Armbruster
2015-11-11 17:03 ` Eric Blake
2015-11-12 13:16 ` Markus Armbruster
2015-11-18 6:27 ` Eric Blake
2015-11-11 6:51 ` [Qemu-devel] [PATCH v11 22/28] qapi: Simplify visiting of alternate types Eric Blake
2015-11-12 14:21 ` Markus Armbruster
2015-11-12 15:54 ` Markus Armbruster
2015-11-13 23:54 ` Eric Blake
2015-11-16 14:31 ` Markus Armbruster
2015-11-11 6:51 ` [Qemu-devel] [PATCH v11 23/28] qapi: Fix alternates that accept 'number' but not 'int' Eric Blake
2015-11-12 15:01 ` Markus Armbruster
2015-11-11 6:51 ` [Qemu-devel] [PATCH v11 24/28] qapi: Add positive tests to qapi-schema-test Eric Blake
2015-11-11 6:51 ` [Qemu-devel] [PATCH v11 25/28] qapi: Simplify visits of optional fields Eric Blake
2015-11-12 15:11 ` Markus Armbruster
2015-11-12 15:30 ` Eric Blake
2015-11-12 16:20 ` Markus Armbruster
2015-11-11 6:51 ` [Qemu-devel] [PATCH v11 26/28] qapi: Move duplicate member checks to schema check() Eric Blake
2015-11-11 6:51 ` [Qemu-devel] [PATCH v11 27/28] qapi: Move duplicate enum value " Eric Blake
2015-11-12 15:46 ` Markus Armbruster
2015-11-12 16:08 ` Eric Blake
2015-11-18 6:48 ` Eric Blake
2015-11-11 6:51 ` [Qemu-devel] [PATCH v11 28/28] qapi: Detect base class loops Eric Blake
2015-11-12 16:06 ` 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=1447224690-9743-1-git-send-email-eblake@redhat.com \
--to=eblake@redhat.com \
--cc=armbru@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.