Commit Graph

41520 Commits

Author SHA1 Message Date
Eric Blake
99df5289d8 qapi: Track location that created an implicit type
A future patch will move some error checking from the parser
to the various QAPISchema*.check() methods, which run only
after parsing completes.  It will thus be possible to create
a python instance representing an implicit QAPI type that
parses fine but will fail validation during check().  Since
all errors have to have an associated 'info' location, we
need a location to be associated with those implicit types.
The intuitive info to use is the location of the enclosing
entity that caused the creation of the implicit type.

Note that we do not anticipate builtin types being used in
an error message (as they are not part of the user's QAPI
input, the user can't cause a semantic error in their
behavior), so we exempt those types from requiring info, by
setting a flag to track the completion of _def_predefineds(),
and tracking that flag in _def_entity().

No change to the generated code.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444710158-8723-13-git-send-email-eblake@redhat.com>
[Missing QAPISchemaArrayType.is_implicit() supplied]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-10-15 08:39:08 +02:00
Eric Blake
46292ba75c qapi: Create simple union type member earlier
For simple unions, we were creating the implicit 'type' tag
member during the QAPISchemaObjectTypeVariants constructor.
This is different from every other implicit QAPISchemaEntity
object, which get created by QAPISchema methods.  Hoist the
creation to the caller (renaming _make_tag_enum() to
_make_implicit_tag()), and pass the entity rather than the
string name, so that we have the nice property that no
entities are created as a side effect within a different
entity.  A later patch will then have an easier time of
associating location info with each entity creation.

No change to generated code.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444710158-8723-10-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-10-15 08:39:08 +02:00
Eric Blake
9f08c8ec73 qapi: Lazy creation of array types
Commit ac88219a had several TODO markers about whether we needed
to automatically create the corresponding array type alongside
any other type.  It turns out that most of the time, we don't!

There are a few exceptions: 1) We have a few situations where we
use an array type in internal code but do not expose that type
through QMP; fix it by declaring a dummy type that forces the
generator to see that we want to use the array type.

2) The builtin arrays (such as intList for QAPI ['int']) must
always be generated, because of the way our QAPI_TYPES_BUILTIN
compile guard works: we have situations (at the very least
tests/test-qmp-output-visitor.c) that include both top-level
"qapi-types.h" (via "error.h") and a secondary
"test-qapi-types.h". If we were to only emit the builtin types
when used locally, then the first .h file would not include all
types, but the second .h does not declare anything at all because
the first .h set QAPI_TYPES_BUILTIN, and we would end up with
compilation error due to things like unknown type 'int8List'.

Actually, we may need to revisit how we do type guards, and
change from a single QAPI_TYPES_BUILTIN over to a different
usage pattern that does one #ifdef per qapi type - right now,
the only types that are declared multiple times between two qapi
.json files for inclusion by a single .c file happen to be the
builtin arrays.  But now that we have QAPI 'include' statements,
it is logical to assume that we will soon reach a point where
we want to reuse non-builtin types (yes, I'm thinking about what
it will take to add introspection to QGA, where we will want to
reuse the SchemaInfo type and friends).  One #ifdef per type
will help ensure that generating the same qapi type into more
than one qapi-types.h won't cause collisions when both are
included in the same .c file; but we also have to solve how to
avoid creating duplicate qapi-types.c entry points.  So that
is a problem left for another day.

Generated code for qapi-types and qapi-visit is drastically
reduced; less than a third of the arrays that were blindly
created were actually needed (a quick grep shows we dropped
from 219 to 69 *List types), and the .o files lost more than
30% of their bulk.  [For best results, diff the generated
files with 'git diff --patience --no-index pre post'.]

Interestingly, the introspection output is unchanged - this is
because we already cull all types that are not indirectly
reachable from a command or event, so introspection was already
using only a subset of array types.  The subset of types
introspected is now a much larger percentage of the overall set
of array types emitted in qapi-types.h (since the larger set
shrunk), but still not 100% (evidence that the array types
emitted for our new Dummy structs, and the new struct itself,
don't affect QMP).

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444710158-8723-9-git-send-email-eblake@redhat.com>
[Moved array info tracking to a later patch]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-10-15 08:39:08 +02:00
Eric Blake
49823c4b43 qapi: Don't use info as witness of implicit object type
A future patch will enable error reporting from the various
QAPISchema*.check() methods.  But to report an error related
to an implicit type, we'll need to associate a location with
the type (the same location as the top-level entity that is
causing the creation of the implicit type), and once we do
that, keying off of whether foo.info exists is no longer a
viable way to determine if foo is an implicit type.

Instead, add an is_implicit() method to QAPISchemaEntity, and use it.
It can be overridden later for ObjectType and EnumType, when implicit
instances of those classes gain info.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444710158-8723-8-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-10-15 08:39:08 +02:00
Eric Blake
849ab13c16 qapi: Drop redundant args-member-array test
qapi-schema-test already ensures that we can correctly compile
an array of enums (__org.qemu_x-command), an array of builtins
(UserDefNativeListUnion), and an array of structs (again
__org.qemu_x-command).  That means args-member-array is not
adding any additional parse-only test coverage, so drop it.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444760807-11307-1-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-10-15 08:39:08 +02:00
Eric Blake
70478cef83 qapi: Drop redundant flat-union-reverse-define test
As of commit 8c3f8e77, we test compilation of forward references
for a struct base type (UserDefOne), flat union base type
(UserDefUnionBase), and flat union branch type
(UserDefFlatUnion2). The only remaining forward reference being
tested for parsing in flat-union-reverse-define was a forward
enum declaration.  Once we make sure that always compiles,
the smaller parse-only test is redundant and can be deleted.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444710158-8723-7-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-10-15 08:39:07 +02:00
Eric Blake
cae95eae62 qapi: Drop redundant returns-int test
qapi-schema-test was already testing that we could have a
command returning int, but burned a command name in the whitelist.
Merge the redundant positive test returns-int, and pick a name
that reduces the whitelist size.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444710158-8723-6-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-10-15 08:39:07 +02:00
Eric Blake
625b251c69 qapi: Move empty-enum to compile-time test
Rather than just asserting that we can parse an empty enum,
let's also make sure we can compile it, by including it in
qapi-schema-test.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444710158-8723-5-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-10-15 08:39:07 +02:00
Eric Blake
baabb84c5b qapi: Drop redundant alternate-good test
The alternate-good.json test was already covered by
qapi-schema-test.json.  As future commits will be tweaking
how alternates are laid out, removing the duplicate test now
reduces churn.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444710158-8723-4-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-10-15 08:39:07 +02:00
Eric Blake
7618b91ff8 qapi: Prepare for errors during check()
The next few patches will start migrating error checking from
ad hoc parse methods into the QAPISchema*.check() methods.  But
for an error message to display, we first have to fix the
overall 'try' to catch those errors.  We also want to enable a
few more assertions, such as making sure every attempt to
raise a semantic error is passed a valid location info, or that
various preconditions hold.

The general approach for moving error checking will then be to
relax an assertion into an if that raises an exception if the
condition does not hold, and removing the counterpart ad hoc
check done during the parse phase.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444710158-8723-3-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-10-15 08:39:07 +02:00
Eric Blake
25a0d9c977 qapi: Use predicate callback to determine visit filtering
Previously, qapi-types and qapi-visit filtered out implicit
objects during visit_object_type() by using 'info' (works since
implicit objects do not [yet] have associated info); meanwhile
qapi-introspect filtered out all schema types on the first pass
by returning a python type from visit_begin(), which was then
used at a distance in QAPISchema.visit() to do the filtering.

Rather than keeping these ad hoc approaches, add a new visitor
callback visit_needed() which returns False to skip a given
entity, and which defaults to True unless overridden.  Use the
new mechanism to simplify all three filtering visitors.

No change to the generated code.

Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444710158-8723-2-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-10-15 08:39:07 +02:00
Eric Blake
d08ac81a45 qapi: Fix regression with '-netdev help'
Commit e36c714e causes 'qemu -netdev help' to dump core, because the
call to visit_end_union() is no longer conditional on whether *obj was
allocated.

Reported by Marc-André Lureau <marcandre.lureau@gmail.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444861825-19256-1-git-send-email-eblake@redhat.com>
[Commit message tweaked to say 'help' instead of '?']
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-10-15 08:37:11 +02:00
Peter Maydell
c49d3411fa QAPI patches
-----BEGIN PGP SIGNATURE-----
 Version: GnuPG v1
 
 iQIcBAABAgAGBQJWG/RTAAoJEDhwtADrkYZTMDEP/j0nndNx1oaQnJ0o0reEDqnw
 b2bUPUOBQ8UqgjFq1o3PXhcaOimx6QA5/VwwJwyW9P2AhjG5MUrRl7/E85bF4lQg
 SpuTU9Hd3PKBE/lMFyhTFVJ8X1QknZihLppsIG6TBv0M4bq7OMAUk0nt6PrHF7o1
 69WVGKrH/jVqhRFJwatXAvefjaUiOsAytVMTQpaUN56KcBbbS+Km4p5+5dW3DIr7
 FHC0z5BgnTl6zpPB61D8iktyc+YDIyl+XdP0U3VO03YKw384khpyl5JnnrhcUNE/
 1sfWG/8yoyFuZv8dK6SkWO4VCM2dayywp3Rb53PYjB18cRtJVyjfiwm/u+0r5JUv
 tUyhR6s+BsJ/PmKffOo+F+iBPg0I1rAcN/fFDbbRdvhrr/IxVneXJJbc1DQ3LupL
 pmM+pNuBlyQ3Ug+nlNJZ5hFmXNyDNcawA3Exy52pEI3gvli3tn5MrIpijxdzXAOk
 ah71uEc0BGtGEYnFMbFhdA8yrg0ddZaaa+hZndgkJESgIREwvRnjjhla3fENqyKl
 nHYE4R2IgTvby9VMi8guJCNo8F8NUVQf4AIAIfTwsHQe0U7RPneN5HOZveqX27Mv
 af/YBXF+KYMe27U388exutHBmuDz9EWIKtnaErT63Gza+Pa0TjL9srE2Ft8wMMvb
 kHTRdK6FFQzH+VX9RuZ7
 =4lX/
 -----END PGP SIGNATURE-----

Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2015-10-12' into staging

QAPI patches

# gpg: Signature made Mon 12 Oct 2015 18:56:35 BST using RSA key ID EB918653
# gpg: Good signature from "Markus Armbruster <armbru@redhat.com>"
# gpg:                 aka "Markus Armbruster <armbru@pond.sub.org>"

* remotes/armbru/tags/pull-qapi-2015-10-12:
  qapi: Simplify gen_visit_fields() error handling
  qapi: Share gen_visit_fields()
  qapi: Share gen_err_check()
  qapi: Consistent generated code: minimize push_indent() usage
  qapi: Consistent generated code: prefer common indentation
  qapi: Consistent generated code: prefer common labels
  qapi: Consistent generated code: prefer visitor 'v'
  qapi: Consistent generated code: prefer error 'err'
  qapi: Reuse code for flat union base validation
  qapi: Test use of 'number' within alternates
  qapi: Add tests for empty unions
  qapi: Avoid assertion failure on union 'type' collision
  qapi: Test for various name collisions
  qapi: Clean up qapi.py per pep8
  qapi: Invoke exception superclass initializer
  qapi: Improve 'include' error message
  qapi: Sort qapi-schema tests
  MAINTAINERS: Specify QAPI include and test files
  MAINTAINERS: Specify QObject include and test files
  docs: Move files from docs/qmp/ to docs/

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2015-10-13 10:42:06 +01:00
Eric Blake
18bdbc3ac8 qapi: Simplify gen_visit_fields() error handling
Since we have consolidated all generated code to use 'err' as
the name of the local variable for error detection, we can
simplify the decision on whether to skip error detection (useful
for deallocation paths) to be a boolean.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-18-git-send-email-eblake@redhat.com>
[Change to gen_visit_fields() simplified]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-10-12 18:46:50 +02:00
Eric Blake
82ca8e4696 qapi: Share gen_visit_fields()
Consolidate the code between visit, command marshalling, and
event generation that iterates over the members of a struct.
It reduces code duplication in the generator, so that a future
patch can reduce the size of generated code while touching only
one instead of three locations.

There are no changes to the generated marshal code.

The visitor code becomes slightly more verbose, but remains
semantically equivalent, and is actually easier to read as
it follows a more common idiom:

|     visit_optional(v, &(*obj)->has_device, "device", &err);
|-    if (!err && (*obj)->has_device) {
|-        visit_type_str(v, &(*obj)->device, "device", &err);
|-    }
|     if (err) {
|         goto out;
|     }
|+    if ((*obj)->has_device) {
|+        visit_type_str(v, &(*obj)->device, "device", &err);
|+        if (err) {
|+            goto out;
|+        }
|+    }

The event code becomes slightly more verbose, but this is
arguably a bug fix: although the visitors are not well
documented, use of an optional member should not be attempted
unless guarded by a prior call to visit_optional().  Works only
because the output qmp visitor has a no-op visit_optional():

|+    visit_optional(v, &has_offset, "offset", &err);
|+    if (err) {
|+        goto out;
|+    }
|     if (has_offset) {
|         visit_type_int(v, &offset, "offset", &err);

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-17-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-10-12 18:46:50 +02:00
Eric Blake
1f35334489 qapi: Share gen_err_check()
qapi-commands has a nice helper gen_err_check(), but did not
use it everywhere. In fact, using it in more places makes it
easier to reduce the lines of code used for generating error
checks.  This in turn will make it easier for later patches
to consolidate another common pattern among the generators.

The generated code has fewer blank lines in qapi-event.c functions,
but has no semantic difference.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-16-git-send-email-eblake@redhat.com>
[Drop another blank line for symmetry]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-10-12 18:46:50 +02:00
Eric Blake
05372f708a qapi: Consistent generated code: minimize push_indent() usage
We had some pointless differences in the generated code for visit,
command marshalling, and events; unifying them makes it easier for
future patches to consolidate to common helper functions.
This is one patch of a series to clean up these differences.

This patch reduces the number of push_indent()/pop_indent() pairs
so that generated code is typically already at its natural output
indentation in the python files.  It is easier to reason about
generated code if the reader does not have to track how much
spacing will be inserted alongside the code, and moreso when all
of the generators use the same patterns (qapi-type and qapi-event
were already using in-place indentation).

Arguably, the resulting python may be a bit harder to read with C
code at the same indentation as python; on the other hand, not
having to think about push_indent() is a win, and most decent
editors provide syntax highlighting that makes it easier to
visually distinguish python code from string literals that will
become C code.

There is no change to the generated output.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-15-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-10-12 18:46:49 +02:00
Eric Blake
e36c714e6a qapi: Consistent generated code: prefer common indentation
We had some pointless differences in the generated code for visit,
command marshalling, and events; unifying them makes it easier for
future patches to consolidate to common helper functions.
This is one patch of a series to clean up these differences.

This patch adjusts gen_visit_union() to use the same indentation
as other functions, namely, by jumping early to the error label
if the object was not set rather than placing the rest of the
body inside an if for when it is set.

No change in semantics to the generated code.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-14-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-10-12 18:46:49 +02:00
Eric Blake
f782399cb4 qapi: Consistent generated code: prefer common labels
We had some pointless differences in the generated code for visit,
command marshalling, and events; unifying them makes it easier for
future patches to consolidate to common helper functions.
This is one patch of a series to clean up these differences.

This patch names the goto labels 'out' (not 'clean') and 'out_obj'
(not 'out_end').  Additionally, the generator was inconsistent on
whether labels had a leading space [our HACKING is silent; while
emacs 'gnu' style adds the space to avoid littering column 1].
For minimal churn, prefer no leading space; this also matches
the style that is more prevalent in current qemu.git.

No change in semantics to the generated code.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-13-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-10-12 18:46:49 +02:00
Eric Blake
f8b7f1a8ea qapi: Consistent generated code: prefer visitor 'v'
We had some pointless differences in the generated code for visit,
command marshalling, and events; unifying them makes it easier for
future patches to consolidate to common helper functions.
This is one patch of a series to clean up these differences.

This patch names the local visitor variable 'v' rather than 'm'.
Related objects, such as 'QapiDeallocVisitor', are also named by
their initials instead of an unrelated leading m.

No change in semantics to the generated code.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-12-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-10-12 18:46:49 +02:00
Eric Blake
2a0f50e8d9 qapi: Consistent generated code: prefer error 'err'
We had some pointless differences in the generated code for visit,
command marshalling, and events; unifying them makes it easier for
future patches to consolidate to common helper functions.
This is one patch of a series to clean up these differences.

This patch consistently names the local error variable 'err' rather
than 'local_err'.

No change in semantics to the generated code.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-11-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-10-12 18:46:49 +02:00
Eric Blake
376863ef48 qapi: Reuse code for flat union base validation
Rather than open-code the check for a valid base type, we
should reuse the common functionality. This allows for
consistent error messages, and also makes it easier for a
later patch to turn on support for inline anonymous base
structures.

Test flat-union-inline is updated to test only one feature
(anonymous branch dictionaries), which can be implemented
independently (test flat-union-bad-base already covers the
idea of an anonymous base dictionary).

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-10-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-10-12 18:46:49 +02:00
Eric Blake
9c51b44129 qapi: Test use of 'number' within alternates
Add some testsuite exposure for use of a 'number' as part of
an alternate.  The current state of the tree has a few bugs
exposed by this: our input parser depends on the ordering of
how the qapi schema declared the alternate, and the parser
does not accept integers for a 'number' in an alternate even
though it does for numbers outside of an alternate.

Mixing 'int' and 'number' in the same alternate is unusual,
since both are supplied by json-numbers, but there does not
seem to be a technical reason to forbid it given that our
json lexer distinguishes between json-numbers that can be
represented as an int vs. those that cannot.

Improve the existing test_visitor_in_alternate() to match the
style of the new test_visitor_in_alternate_number(), and to
ensure full coverage of all possible qtype parsing.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-9-git-send-email-eblake@redhat.com>
[Eric's follow-up fixes squashed in]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-10-12 18:46:13 +02:00
Eric Blake
8d25dd101f qapi: Add tests for empty unions
The documentation claims that alternates are useful for
allowing two or more types, although nothing enforces this.
Meanwhile, it is silent on whether empty unions are allowed.
In practice, the generated code will compile, in part because
we have a 'void *data' branch; but attempting to visit such a
type will cause an abort().  While there's no technical reason
that a degenerate union could not be made to work, it's harder
to justify the time spent in chasing known (the current
abort() during visit) and unknown corner cases, than it would
be to just outlaw them.  A future patch will probably take the
approach of forbidding them; in the meantime, we can at least
add testsuite coverage to make it obvious where things stand.

In addition to adding tests to expose the problems, we also
need to adjust existing tests that are meant to test something
else, but which could fail for the wrong reason if we reject
degenerate alternates/unions.

Note that empty structs are explicitly supported (for example,
right now they are the only way to specify that one branch of a
flat union adds no additional members), and empty enums are
covered by the testsuite as working (even if they do not seem
to have much use).

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-8-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-10-12 18:44:54 +02:00
Eric Blake
7b2a5c2f9a qapi: Avoid assertion failure on union 'type' collision
The previous commit added two tests that triggered an assertion
failure. It's fairly straightforward to avoid the failure by
just outright forbidding the collision between a union's tag
values and its discriminator name (including the implicit name
'kind' supplied for simple unions [*]).  Ultimately, we'd like
to move the collision detection into QAPISchema*.check(), but
for now it is easier just to enhance the existing checks.

[*] Of course, down the road, we have plans to rename the simple
union tag name to 'type' to match the QMP wire name, but the
idea of the collision will still be present even then.

Technically, we could avoid the collision by naming the C union
members representing each enum value as '_case_value' rather
than 'value'; but until we have an actual qapi client (and not
just our testsuite) that has a legitimate reason to match a
case label to the name of a QMP key and needs the name munging
to satisfy the compiler, it's easier to just reject the qapi
as invalid.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-7-git-send-email-eblake@redhat.com>
[Polished a few comments]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-10-12 18:44:54 +02:00
Eric Blake
d220fbcd1d qapi: Test for various name collisions
Expose some weaknesses in the generator: we don't always forbid
the generation of structs that contain multiple members that map
to the same C or QMP name.  This has already been marked FIXME in
qapi.py in commit d90675f, but having more tests will make sure
future patches produce desired behavior; and updating existing
patches to better document things doesn't hurt, either.  Some of
these collisions are already caught in the old-style parser
checks, but ultimately we want all collisions to be caught in the
new-style QAPISchema*.check() methods.

This patch focuses on C struct members, and does not consider
collisions between commands and events (affecting C function
names), or even collisions between generated C type names with
user type names (for things like automatic FOOList struct
representing array types or FOOKind for an implicit enum).

There are two types of struct collisions we want to catch:
 1) Collision between two keys in a JSON object. qapi.py prevents
    that within a single struct (see test duplicate-key), but it is
    possible to have collisions between a type's members and its
    base type's members (existing tests struct-base-clash,
    struct-base-clash-deep), and its flat union variant members
    (renamed test flat-union-clash-member).
 2) Collision between two members of the C struct that is generated
    for a given QAPI type:
    a) Multiple QAPI names map to the same C name (new test
       args-name-clash)
    b) A QAPI name maps to a C name that is used for another purpose
       (new tests flat-union-clash-branch, struct-base-clash-base,
       union-clash-data). We already fixed some such cases in commit
       0f61af3e and 1e6c1616, but more remain.
    c) Two C names generated for other purposes clash
       (updated test alternate-clash, new test union-clash-branches,
       union-clash-type, flat-union-clash-type)

Ultimately, if we need to have a flat union where a tag value
clashes with a base member name, we could change the generator to
name the union (using 'foo.u.value' rather than 'foo.value') or
otherwise munge the C name corresponding to tag values.  But
unless such a need arises, it will probably be easier to just
forbid these collisions.

Some of these negative tests will be deleted later, and positive
tests added to qapi-schema-test.json in their place, when the
generator code is reworked to avoid particular code generation
collisions in class 2).

[Note that viewing this patch with git rename detection enabled
may see some confusion due to renaming some tests while adding
others, but where the content is similar enough that git picks
the wrong pre- and post-patch files to associate]

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-6-git-send-email-eblake@redhat.com>
[Improve commit message and comments a bit, drop an unrelated test]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-10-12 18:44:54 +02:00
Eric Blake
437db2549b qapi: Clean up qapi.py per pep8
Silence pep8, and make pylint a bit happier.  Just style cleanups,
plus killing a useless comment in camel_to_upper(); no semantic
changes.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-5-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-10-12 18:44:54 +02:00
Eric Blake
59b0054265 qapi: Invoke exception superclass initializer
pylint recommends that every exception class should explicitly
invoke the superclass __init__, even though things seem to work
fine without it.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-4-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-10-12 18:44:54 +02:00
Eric Blake
7408fb67c0 qapi: Improve 'include' error message
Use of '"...%s" % include' to print non-strings can lead to
ugly messages, such as this (if the .json change is applied
without the qapi.py change):
 Expected a file name (string), got: OrderedDict()

Better is to just omit the actual non-string value in the
message.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-3-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-10-12 18:44:54 +02:00
Eric Blake
1ffe818a39 qapi: Sort qapi-schema tests
Recent changes to qapi have provided quite a bit of churn in
the makefile, because we are inconsistent on what order test
names appear in, and on whether to re-wrap the list of tests or
just add arbitrary line lengths.  Writing the list in a sorted
fashion, one test per line, will make future patches easier
to see what tests are being added or removed by a patch.

Although it is tempting to use $(wildcard qapi-schema/*.json)
for a more compact listing, such an approach would risk picking
up leftover garbage .json files in the directory; so keeping
the list explicit is safer for ensuring reproducible tarballs
and test results.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443565276-4535-2-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-10-12 18:44:54 +02:00
Markus Armbruster
ac4abb9aeb MAINTAINERS: Specify QAPI include and test files
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1443111117-29831-4-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
2015-10-12 18:44:53 +02:00
Markus Armbruster
7735d2b504 MAINTAINERS: Specify QObject include and test files
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1443111117-29831-3-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
2015-10-12 18:44:53 +02:00
Markus Armbruster
9b89b6a287 docs: Move files from docs/qmp/ to docs/
Giving QMP its own subdirectory in docs/ is hardly worthwhile when we
have just four files, and one of them isn't even in the subdirectory.
Move the files from docs/qmp/ to docs/, renaming docs/qmp/README to
docs/qmp-intro.

Update MAINTAINERS.  The new pattern also captures the fourth file
docs/writing-qmp-commands.txt.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1443111117-29831-2-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
2015-10-12 18:44:53 +02:00
Peter Maydell
5451316ed0 Pull request
v2:
  * Fix virtio 16lx -> HWADDR_PRIx format specifier [Peter]
 -----BEGIN PGP SIGNATURE-----
 Version: GnuPG v1
 
 iQEcBAABAgAGBQJWG4kaAAoJEJykq7OBq3PI+EoIALrMej8cpS8a1ByGcnxoXi0k
 SJlhpUC6M9F1YyY6Vx/juRcTibBuE4RNrWirdVXOkINwyHrOsGRd8p8yLWCdUO9G
 3Yznu9fpHcoPRojcKh5m6QhZzm1HUqjO6GoaZMOCpGdrwhUo8ThwBnXR0ffCF98v
 Is0kTkoUVSK5IVHBamzb/myVKSWb6uOOFO6Pc7Kvivlui7cNMUGom2pS6FJD4eEG
 K0+EUoKBGYYwKhZS6IBUDPGFN9takrqkzZODiheDrVegJTN64gUvzMJOgU5P/F7H
 jhRFio8LqxQC3uhbYDWyMS4PVWbm7RjfYu/ph9K3+/s3egwLs+VsNkv4lsf8x+Q=
 =i0do
 -----END PGP SIGNATURE-----

Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging

Pull request

v2:
 * Fix virtio 16lx -> HWADDR_PRIx format specifier [Peter]

# gpg: Signature made Mon 12 Oct 2015 11:19:06 BST using RSA key ID 81AB73C8
# gpg: Good signature from "Stefan Hajnoczi <stefanha@redhat.com>"
# gpg:                 aka "Stefan Hajnoczi <stefanha@gmail.com>"

* remotes/stefanha/tags/block-pull-request:
  sdhci.c: Limit the maximum block size
  block: switch from g_slice allocator to malloc
  virtio dataplane: adapt dataplane for virtio Version 1
  virtio-blk: use blk_io_plug/unplug for Linux AIO batching
  sdhci: Pass drive parameter to sdhci-pci via qdev property

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2015-10-12 15:52:54 +01:00
Peter Maydell
0bf224d5da -----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
 
 iQEcBAABAgAGBQJWG2e/AAoJEO8Ells5jWIRcYcH/2D11W8cToCBjGDuw/u9K1ht
 S3oGyFasOEq3lm3+a3zQE+vDw0RDkjLEMhcTVwNskJQl6k6Ts5JleTZ6wffvUKPM
 UCozgPOCt1ZAdGskwdbByc+NhaVBHIiEsmlbDKqP22CENdDx6GWjcFW4brA4tQJQ
 AW36EH77j/M+7/KiSukcUfIexILUZJRfN+ICJVyNTpGsqUNJtFqiVPBMPyJhKCEq
 3pr3yJ2lf78SAEF5kBeBc9r/PDWUhtqExBsrK0L8Ey1FdrCy8ldqDPGecT4TsxNv
 W/KX5AqhKSsMI8DQKdbv/IKaUdjYWNjTRQ2Qjm8Vt0hcW0PhxR0NYi6bV4yjDNM=
 =f26Q
 -----END PGP SIGNATURE-----

Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into staging

# gpg: Signature made Mon 12 Oct 2015 08:56:47 BST using RSA key ID 398D6211
# gpg: Good signature from "Jason Wang (Jason Wang on RedHat) <jasowang@redhat.com>"
# gpg: WARNING: This key is not certified with sufficiently trusted signatures!
# gpg:          It is not certain that the signature belongs to the owner.
# Primary key fingerprint: 215D 46F4 8246 689E C77F  3562 EF04 965B 398D 6211

* remotes/jasowang/tags/net-pull-request:
  tests: add test cases for netfilter object
  netfilter: add a netbuffer filter
  net/queue: export qemu_net_queue_append_iov
  netfilter: print filter info associate with the netdev
  netfilter: add an API to pass the packet to next filter
  net/queue: introduce NetQueueDeliverFunc
  net: merge qemu_deliver_packet and qemu_deliver_packet_iov
  netfilter: hook packets before net queue send
  init/cleanup of netfilter object
  vl.c: init delayed object after net_init_clients
  vmxnet3: Add support for VMXNET3_CMD_GET_ADAPTIVE_RING_INFO command
  e1000: use alias for default model
  vmxnet3: Support reading IMR registers on bar0
  net/vmxnet3: Refine l2 header validation

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2015-10-12 14:29:29 +01:00
Alistair Francis
9201bb9a8c sdhci.c: Limit the maximum block size
It is possible for the guest to set an invalid block
size which is larger then the fifo_buffer[] array. This
could cause a buffer overflow.

To avoid this limit the maximum size of the blksize variable.

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
Reported-by: Intel Security ATR <secure@intel.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
Message-id: abe4c51f513290bbb85d1ee271cb1a3d463d7561.1444067470.git.alistair.francis@xilinx.com
Suggested-by: Igor Mitsyanko <i.mitsyanko@gmail.com>
Reported-by: Intel Security ATR <secure@intel.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2015-10-12 11:17:45 +01:00
Paolo Bonzini
c84b31926f block: switch from g_slice allocator to malloc
Simplify memory allocation by sticking with a single API.  GSlice
is not that fast anyway (tcmalloc/jemalloc are better).

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2015-10-12 11:17:45 +01:00
Pierre Morel
a9718ef000 virtio dataplane: adapt dataplane for virtio Version 1
Let dataplane allocate different region for the desc/avail/used
ring regions.
Take VIRTIO_RING_F_EVENT_IDX into account to increase the used/avail
rings accordingly.

[Fix 32-bit builds by changing 16lx format specifier to HWADDR_PRIx.
--Stefan]

Signed-off-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
Tested-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
Message-id: 1441625636-23773-1-git-send-email-pmorel@linux.vnet.ibm.com
(changed __virtio16 into uint16_t,
 map descriptor table and available ring read-only)
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2015-10-12 11:16:45 +01:00
Peter Maydell
7684922390 Fix device introspection regressions
-----BEGIN PGP SIGNATURE-----
 Version: GnuPG v1
 
 iQIcBAABAgAGBQJWF8SNAAoJEDhwtADrkYZTSaUQAJdIB2CiO6COT0T36ZNU/yi8
 wkFdmBt6Ejks1v8OBhoz+NEwMomj55uyeG+ck4BfvNAZJqpYPSKeZdviu/U/UDHw
 BJmUBk5beMcTay66DhQ+0bASfn+3nldM6vXExZEsxi43dLAxyksOd6WZ7L3LieDI
 V7mXJ02y/E8tcwuQqiAutU4N/6JlF2PIJpPgDZaPEJIFbW/LBBUoFneYmK4Mv8sZ
 SMjgEulE4JZ/WDeZJYxwWHmNFzrUgTwikq2ydPxawBK6zkoC3JdlkjHdOKWhkQfV
 tMbnRjHmlN9uO3zoF/aIZgFVy4pbnb+f4bnWPqLG55U5z8WXmXao8LlThlzkRdjw
 xrqW8yvwMk1owRLlhjxVR4iyxTXPnp/S76QlEVJbokGFW/MhWhctQoIH0C2lfcgG
 t0A642Xy6NXy+vZqJLJxzigBGn8zT98BXfKptWLHr9aun2U5Vpou0ylf5nsiojL+
 5HR+bB4J/ZVbxIW1OIJqR/cnSKzZqSLCqBB3jnCP60dIgaD7/JeUSz91AFZDPRcb
 PmGsSCa1rjPC66R/QmxYwZ3YGuz0imKWDALqkudsNJnklBVMLxfC+Gp5Sg4D475K
 qzwgRAcXv9wsSPumwoxaQBZxHd4Xuj91e7QmMO+z1un6oOb07fh2Wt5Ib/UoqYM7
 +Ry+yGwO5sRkwZhxdaxM
 =QXLe
 -----END PGP SIGNATURE-----

Merge remote-tracking branch 'remotes/armbru/tags/pull-monitor-2015-10-09' into staging

Fix device introspection regressions

# gpg: Signature made Fri 09 Oct 2015 14:43:41 BST using RSA key ID EB918653
# gpg: Good signature from "Markus Armbruster <armbru@redhat.com>"
# gpg:                 aka "Markus Armbruster <armbru@pond.sub.org>"

* remotes/armbru/tags/pull-monitor-2015-10-09:
  Revert "qdev: Use qdev_get_device_class() for -device <type>,help"
  qdev: Protect device-list-properties against broken devices
  qmp: Fix device-list-properties not to crash for abstract device
  device-introspect-test: New, covering device introspection
  libqtest: New hmp() & friends
  libqtest: Clean up unused QTestState member sigact_old
  tests: Fix how qom-test is run
  macio: move DBDMA_init from instance_init to realize
  hw: do not pass NULL to memory_region_init from instance_init
  memory: allow destroying a non-empty MemoryRegion
  virtio-input: Fix device introspection on non-Linux hosts
  update-linux-headers: Rename SW_MAX to SW_MAX_

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2015-10-12 11:07:38 +01:00
Stefan Hajnoczi
fc73548e44 virtio-blk: use blk_io_plug/unplug for Linux AIO batching
The raw-posix block driver implements Linux AIO batching so multiple
requests can be submitted with a single io_submit(2) system call.
Batching is currently only used by virtio-scsi and
virtio-blk-data-plane.

Enable batching for regular virtio-blk so the number of io_submit(2)
system calls is reduced for workloads with queue depth > 1.

In 4KB random read performance tests with queue depth 32, the CPU
utilization on the host is reduced by 9.4%.  The fio job is as follows:

  [global]
  bs=4k
  ioengine=libaio
  iodepth=32
  direct=1
  sync=0
  time_based=1
  runtime=30
  clocksource=gettimeofday
  ramp_time=5

  [job1]
  rw=randread
  filename=/dev/vdb
  size=4096M
  write_bw_log=fio
  write_iops_log=fio
  write_lat_log=fio
  log_avg_msec=1000

This benchmark was run on an raw image on LVM.  The disk was an SSD
drive and -drive cache=none,aio=native was used.

Tested-by: Pradeep Surisetty <psuriset@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
2015-10-12 09:21:10 +01:00
Kevin O'Connor
5ec911c30f sdhci: Pass drive parameter to sdhci-pci via qdev property
Commit 19109131 disabled the sdhci-pci support because it used
drive_get_next().  This patch reenables sdhci-pci and changes it to
pass the drive via a qdev property - for example:
 -device sdhci-pci,drive=drive0 -drive id=drive0,if=sd,file=myimage

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2015-10-12 09:21:10 +01:00
Yang Hongyang
89b1273742 tests: add test cases for netfilter object
Using qtest qmp interface to implement following cases:
1) add/remove netfilter
2) add a netfilter then delete the netdev
3) add/remove more than one netfilters
4) add more than one netfilters and then delete the netdev

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
2015-10-12 13:34:32 +08:00
Yang Hongyang
7dbb11c84f netfilter: add a netbuffer filter
This filter is to buffer/release packets. Can be used when using
MicroCheckpointing or other Remus like VM FT solutions.
You can also use it to crudely simulate network delay.  Doesn't
actually delay individual packets, but batches them together, which is
a delay of sorts.

Usage:
 -netdev tap,id=bn0
 -object filter-buffer,id=f0,netdev=bn0,queue=rx,interval=1000

NOTE:
 Interval is in microseconds, it can't be omitted currently, and can't be 0.

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
2015-10-12 13:31:28 +08:00
Yang Hongyang
b68c7f7692 net/queue: export qemu_net_queue_append_iov
This will be used by buffer filter implementation later to
queue packets.

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
2015-10-12 13:31:18 +08:00
Yang Hongyang
a4960f52e7 netfilter: print filter info associate with the netdev
When execute "info network", print filter info also.
add a info_str member to NetFilterState, store specific filters
info.

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
2015-10-12 13:30:56 +08:00
Yang Hongyang
7ef7bc8586 netfilter: add an API to pass the packet to next filter
add an API qemu_netfilter_pass_to_next() to pass the packet
to next filter.

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
2015-10-12 13:30:45 +08:00
Yang Hongyang
3e033a46a7 net/queue: introduce NetQueueDeliverFunc
net/queue.c has logic to send/queue/flush packets but a
qemu_deliver_packet_iov() call is hardcoded. Abstract this
func so that we can use our own deliver function in netfilter.

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
2015-10-12 13:30:07 +08:00
Yang Hongyang
fefe2a78ab net: merge qemu_deliver_packet and qemu_deliver_packet_iov
qemu_deliver_packet_iov already have the compat delivery, we
can drop qemu_deliver_packet.

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
2015-10-12 13:30:04 +08:00
Yang Hongyang
e64c770d1f netfilter: hook packets before net queue send
Capture packets that will be sent.

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
2015-10-12 13:29:39 +08:00
Yang Hongyang
fdccce4596 init/cleanup of netfilter object
Add a netfilter object based on QOM.

A netfilter is attached to a netdev, captures all network packets
that pass through the netdev. When we delete the netdev, we also
delete the netfilter object attached to it, because if the netdev is
removed, the filter which attached to it is useless.

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
2015-10-12 13:23:40 +08:00