We started moving away from the use of the 'void *data' member
in the C union corresponding to a QAPI union back in commit
544a373; recent commits have gotten rid of other uses. Now
that it is completely unused, we can remove the member itself
as well as the FIXME comment. Update the testsuite to drop the
negative test union-clash-data.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <1457021813-10704-11-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Dan Berrange reported a case where he needs to work with a
QCryptoBlockOptions union type using the OptsVisitor, but only
visit one of the branches of that type (the discriminator is not
visited directly, but learned externally). When things were
boxed, it was easy: just visit the variant directly, which took
care of both allocating the variant and visiting its members, then
store that pointer in the union type. But now that things are
unboxed, we need a way to visit the members without allocation,
done by exposing visit_type_FOO_members() to the user.
Before the patch, we had quite a bit of code associated with
object_members_seen to make sure that a declaration of the helper
was in scope before any use of the function. But now that the
helper is public and declared in the header, the .c file no
longer needs to worry about topological sorting (the helper is
always in scope), which leads to some nice cleanups.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1457021813-10704-4-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
C types and JSON objects don't have fields, but members. We
shouldn't gratuitously invent terminology. This patch is a
strict renaming of static genarated functions, plus the naming
of the dummy filler member for empty structs, before the next
patch exposes some of that naming to the rest of the code base.
Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1457021813-10704-3-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
C types and JSON objects don't have fields, but members. We
shouldn't gratuitously invent terminology. This patch is a
strict renaming of generator code internals (including testsuite
comments), before later patches rename C interfaces.
No change to generated code with this patch.
Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1457021813-10704-2-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Pretty printing of JSON responses is important to be able to understand
large responses from query commands in particular. Unfortunately this
was broken during the addition of the verbose flag in
commit 1ceca07e48
Author: John Snow <jsnow@redhat.com>
Date: Wed Apr 29 15:14:04 2015 -0400
scripts: qmp-shell: Add verbose flag
This is because that change turned the python data structure into a
formatted JSON string before the pretty print was given it. So we're
just pretty printing a string, which is a no-op.
The original pretty printer would output python objects.
(QEMU) query-chardev
{ u'return': [ { u'filename': u'vc',
u'frontend-open': False,
u'label': u'parallel0'},
{ u'filename': u'vc',
u'frontend-open': True,
u'label': u'serial0'},
{ u'filename': u'unix:/tmp/qemp,server',
u'frontend-open': True,
u'label': u'compat_monitor0'}]}
This fixes the problem by switching to outputting pretty formatted JSON
text instead. This has the added benefit that the pretty printed output
is now valid JSON text. Due to the way the verbose flag was handled, the
pretty printing now applies to the command sent, as well as its response:
(QEMU) query-chardev
{
"execute": "query-chardev",
"arguments": {}
}
{
"return": [
{
"frontend-open": false,
"label": "parallel0",
"filename": "vc"
},
{
"frontend-open": true,
"label": "serial0",
"filename": "vc"
},
{
"frontend-open": true,
"label": "compat_monitor0",
"filename": "unix:/tmp/qmp,server"
}
]
}
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <1456224706-1591-1-git-send-email-berrange@redhat.com>
Tested-by: Kashyap Chamarthy <kchamart@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
[Bonus fix: multiple -p now work]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Formalizes the existence of the 'event_trans' and 'event_exec' event
attributes, which until now were monkey-patched only when necessary.
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
Message-id: 145640558759.20978.6374959404425591089.stgit@localhost
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
This property identifies events that trace vCPU-specific information.
It adds a "CPUState*" argument to events with the property, identifying
the vCPU raising the event. TCG translation events also have a
"TCGv_env" implicit argument that is later used as the "CPUState*"
argument at execution time.
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
Message-id: 145641861797.30295.6991314023181842105.stgit@localhost
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
The current code forces the use of a chain of ".original" dereferences,
which looks odd.
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
Message-id: 145641858988.30295.7223459456488075843.stgit@localhost
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Lets the user manage event arguments as a list, and simplifies argument
concatenation.
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 145641858432.30295.3069911069472672646.stgit@localhost
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Lowercase them.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
All lowercase, use-dash instead of CamelCase.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Ignore files which have a .inc.c extension -- these are not headers
but they are not standalone C source files either, so we can't make
any automated decisions about what #include directives they should
have.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <1456238983-10160-3-git-send-email-peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <rth@twiddle.net>
When generating the trace/generated-ust.c source file, make sure
it includes osdep.h as its first include.
This fixes compilation with --enable-trace-backends=ust
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 1456240661-15422-1-git-send-email-peter.maydell@linaro.org
Reviewed-by: Eric Blake <eblake@redhat.com>
Add a --all option which will run the script on every C
source and header file in the repository (except for those
in a few directories which contain standalone guest code).
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Enhance clean-includes to handle header files as well as .c source
files. For headers we merely remove all the redundant #include
lines, including any includes of qemu/osdep.h itself.
There is a simple mollyguard on the include file processing to
skip a few key headers like osdep.h itself, to avoid producing
bad patches if the script is run on every file in include/.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
They seem to have snuck in when applying Janosch Frank
<frankja@linux.vnet.ibm.com>'s previous patch.
Signed-off-by: Fam Zheng <famz@redhat.com>
Message-Id: <1455848416-13177-1-git-send-email-famz@redhat.com>
Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Tested-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
After recent changes, the only remaining use of
visit_start_implicit_struct() is for allocating the space needed
when visiting an alternate. Since the term 'implicit struct' is
hard to explain, rename the function to its current usage. While
at it, we can merge the functionality of visit_get_next_type()
into the same function, making it more like visit_start_struct().
Generated code is now slightly smaller:
| {
| Error *err = NULL;
|
|- visit_start_implicit_struct(v, (void**) obj, sizeof(BlockdevRef), &err);
|+ visit_start_alternate(v, name, (GenericAlternate **)obj, sizeof(**obj),
|+ true, &err);
| if (err) {
| goto out;
| }
|- visit_get_next_type(v, name, &(*obj)->type, true, &err);
|- if (err) {
|- goto out_obj;
|- }
| switch ((*obj)->type) {
| case QTYPE_QDICT:
| visit_start_struct(v, name, NULL, 0, &err);
...
| }
|-out_obj:
|- visit_end_implicit_struct(v);
|+ visit_end_alternate(v);
| out:
| error_propagate(errp, err);
| }
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1455778109-6278-16-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
There's no reason to do two malloc's for a flat union; let's just
inline the branch struct directly into the C union branch of the
flat union.
Surprisingly, fewer clients were actually using explicit references
to the branch types in comparison to the number of flat unions
thus modified.
This lets us reduce the hack in qapi-types:gen_variants() added in
the previous patch; we no longer need to distinguish between
alternates and flat unions.
The change to unboxed structs means that u.data (added in commit
cee2dedb) is now coincident with random fields of each branch of
the flat union, whereas beforehand it was only coincident with
pointers (since all branches of a flat union have to be objects).
Note that this was already the case for simple unions - but there
we got lucky. Remember, visit_start_union() blindly returns true
for all visitors except for the dealloc visitor, where it returns
the value !!obj->u.data, and that this result then controls
whether to proceed with the visit to the variant. Pre-patch,
this meant that flat unions were testing whether the boxed pointer
was still NULL, and thereby skipping visit_end_implicit_struct()
and avoiding a NULL dereference if the pointer had not been
allocated. The same was true for simple unions where the current
branch had pointer type, except there we bypassed visit_type_FOO().
But for simple unions where the current branch had scalar type, the
contents of that scalar meant that the decision to call
visit_type_FOO() was data-dependent - the reason we got lucky there
is that visit_type_FOO() for all scalar types in the dealloc visitor
is a no-op (only the pointer variants had anything to free), so it
did not matter whether the dealloc visit was skipped. But with this
patch, we would risk leaking memory if we could skip a call to
visit_type_FOO_fields() based solely on a data-dependent decision.
But notice: in the dealloc visitor, visit_type_FOO() already handles
a NULL obj - it was only the visit_type_implicit_FOO() that was
failing to check for NULL. And now that we have refactored things to
have the branch be part of the parent struct, we no longer have a
separate pointer that can be NULL in the first place. So we can just
delete the call to visit_start_union() altogether, and blindly visit
the branch type; there is no change in behavior except to the dealloc
visitor, where we now unconditionally visit the branch, but where that
visit is now always safe (for a flat union, we can no longer
dereference NULL, and for a simple union, visit_type_FOO() was already
safely handling NULL on pointer types).
Unfortunately, simple unions are not as easy to switch to unboxed
layout; because we are special-casing the hidden implicit type with
a single 'data' member, we really DO need to keep calling another
layer of visit_start_struct(), with a second malloc; although there
are some cleanups planned for simple unions in later patches.
visit_start_union() and gen_visit_implicit_struct() are now unused.
Drop them.
Note that after this patch, the only remaining use of
visit_start_implicit_struct() is for alternate types; the next patch
will do further cleanup based on that fact.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1455778109-6278-14-git-send-email-eblake@redhat.com>
[Dead code deletion squashed in, commit message updated accordingly]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
There's no reason to do two malloc's for an alternate type visiting
a QAPI struct; let's just inline the struct directly as the C union
branch of the struct.
Surprisingly, no clients were actually using the struct member prior
to this patch outside of the testsuite; an earlier patch in the series
added some testsuite coverage to make the effect of this patch more
obvious.
In qapi.py, c_type() gains a new is_unboxed flag to control when we
are emitting a C struct unboxed within the context of an outer
struct (different from our other two modes of usage with no flags
for normal local variable declarations, and with is_param for adding
'const' in a parameter list). I don't know if there is any more
pythonic way of collapsing the two flags into a single parameter,
as we never have a caller setting both flags at once.
Ultimately, we want to also unbox branches for QAPI unions, but as
that touches a lot more client code, it is better as separate
patches. But since unions and alternates share gen_variants(), I
had to hack in a way to test if we are visiting an alternate type
for setting the is_unboxed flag: look for a non-object branch.
This works because alternates have at least two branches, with at
most one object branch, while unions have only object branches.
The hack will go away in a later patch.
The generated code difference to qapi-types.h is relatively small:
| struct BlockdevRef {
| QType type;
| union { /* union tag is @type */
| void *data;
|- BlockdevOptions *definition;
|+ BlockdevOptions definition;
| char *reference;
| } u;
| };
The corresponding spot in qapi-visit.c calls visit_type_FOO(), which
first calls visit_start_struct() to allocate or deallocate the member
and handle a layer of {} from the JSON stream, then visits the
members. To peel off the indirection and the memory management that
comes with it, we inline this call, then suppress allocation /
deallocation by passing NULL to visit_start_struct(), and adjust the
member visit:
| switch ((*obj)->type) {
| case QTYPE_QDICT:
|- visit_type_BlockdevOptions(v, name, &(*obj)->u.definition, &err);
|+ visit_start_struct(v, name, NULL, 0, &err);
|+ if (err) {
|+ break;
|+ }
|+ visit_type_BlockdevOptions_fields(v, &(*obj)->u.definition, &err);
|+ error_propagate(errp, err);
|+ err = NULL;
|+ visit_end_struct(v, &err);
| break;
| case QTYPE_QSTRING:
| visit_type_str(v, name, &(*obj)->u.reference, &err);
The visit of non-object fields is unchanged.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1455778109-6278-13-git-send-email-eblake@redhat.com>
[Commit message tweaked]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
We have several instances of methods that do an early exit if
output is not needed, then log that output is being generated,
and finally produce the output; see qapi-types.py:gen_object()
and qapi-visit.py:gen_visit_implicit_struct(). The odd man
out was gen_visit_fields_decl(); rearrange it to be more like
the others. No semantic change or difference to generated code.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1455778109-6278-12-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Right now, we emit the branches of union types as a boxed pointer,
and it suffices to have a forward declaration of the type. However,
a future patch will swap things to directly use the branch type,
instead of hiding it behind a pointer. For this to work, the
compiler needs the full definition of the type, not just a forward
declaration, prior to the union that is including the branch type.
This patch just adds topological sorting to hoist all types
mentioned in a branch of a union to be fully declared before the
union itself. The sort is always possible, because we do not
allow circular union types that include themselves as a direct
branch (it is, however, still possible to include a branch type
that itself has a pointer to the union, for a type that can
indirectly recursively nest itself - that remains safe, because
that the member of the branch type will remain a pointer, and the
QMP representation of such a type adds another {} for each recurring
layer of the union type).
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1455778109-6278-11-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
By sticking the next pointer first, we don't need a union with
64-bit padding for smaller types. On 32-bit platforms, this
can reduce the size of uint8List from 16 bytes (or 12, depending
on whether 64-bit ints can tolerate 4-byte alignment) down to 8.
It has no effect on 64-bit platforms (where alignment still
dictates a 16-byte struct); but fewer anonymous unions is still
a win in my book.
It requires visit_next_list() to gain a size parameter, to know
what size element to allocate; comparable to the size parameter
of visit_start_struct().
I debated about going one step further, to allow for fewer casts,
by doing:
typedef GenericList GenericList;
struct GenericList {
GenericList *next;
};
struct FooList {
GenericList base;
Foo *value;
};
so that you convert to 'GenericList *' by '&foolist->base', and
back by 'container_of(generic, GenericList, base)' (as opposed to
the existing '(GenericList *)foolist' and '(FooList *)generic').
But doing that would require hoisting the declaration of
GenericList prior to inclusion of qapi-types.h, rather than its
current spot in visitor.h; it also makes iteration a bit more
verbose through 'foolist->base.next' instead of 'foolist->next'.
Note that for lists of objects, the 'value' payload is still
hidden behind a boxed pointer. Someday, it would be nice to do:
struct FooList {
FooList *next;
Foo value;
};
for one less level of malloc for each list element. This patch
is a step in that direction (now that 'next' is no longer at a
fixed non-zero offset within the struct, we can store more than
just a pointer's-worth of data as the value payload), but the
actual conversion would be a task for another series, as it will
touch a lot of code.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1455778109-6278-10-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
We were passing 'Foo **obj' to the internal helper function, but
all uses within the helper were via reads of '*obj'. Refactor
things to pass one less level of indirection, by having the
callers dereference before calling.
For an example of the generated code change:
|-static void visit_type_BalloonInfo_fields(Visitor *v, BalloonInfo **obj, Error **errp)
|+static void visit_type_BalloonInfo_fields(Visitor *v, BalloonInfo *obj, Error **errp)
| {
| Error *err = NULL;
|
|- visit_type_int(v, "actual", &(*obj)->actual, &err);
|+ visit_type_int(v, "actual", &obj->actual, &err);
| error_propagate(errp, err);
| }
|
|@@ -261,7 +261,7 @@ void visit_type_BalloonInfo(Visitor *v,
| if (!*obj) {
| goto out_obj;
| }
|- visit_type_BalloonInfo_fields(v, obj, &err);
|+ visit_type_BalloonInfo_fields(v, *obj, &err);
| out_obj:
The refactoring will also make it easier to reuse the helpers in
a future patch when implicit structs are stored directly in the
parent struct rather than boxed through a pointer.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1455778109-6278-9-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
gen_visit_union() is now just like gen_visit_struct(). Rename
it to gen_visit_object(), use it for structs, and drop
gen_visit_struct(). Output is unchanged.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1453902888-20457-4-git-send-email-armbru@redhat.com>
[split out variant handling, rebase to earlier changes]
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1455778109-6278-8-git-send-email-eblake@redhat.com>
We initially created the static visit_type_FOO_fields() helper
function for reuse of code - we have cases where the initial
setup for a visit has different allocation (depending on whether
the fields represent a stand-alone type or are embedded as part
of a larger type), but where the actual field visits are
identical once a pointer is available.
Up until the previous patch, visit_type_FOO_fields() was only
used for structs (no variants), so it was covering every field
for each type where it was emitted.
Meanwhile, the code for visiting unions looks like:
static visit_type_U_fields() {
visit base;
visit local_members;
}
visit_type_U() {
visit_start_struct();
visit_type_U_fields();
visit variants;
visit_end_struct();
}
which splits the fields of the union visit across two functions.
Move the code to visit variants to live inside visit_type_U_fields(),
while making it conditional on having variants so that all other
instances of the helper function remain unchanged. This is also
a step closer towards unifying struct and union visits, and towards
allowing one union type to be the branch of another flat union.
The resulting diff to the generated code is a bit hard to read,
but it can be verified that it touches only union types, and that
the end result is the following general structure:
static visit_type_U_fields() {
visit base;
visit local_members;
visit variants;
}
visit_type_U() {
visit_start_struct();
visit_type_U_fields();
visit_end_struct();
}
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1455778109-6278-7-git-send-email-eblake@redhat.com>
[gen_visit_struct_fields() parameter variants made mandatory]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
For a simple union SU, gen_visit_union() generates a visit of its
single tag member, like this:
visit_type_SUKind(v, "type", &(*obj)->type, &err);
For a flat union FU with base B, it generates a visit of its base
fields:
visit_type_B_fields(v, (B **)obj, &err);
Instead, we can simply visit the common members using the same fields
visit function we use for structs, generated with
gen_visit_struct_fields(). This function visits the base if any, then
the local members.
For a simple union SU, visit_type_SU_fields() contains exactly the old
tag member visit, because there is no base, and the tag member is the
only member. For instance, the code generated for qapi-schema.json's
KeyValue changes like this:
+static void visit_type_KeyValue_fields(Visitor *v, KeyValue **obj, Error **errp)
+{
+ Error *err = NULL;
+
+ visit_type_KeyValueKind(v, "type", &(*obj)->type, &err);
+ if (err) {
+ goto out;
+ }
+
+out:
+ error_propagate(errp, err);
+}
+
void visit_type_KeyValue(Visitor *v, const char *name, KeyValue **obj, Error **errp)
{
Error *err = NULL;
@@ -4863,7 +4911,7 @@ void visit_type_KeyValue(Visitor *v, con
if (!*obj) {
goto out_obj;
}
- visit_type_KeyValueKind(v, "type", &(*obj)->type, &err);
+ visit_type_KeyValue_fields(v, obj, &err);
if (err) {
goto out_obj;
}
For a flat union FU, visit_type_FU_fields() contains exactly the old
base fields visit, because there is a base, but no members. For
instance, the code generated for qapi-schema.json's CpuInfo changes
like this:
static void visit_type_CpuInfoBase_fields(Visitor *v, CpuInfoBase **obj, Error **errp);
+static void visit_type_CpuInfo_fields(Visitor *v, CpuInfo **obj, Error **errp)
+{
+ Error *err = NULL;
+
+ visit_type_CpuInfoBase_fields(v, (CpuInfoBase **)obj, &err);
+ if (err) {
+ goto out;
+ }
+
+out:
+ error_propagate(errp, err);
+}
+
static void visit_type_CpuInfoX86_fields(Visitor *v, CpuInfoX86 **obj, Error **errp)
...
@@ -3485,7 +3509,7 @@ void visit_type_CpuInfo(Visitor *v, cons
if (!*obj) {
goto out_obj;
}
- visit_type_CpuInfoBase_fields(v, (CpuInfoBase **)obj, &err);
+ visit_type_CpuInfo_fields(v, obj, &err);
if (err) {
goto out_obj;
}
As you see, the generated code grows a bit, but in practice, it's lost
in the noise: qapi-schema.json's qapi-visit.c gains roughly 1%.
This simplification became possible with commit 441cbac "qapi-visit:
Convert to QAPISchemaVisitor, fixing bugs". It's a step towards
unifying gen_struct() and gen_union().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1453902888-20457-2-git-send-email-armbru@redhat.com>
[improve commit message examples]
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1455778109-6278-6-git-send-email-eblake@redhat.com>
[Commit message tweaked]
The whole point of an alternate is to allow some type-safety while
still accepting more than one JSON type. Meanwhile, the 'any'
type exists to bypass type-safety altogether. The two are
incompatible: you can't accept every type, and still tell which
branch of the alternate to use for the parse; fix this to give a
sane error instead of a Python stack trace.
Note that other types that can't be alternate members are caught
earlier, by check_type().
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1455778109-6278-4-git-send-email-eblake@redhat.com>
[Commit message tweaked]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Empty unions serve no purpose, and while we compile with gcc
which permits them, strict C99 forbids them. We happen to inject
a dummy 'void *data' member into the C unions that represent QAPI
unions and alternates, but we want to get rid of that member (it
pollutes the namespace for no good reason), which would leave us
with an empty union if the user didn't provide any branches. While
empty structs make sense in QAPI, empty unions don't add any
expressiveness to the QMP language. So prohibit them at parse
time. Update the documentation and testsuite to match.
Note that the documentation already mentioned that alternates
should have "two or more JSON data types"; so this also fixes
the code to enforce that. However, we have existing uses of a
union type with only one branch, so the 2-or-more strictness
is intentionally limited to alternates.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1455778109-6278-3-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
When we added support for a user-specified prefix for an enum
type (commit 351d36e), we forgot to teach the qapi-visit code
to honor that prefix in the case of using a prefixed enum as
the discriminator for a flat union. While there is still some
on-list debate on whether we want to keep prefixes, we should
at least make it work as long as it is still part of the code
base.
Reported-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1455665965-27638-1-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
* qemu-char fixes from Daniel and Marc-André
* Bug fixes that break qemu-iotests
* Changes to fix reset from panicked state
* checkpatch false positives for designated initializers
* TLS support in the NBD servers and clients
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQEcBAABCAAGBQJWw03lAAoJEL/70l94x66D/0MH/3Nctz5y1GKgAX0i6rKErV3/
hvPt6JHdWd7uBtowzO5kOy3fyOnVVST6jNHMQPAmJplUC40s6Ca0hycw9TjdJUdu
ULq0Ba7tQ1TAXowDqibtEn+iTkzSrocTJLfEglNscKzJ4y5w0vc5Bt5PgPB65mbn
oTo/YR8KyRWS6rXjyNnKb0PCaYEQziBndjuIxp9yJUsLcw1UgQJVcrUNEIiciOWu
SlWDxvJJQt5cCrTPnUXeBdjJVGaLxbcpe2llEJnIuf6Pjq4u7J0y+DJ40y0DCi9q
v3V4r16HhAGmBZNIlCtbClfhb/sRTVhONQiS9ehhROo8QCaL1psc11HWvMJ0tx0=
=CDoZ
-----END PGP SIGNATURE-----
Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging
* Coverity fixes for IPMI and mptsas
* qemu-char fixes from Daniel and Marc-André
* Bug fixes that break qemu-iotests
* Changes to fix reset from panicked state
* checkpatch false positives for designated initializers
* TLS support in the NBD servers and clients
# gpg: Signature made Tue 16 Feb 2016 16:27:17 GMT using RSA key ID 78C7AE83
# gpg: Good signature from "Paolo Bonzini <bonzini@gnu.org>"
# gpg: aka "Paolo Bonzini <pbonzini@redhat.com>"
* remotes/bonzini/tags/for-upstream: (28 commits)
nbd: enable use of TLS with nbd-server-start command
nbd: enable use of TLS with qemu-nbd server
nbd: enable use of TLS with NBD block driver
nbd: implement TLS support in the protocol negotiation
nbd: use "" as a default export name if none provided
nbd: always query export list in fixed new style protocol
nbd: allow setting of an export name for qemu-nbd server
nbd: make client request fixed new style if advertised
nbd: make server compliant with fixed newstyle spec
nbd: invert client logic for negotiating protocol version
nbd: convert to using I/O channels for actual socket I/O
nbd: convert blockdev NBD server to use I/O channels for connection setup
nbd: convert qemu-nbd server to use I/O channels for connection setup
nbd: convert block client to use I/O channels for connection setup
qemu-nbd: add support for --object command line arg
qom: add helpers for UserCreatable object types
ipmi: sensor number should not exceed MAX_SENSORS
mptsas: fix wrong formula
mptsas: fix memory leak
mptsas: add missing va_end
...
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Include qemu/osdep.h as the first include in generated .c files,
so they don't implicitly rely on some other included header
to pull it in.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
In the .c files generated by this script, include qemu/osdep.h
as the first included header, not config.h.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
As a followup to commit cbf2115, clean up the includes in files
generated by QAPI so that osdep.h is included first in .c files,
and headers which it implies are not included manually. This
patch is done manually, since Coccinelle (and therefore
scripts/clean-includes) doesn't see into the generator scripts.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Now, macro definition such as "#define abc(x) [x] = y" should pass
without an error.
Signed-off-by: Leonid Bloch <leonid@daynix.com>
Message-Id: <1446112118-12376-3-git-send-email-leonid@daynix.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Previously, an error was printed in cases such as:
{ [1] = 5, [2] = 6 }
The space passed OK after a curly brace, but not after a comma.
Now, a space before a square bracket is allowed, if a comma comes before
it.
Signed-off-by: Leonid Bloch <leonid@daynix.com>
Message-Id: <1446112118-12376-2-git-send-email-leonid@daynix.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
It's not 100% obvious to project newcomers that all patches should be sent
there; checkpatch doesn't say so, and since it mentions other lists to CC,
the wording "the list" from the SubmitAPatch wiki page can be taken
to mean only those lists, not the main list too. We would like therefore
to add a catch-all entry for qemu-devel@nongnu.org.
On its own, this would break fallback to git, because now every file
has a maintainer of sorts. Modify get_maintainer.pl so that mailing
lists (L: lines) no longer prevent the fallback, only humans (M:
entries).
Several pre-existing entries have a list but no human. These now
fall back to git. That's a feature.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: John Snow <jsnow@redhat.com>
Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>
Message-Id: <1454987065-12961-1-git-send-email-swarren@wwwdotorg.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
On kernels build without CONFIG_TRACING kvm_stat will bail out even
when traces are not used. This is not very helpful, especially if the
user can't install a new kernel. Instead, we should warn the user and
fall back to debugfs statistics.
These changes check if trace statistics were selected without kernel
support, warn with a small timeout, set the debugfs statistics option
to True and the tracefs one to False.
Fixes: 7aa4ee5 ('scripts/kvm/kvm_stat: Improve debugfs access checking')
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1454485291-43849-2-git-send-email-frankja@linux.vnet.ibm.com>
[Exit if -t is passed explicitly. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Commit 86f4b687 broke compilation on MIPS and SPARC, which have a
preprocessor pollution of '#define mips 1' and '#define sparc 1',
respectively. Treat it the same way as we do for the pollution with
'unix', so that QMP remains backwards compatible and only the C code
needs to use the alternative 'q_mips', 'q_sparc' spelling.
CC: James Hogan <james.hogan@imgtec.com>
CC: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Eric Blake <eblake@redhat.com>
Tested-by: James Hogan <james.hogan@imgtec.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
No backend was setting an error when ending the visit of a list or
implicit struct, or when moving to the next list node. Make the
callers a bit easier to follow by making this a part of the contract,
and removing the errp argument - callers can then unconditionally end
an object as part of cleanup without having to think about whether a
second error is dominated by a first, because there is no second
error.
A later patch will then tackle the larger task of splitting
visit_end_struct(), which can indeed set an error.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1454075341-13658-24-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
visit_start_struct() and visit_type_enum() had a 'kind' argument
that was usually set to either the stringized version of the
corresponding qapi type name, or to NULL (although some clients
didn't even get that right). But nothing ever used the argument.
It's even hard to argue that it would be useful in a debugger,
as a stack backtrace also tells which type is being visited.
Therefore, drop the 'kind' argument as dead.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <1454075341-13658-22-git-send-email-eblake@redhat.com>
[Harmless rebase mistake cleaned up]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
JSON uses "name":value, but many of our visitor interfaces were
called with visit_type_FOO(v, &value, name, errp). This can be
a bit confusing to have to mentally swap the parameter order to
match JSON order. It's particularly bad for visit_start_struct(),
where the 'name' parameter is smack in the middle of the
otherwise-related group of 'obj, kind, size' parameters! It's
time to do a global swap of the parameter ordering, so that the
'name' parameter is always immediately after the Visitor argument.
Additional reason in favor of the swap: the existing include/qjson.h
prefers listing 'name' first in json_prop_*(), and I have plans to
unify that file with the qapi visitors; listing 'name' first in
qapi will minimize churn to the (admittedly few) qjson.h clients.
Later patches will then fix docs, object.h, visitor-impl.h, and
those clients to match.
Done by first patching scripts/qapi*.py by hand to make generated
files do what I want, then by running the following Coccinelle
script to affect the rest of the code base:
$ spatch --sp-file script `git grep -l '\bvisit_' -- '**/*.[ch]'`
I then had to apply some touchups (Coccinelle insisted on TAB
indentation in visitor.h, and botched the signature of
visit_type_enum() by rewriting 'const char *const strings[]' to
the syntactically invalid 'const char*const[] strings'). The
movement of parameters is sufficient to provoke compiler errors
if any callers were missed.
// Part 1: Swap declaration order
@@
type TV, TErr, TObj, T1, T2;
identifier OBJ, ARG1, ARG2;
@@
void visit_start_struct
-(TV v, TObj OBJ, T1 ARG1, const char *name, T2 ARG2, TErr errp)
+(TV v, const char *name, TObj OBJ, T1 ARG1, T2 ARG2, TErr errp)
{ ... }
@@
type bool, TV, T1;
identifier ARG1;
@@
bool visit_optional
-(TV v, T1 ARG1, const char *name)
+(TV v, const char *name, T1 ARG1)
{ ... }
@@
type TV, TErr, TObj, T1;
identifier OBJ, ARG1;
@@
void visit_get_next_type
-(TV v, TObj OBJ, T1 ARG1, const char *name, TErr errp)
+(TV v, const char *name, TObj OBJ, T1 ARG1, TErr errp)
{ ... }
@@
type TV, TErr, TObj, T1, T2;
identifier OBJ, ARG1, ARG2;
@@
void visit_type_enum
-(TV v, TObj OBJ, T1 ARG1, T2 ARG2, const char *name, TErr errp)
+(TV v, const char *name, TObj OBJ, T1 ARG1, T2 ARG2, TErr errp)
{ ... }
@@
type TV, TErr, TObj;
identifier OBJ;
identifier VISIT_TYPE =~ "^visit_type_";
@@
void VISIT_TYPE
-(TV v, TObj OBJ, const char *name, TErr errp)
+(TV v, const char *name, TObj OBJ, TErr errp)
{ ... }
// Part 2: swap caller order
@@
expression V, NAME, OBJ, ARG1, ARG2, ERR;
identifier VISIT_TYPE =~ "^visit_type_";
@@
(
-visit_start_struct(V, OBJ, ARG1, NAME, ARG2, ERR)
+visit_start_struct(V, NAME, OBJ, ARG1, ARG2, ERR)
|
-visit_optional(V, ARG1, NAME)
+visit_optional(V, NAME, ARG1)
|
-visit_get_next_type(V, OBJ, ARG1, NAME, ERR)
+visit_get_next_type(V, NAME, OBJ, ARG1, ERR)
|
-visit_type_enum(V, OBJ, ARG1, ARG2, NAME, ERR)
+visit_type_enum(V, NAME, OBJ, ARG1, ARG2, ERR)
|
-VISIT_TYPE(V, OBJ, NAME, ERR)
+VISIT_TYPE(V, NAME, OBJ, ERR)
)
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <1454075341-13658-19-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
C compilers are allowed to represent enums as a smaller type
than int, if all enum values fit in the smaller type. There
are even compiler flags that force the use of this smaller
representation, although using them changes the ABI of a
binary. Therefore, our generated code for visit_type_ENUM()
(for all qapi enums) was wrong for casting Enum* to int* when
calling visit_type_enum().
It appears that no one has been using compiler ABI switches
for qemu, because if they had, we are potentially dereferencing
beyond bounds or even risking a SIGBUS on platforms where
unaligned pointer dereferencing is fatal. But it is still
better to avoid the practice entirely, and just use the correct
types.
This matches the fix for alternate qapi types, done earlier in
commit 0426d53 "qapi: Simplify visiting of alternate types",
with generated code changing as:
| void visit_type_QType(Visitor *v, QType *obj, const char *name, Error **errp)
| {
|- visit_type_enum(v, (int *)obj, QType_lookup, "QType", name, errp);
|+ int value = *obj;
|+ visit_type_enum(v, &value, QType_lookup, "QType", name, errp);
|+ *obj = value;
| }
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <1454075341-13658-17-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
The generated code can call visit_end_union() without having called
visit_start_union(). Example:
if (!*obj) {
goto out_obj;
}
visit_type_CpuInfoBase_fields(v, (CpuInfoBase **)obj, &err);
if (err) {
goto out_obj; // if we go from here...
}
if (!visit_start_union(v, !!(*obj)->u.data, &err) || err) {
goto out_obj;
}
switch ((*obj)->arch) {
[...]
}
out_obj:
// ... then *obj is true, and ...
error_propagate(errp, err);
err = NULL;
if (*obj) {
// we end up here
visit_end_union(v, !!(*obj)->u.data, &err);
}
error_propagate(errp, err);
Harmless only because no visitor implements end_union(). Clean it up
anyway, by deleting the function as useless.
Messed up since we have visit_end_union (commit cee2ded).
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1453902888-20457-3-git-send-email-armbru@redhat.com>
[expand scope of patch to delete rather than repair]
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1454075341-13658-13-git-send-email-eblake@redhat.com>
Inside the generated code between visit_start_struct() and
visit_end_struct(), we were blindly setting the error into
the caller's errp parameter. But a future patch to split
visit_end_struct() will require that we take action based
on whether an error has occurred, which requires us to track
all actions through a local err. Rewrite the visits to be
more in line with the other generated calls.
Generated code changes look like:
| visit_start_struct(v, (void **)obj, "Abort", name, sizeof(Abort), &err);
|- if (!err) {
|- if (*obj) {
|- visit_type_Abort_fields(v, obj, errp);
|- }
|- visit_end_struct(v, &err);
|+ if (err) {
|+ goto out;
| }
|+ if (!*obj) {
|+ goto out_obj;
|+ }
|+ visit_type_Abort_fields(v, obj, &err);
|+ error_propagate(errp, err);
|+ err = NULL;
|+out_obj:
|+ visit_end_struct(v, &err);
|+out:
| error_propagate(errp, err);
| }
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1454075341-13658-12-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
All other successful clients of visit_start_struct() were paired
with an unconditional visit_end_struct(); but the generated
code for events was relying on qmp_output_visitor_cleanup() to
work on an incomplete visit. Alter the code to guarantee that
the struct is completed, which will make a future patch to
split visit_end_struct() easier to reason about. While at it,
drop some assertions and comments that are not present in other
uses of the qmp output visitor, and pass NULL rather than "" as
the 'kind' parameter (matching most other uses where obj is NULL).
The changes to the generated code look like:
| qmp = qmp_event_build_dict("DEVICE_TRAY_MOVED");
|
| qov = qmp_output_visitor_new();
|- g_assert(qov);
|-
| v = qmp_output_get_visitor(qov);
|- g_assert(v);
|
|- /* Fake visit, as if all members are under a structure */
|- visit_start_struct(v, NULL, "", "DEVICE_TRAY_MOVED", 0, &err);
|+ visit_start_struct(v, NULL, NULL, "DEVICE_TRAY_MOVED", 0, &err);
| if (err) {
| goto out;
| }
| visit_type_str(v, (char **)&device, "device", &err);
| if (err) {
|- goto out;
|+ goto out_obj;
| }
| visit_type_bool(v, &tray_open, "tray-open", &err);
| if (err) {
|- goto out;
|+ goto out_obj;
| }
|- visit_end_struct(v, &err);
|+out_obj:
|+ visit_end_struct(v, err ? NULL : &err);
| if (err) {
| goto out;
| }
|
| obj = qmp_output_get_qobject(qov);
|- g_assert(obj != NULL);
|+ g_assert(obj);
|
| qdict_put_obj(qmp, "data", obj);
| emit(QAPI_EVENT_DEVICE_TRAY_MOVED, qmp, &err);
Note that the 'goto out_obj' with no intervening code before the
label, as well as the construct of 'err ? NULL : &err', are both
a bit unusual but also temporary; they get fixed in a later patch
that splits visit_end_struct() to drop its errp parameter by moving
some checking before the label. But until that time, this was the
simplest way to avoid the appearance of passing a possibly-set
error to visit_end_struct(), even though actual code inspection
shows that visit_end_struct() for a QMP output visitor will never
set an error.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1454075341-13658-11-git-send-email-eblake@redhat.com>
[Commit message's code diff tweaked]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Commit 5cdc8831 reworked gen_params() to be simpler, but forgot
to clean up a now-unused errp named argument.
No change to generated code.
Reported-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1454075341-13658-6-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
This reverts commit 662da3854e.
We require Python 2.6 now (commit fec2103).
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <1450425164-24969-4-git-send-email-armbru@redhat.com>
PEP 8 calls for it, because it's forward compatible with Python 3.
Supported since Python 2.6, which we require (commit fec2103).
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <1450425164-24969-3-git-send-email-armbru@redhat.com>
PEP 8 calls for it, because it's forward compatible with Python 3.
Supported since Python 2.6, which we require (commit fec2103).
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <1450425164-24969-2-git-send-email-armbru@redhat.com>
Commit 8304402033 changed the name of the
e1000-82540em device to e1000. This was flagged:
Section "e1000-82540em" does not exist in dest
Add the mapping to the changed section names dictionary so the checker
can proceed.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Message-Id: <7ccfe834c897142dceaa4da87c13b7059fa12aa8.1450416947.git.amit.shah@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
This is more cache friendly on the fast path, where we already have
the event id available.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
The module docstring is changed into a multi-line comment to comply
with pep 257.
The comment about the docstring that gets used by gdb to print the
help is moved to the location of the docstring.
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1453464520-3882-7-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
By modelling the ELF with ctypes we not only gain full python 3
support but can also create dumps for different architectures more easily.
Tested-by: Andrew Jones <drjones@redhat.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1453464520-3882-6-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Increase readability by adding newlines and comments, as well as
removing wrong whitespaces and C style braces around conditionals and
loops.
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1453464520-3882-5-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This commit does not make the script python 3 compatible, it is a
preparation that fixes the easy and common incompatibilities.
Print is a function in python 3 and therefore needs braces around its
arguments.
Range does not cast a gdb.Value object to int in python 3, we have to
do it ourselves.
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1453464520-3882-4-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The functions dealing with qemu components rarely used parts of the
class, so they were moved out of the class.
As the uintptr_t variable is needed both within and outside the class,
it was made a constant and moved to the top.
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1453464520-3882-3-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The constants bloated the class definition and were therefore moved to
the top.
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1453464520-3882-2-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Added a description text that explains what the script does and which
requirements have to be met to let it run.
The help formatter class is needed as the default optparse formatter
makes the text unreadable.
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-35-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Interactively changing the filter is much more useful than the
drilldown, because it is more versatile.
With this patch, the filter can be changed by pressing 'f' in the text
ui and entering a new filter regex.
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-34-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
When filtering, the group leader event should not be disabled, as all
other events under it will also be disabled. Also we should make sure
that values from disabled fields will not be displayed.
This also filters the fields from the log and batch output for better
readability.
Also the drilldown update now directly checks for the stats' field
filter and (un)sets drilldown accordingly.
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-33-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Setting the hard limit as a unprivileged user either returns an error
when it is higher than the current one or irreversibly sets it lower.
Therefore we leave the hardlimit untouched as long as we don't need to
raise it as this needs CAP_SYS_RESOURCE.
This gives admins the possibility to run the script as an unprivileged
user to increase security.
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-32-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The struct read_format, which denotes the returned values on a read
states that the values are u64 and not long long which is used for
struct unpacking.
Therefore the 'q' long long formatter was exchanged with 'Q' which is
the format for u64 data.
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-31-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
All initializations of the ctypes struct that don't need additional
information were moved to its init method. The unneeded
initializations for sample_type and sample_period were removed as they
do not affect the counters that are read.
This improves readability of the setup_event_attribute by halfing its
LOC.
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-30-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The key names in log mode were capped to 10 characters which is not
enough for distinguishing between keys. Capping was therefore removed.
In batch mode the spacing between keys and values was too narrow and
therefore had to be extended to 42.
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-29-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The tui function itself had a few sub-functions and therefore
basically already was class-like. Making it an actual one with proper
methods improved readability.
The curses wrapper was dropped in favour of __entry/exit__ methods
that implement the same behaviour.
Also renamed single character variable name, so the name reflects the
content.
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-28-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The architecture detection method directly accesses vmx and smv exit
reason constants. Therefore we don't need it anymore.
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-27-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Using global variables and multiple initialization functions for arch
specific data makes the code hard to read. By grouping them in the
Arch classes we encapsulate and initialize them in one place.
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-26-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Added additional newlines for readability.
Factored out attribute and event setup code into own methods.
Exchanged file() with preferred open().
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-25-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Introduced separating newlines for readability and removed special
treatment/variable of the group leader. Renamed fmt to read_format.
The group leader's file descriptor will not be turned into a file
object anymore, instead os.read is used to read from the descriptor.
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-24-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Converted class definition to new style and renamed improper named
variables.
Introduced property for fields_filter.
Moved member variable declaration to init, so one can see all class
variables when reading the init method.
Completely clear the values dict, as we don't need to keep single values.
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-23-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The variable was only used in one class but still was defined
globally. Additionaly the detect_platform routine which prepares the
data that goes into the variable was called on each start of the
script, no matter if the class was needed.
To make the variable local to the TracepointProvider class, a new
function that calls detect_platform and returns the filters was
introduced.
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-22-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reading /sys/devices/system/cpu/online makes opening the cpu
directories unnecessary and works on more/older systems.
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-21-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Variables with bad names like f and m were renamed to their full name,
so it is clearer which data they contain.
Unneeded variables were removed and the field generating code was
moved in an own function.
dict.iteritems() was removed as directly iterating over a dictionary
also yields the needed keys.
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-20-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
As previous commit authors used a mixture of setters/getters and
direct access to class variables consolidating them the python way
improved readability.
Properties allow us to assign a value to a class variable through a
setter without the need to call the setter ourselves.
Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-19-git-send-email-frankja@linux.vnet.ibm.com>
[prop.setter is new in Python 2.6, which is the earliest supported
version. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The underscore in front of the function name does not comply with the
python coding guidelines.
Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-18-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The online cpus detection method is in the Stats class but does not
use any class variables.
Moving it out of the class to the platform detection function makes
the Stats class more readable.
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-17-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
s390 machines can also be detected via uname -m, i.e. python's
os.uname, no need for more complicated checks.
Calling uname once and saving its value for multiple checks is
perfectly sufficient. We don't expect the machine's architecture to
change when the script is running anyway.
On multi-cpu systems x86_init currently will get called multiple
times, returning makes sure we don't waste cicles on that.
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-16-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
As num cpus * 1000 is NOT a sensible rlimit, we need to calculate a
more accurate rlimit.
The number of open files is directly dependent on the cpu count and on
the number of trace points per cpu. A additional constant works as a
buffer for files that are needed by python or do get opened when the
script runs.
Hence we have:
cpus * traces + constant
Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-15-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
In 2008 a patch was written that introduced ctypes.get_errno() and
set_errno() as official interfaces to the libc errno variable. Using
them we can avoid accessing private libc variables.
The patch was included in python 2.6.
Also we need to raise the right exception, with the right parameters
and a helpful message.
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-14-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
When it is next to the TracepointProvider less scrolling is needed to
change related, surrounding code.
Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-13-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Filter, id and byte are builtin python modules which should not be
redefined by local variables.
Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-12-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Keyword assignments should not not have spaces around the equal
character according to PEP8.
Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-11-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The main function should be the main location for initialization and
helps encapsulating variables into a scope. This way they don't have
to be global and might be mistaken for local ones.
As the providers variable is scoped now it can't be accessed from
within the Stats class. Hence, the global access to the variable was
changed to a local one.
Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-10-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Access checking with F_OK was replaced with the better readable
os.path.exists().
On Linux exists() returns False when the user doesn't have sufficient
permissions for statting the directory. Therefore the error message
now states that sufficient rights are needed when the check fails.
Also added check for /sys/kernel/debug/tracing/.
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-9-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Paths to debugfs and trace dirs are now specified globally to remove
redundancies in the code.
Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-8-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The exit reasons dictionaries were defined number -> value but later
on were accessed the other way around. Therefore a invert function
inverted them.
Defining them the right way removes the need to invert them and
therefore also speeds up the script's setup process.
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-7-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Updating globals over the globals().update() method is not the
standard way of changing globals. Marking variables as global and
modifying them the standard way is better readable.
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-6-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Only two of the constants are actually needed to set up the events, so
the others were removed. All variables that used them were also removed.
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-5-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Constants should be uppercase with separating underscores, as
requested in PEP8. This helps identifying them when reading the code.
Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-4-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Os.walk gives back lists of directories and files, no need to filter
directories from the list that listdir gives back.
To make it better understandable a wrapper with docstring was
introduced.
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-3-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Removed multiple imports of the same module and moved all imports to
the top.
It is not necessary to import a module each time one of its
functions/classes is used.
For readability each import should get its own line.
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Message-Id: <1452525484-32309-2-git-send-email-frankja@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Add a new scripts/clean-includes, which can be used to automatically
ensure that a C source file includes qemu/osdep.h first and doesn't
then include any headers which osdep.h provides already.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 1449505425-32022-2-git-send-email-peter.maydell@linaro.org
We don't want newlines embedded in error messages. This seems to be a common
problem with new code so let's try to catch it with checkpatch.
This will not catch cases where newlines are inserted into the middle of an
existing multi-line statement. But those cases should be rare.
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Message-Id: <1449858642-24267-1-git-send-email-jjherne@linux.vnet.ibm.com>
[Rephrased "Error function text" to "Error messages", dropped
error_vprintf, error_printf, error_printf from $qemu_error_funcs,
because they may legitimately print newlines]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
The checkpatch.pl script has a special case to permit the following
operators to have no spaces around them:
<< >> & ^ | + - * / %
QEMU style prefers all operators to consistently have spacing around
them, so remove this special case handling. This avoids reviewers
having to manually note it during code review.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Stefan Weil <sw@weilnetz.de>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
QEMU now uses internally composed DSDT so drop now
empty *.dsl templates and related *.generated
binary blobs.
Also since templates are not used anymore/obolete
remove utility scripts used for extracting/patching
AML blobs compiled by IASL and for updating them
in git tree.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
The following exception is threw:
Python Exception <class 'NameError'> name 'long' is not defined:
Error occurred in Python command: name 'long' is not defined
Python 2.4+, int()/long() have been unified, so replace long
with int.
Signed-off-by: Yang Wei <w90p710@gmail.com>
Message-id: 1449316340-4030-1-git-send-email-w90p710@gmail.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Implement a QIOChannel subclass that supports sockets I/O.
The implementation is able to manage a single socket file
descriptor, whether a TCP/UNIX listener, TCP/UNIX connection,
or a UDP datagram. It provides APIs which can listen and
connect either asynchronously or synchronously. Since there
is no asynchronous DNS lookup API available, it uses the
QIOTask helper for spawning a background thread to ensure
non-blocking operation.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
We model all the non-deprecated memory allocation functions from
https://developer.gnome.org/glib/stable/glib-Memory-Allocation.html
except for g_memdup(), g_clear_pointer(), g_steal_pointer(). We don't
use the latter two. Model the former.
Coverity now reports an OVERRUN
vl.c:2317: alloc_strlen: Allocating insufficient memory for the terminating null of the string.
Correct, but we omit the terminating null intentionally there.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1448901152-11716-1-git-send-email-armbru@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
In my testing, Coverity reported two more CHECKED_RETURN:
* qemu-char.c:1248: fixed in commit c1f2448: "qemu-char: retry g_poll
on EINTR".
* migration/qemu-file-unix.c:75: harmless, cleaned up in commit
4e39f57 "migration: Clean up use of g_poll() in
socket_writev_buffer()
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1450336833-27710-1-git-send-email-armbru@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
It should be fairly obvious that qapi base classes need to
form an acyclic graph, since QMP cannot specify the same
key more than once, while base classes are included as flat
members alongside other members added by the child. But the
old check_member_clash() parser function was not prepared to
check for this, and entered an infinite recursion (at least
until Python gives up, complaining about nesting too deep).
Now that check_member_clash() has been recently removed,
attempts at self-inheritance trigger an assertion failure
introduced by commit ac88219a. The obvious fix is to turn
the assertion into a conditional.
This patch includes both the tests (base-cycle-direct and
base-cycle-indirect) and the fix, since the .err file output
for the unfixed case is not useful (particularly when it was
warning about unbounded recursion, as that limit may be
platform-specific).
We don't need to worry about cycles in flat unions (neither
the base type nor the type of a variant can be a union) nor
in alternates (alternate branches cannot themselves be an
alternate). But if we later allow a union type as a variant,
we will still be okay, as QAPISchemaObjectTypeVariants.check()
triggers the same QAPISchemaObjectType.check() that will
detect any loops.
Likewise, we need not worry about the case of diamond
inheritance where the same class is used for a flat union base
class and one of its variants; either both uses will introduce
a collision in trying to insert the same member name twice, or
the shared type is empty and changes nothing.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1449033659-25497-16-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
With the recent commit 'qapi: Detect collisions in C member
names', we have two different locations for detecting clashes -
one at parse time, and another at QAPISchema*.check() time.
Remove all of the ad hoc parser checks, and delete associated
code (for example, the global check_member_clash() method is
no longer needed).
Testing this showed that the test union-bad-branch wasn't adding
much: union-clash-branches also exposes the error message when
branches collide, and we've recently fixed things to avoid an
implicit collision with max. Likewise, the error for
enum-clash-member changes to report our new detection of
upper case in a value name, unless we modify the test to use
all lower case.
The wording of several error messages has changed, but the
change is generally an improvement rather than a regression.
No change to generated code.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1449033659-25497-15-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
We document that members of enums and objects should be
'lower-case', although we were not enforcing it. We have to
whitelist a few pre-existing entities that violate the norms.
Add three new tests to expose the new error message, each of
which first uses the whitelisted name 'UuidInfo' to prove the
whitelist works, then triggers the failure (this is the same
pattern used in the existing returns-whitelist.json test).
Note that by adding this check, we have effectively forbidden
an entity with a case-insensitive clash of member names, for
any entity that is not on the whitelist (although there is
still the possibility to clash via '-' vs. '_').
Not done here: a future patch should also add naming convention
support and whitelist exceptions for command, event, and type
names.
The additions to QAPISchemaMember.check_clash() check whether
info['name'] is in the whitelist (the top-most entity name at
the point 'info' tracks), rather than self.owner (the type,
possibly implicit, that directly owns the member), because it
is easier to maintain the whitelist by the names actually in
the user's .json file, rather than worrying about the names
of implicit types.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1449033659-25497-14-git-send-email-eblake@redhat.com>
[Simplified a bit as per discussion with Eric]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Rather than using just an array of strings, make enum.values be
an array of the new QAPISchemaMember type, and add a helper
member_names() method to get back at the original list of names.
Likewise, creating an enum requires wrapping strings, via a new
QAPISchema._make_enum_members() method. The benefit of wrapping
enum members in a QAPISchemaMember Python object is that we now
share the existing code for C name clash detection (although the
code is not yet active until a later commit removes the earlier
ad hoc parser checks).
In a related change, the QAPISchemaMember._pretty_owner() method
needs to learn about one more implicit type name: the generated
enum associated with a simple union.
In the interest of keeping the changes of this patch local to one
file, the visitor interface still passes just a list of names
rather than the full list of QAPISchemaMember instances. We may
want to revisit this in the future, if the consistency with
visit_object_type() is worth it.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1449033659-25497-12-git-send-email-eblake@redhat.com>
[Eric's simplifying followup squashed in]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
We want to share some clash detection code between enum values
and object type members. To assist with that, split off part
of QAPISchemaObjectTypeMember into a new base class
QAPISchemaMember that tracks name, owner, and common clash
detection code; while the former keeps the additional fields
for type and optional flag.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1449033659-25497-11-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
For less code, reflect the determined boolean value of an optional
visit back to the caller instead of making the caller read the
boolean after the fact.
The resulting generated code has the following diff:
|- visit_optional(v, &has_fdset_id, "fdset-id");
|- if (has_fdset_id) {
|+ if (visit_optional(v, &has_fdset_id, "fdset-id")) {
| visit_type_int(v, &fdset_id, "fdset-id", &err);
| if (err) {
| goto out;
| }
| }
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1449033659-25497-10-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
None of the visitor callbacks would set an error when testing
if an optional field was present; make this part of the interface
contract by eliminating the errp argument.
The resulting generated code has a nice diff:
|- visit_optional(v, &has_fdset_id, "fdset-id", &err);
|- if (err) {
|- goto out;
|- }
|+ visit_optional(v, &has_fdset_id, "fdset-id");
| if (has_fdset_id) {
| visit_type_int(v, &fdset_id, "fdset-id", &err);
| if (err) {
| goto out;
| }
| }
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1449033659-25497-9-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
The QMP input visitor allows integral values to be assigned by
promotion to a QTYPE_QFLOAT. However, when parsing an alternate,
we did not take this into account, such that an alternate that
accepts 'number' and some other type, but not 'int', would reject
integral values.
With this patch, we now have the following desirable table:
alternate has case selected for
'int' 'number' QTYPE_QINT QTYPE_QFLOAT
no no error error
no yes 'number' 'number'
yes no 'int' error
yes yes 'int' 'number'
While it is unlikely that we will ever use 'number' in an
alternate other than in the testsuite, it never hurts to be
more precise in what we allow.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1449033659-25497-8-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Now that alternates no longer use an implicit tag, we can
inline _make_implicit_tag() into its one caller,
_def_union_type().
No change to generated code.
Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1449033659-25497-7-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Previously, the generated code in qapi-types.c initialized all
enum lookup tables first, prior to any other definitions. But
there are no topological sorting requirements that mandate this
layout, so we can drop the QAPISchemaGenTypeVisitor._fwdefn
field and just generate all definitions in visitation order.
The generated code shows some churn due to reordering, but it
is still fairly straightforward to follow (all the deletions
occur in one hunk, and all the deleted lines are re-inserted
in the same order later in the same files, just spread across
multiple insertion points).
Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1449033659-25497-6-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Previously, working with alternates required two lookup arrays
and some indirection: for type Foo, we created Foo_qtypes[]
which maps each qtype to a value of the generated FooKind enum,
then look up that value in FooKind_lookup[] like we do for other
union types.
This has a couple of subtle bugs. First, the generator was
creating a call with a parameter '(int *) &(*obj)->type' where
type is an enum type; this is unsafe if the compiler chooses
to store the enum type in a different size than int, where
assigning through the wrong size pointer can corrupt data or
cause a SIGBUS.
Related bug, not not fixed in this patch: qapi-visit.py's
gen_visit_enum() generates a cast of its enum * argument to
int *. Marked FIXME.
Second, since the values of the FooKind enum start at zero, all
entries of the Foo_qtypes[] array that were not explicitly
initialized will map to the same branch of the union as the
first member of the alternate, rather than triggering a desired
failure in visit_get_next_type(). Fortunately, the bug seldom
bites; the very next thing the input visitor does is try to
parse the incoming JSON with the wrong parser, which normally
fails; the output visitor is not used with a C struct in that
state, and the dealloc visitor has nothing to clean up (so
there is no leak).
However, the second bug IS observable in one case: parsing an
integer causes unusual behavior in an alternate that contains
at least a 'number' member but no 'int' member, because the
'number' parser accepts QTYPE_QINT in addition to the expected
QTYPE_QFLOAT (that is, since 'int' is not a member, the type
QTYPE_QINT accidentally maps to FooKind 0; if this enum value
is the 'number' branch the integer parses successfully, but if
the 'number' branch is not first, some other branch tries to
parse the integer and rejects it). A later patch will worry
about fixing alternates to always parse all inputs that a
non-alternate 'number' would accept, for now this is still
marked FIXME in the updated test-qmp-input-visitor.c, to
merely point out that new undesired behavior of 'ans' matches
the existing undesired behavior of 'asn'.
This patch fixes the default-initialization bug by deleting the
indirection, and modifying get_next_type() to directly assign a
QTypeCode parameter. This in turn fixes the type-casting bug,
as we are no longer casting a pointer to enum to a questionable
size. There is no longer a need to generate an implicit FooKind
enum associated with the alternate type (since the QMP wire
format never uses the stringized counterparts of the C union
member names). Since the updated visit_get_next_type() does not
know which qtypes are expected, the generated visitor is
modified to generate an error statement if an unexpected type is
encountered.
Callers now have to know the QTYPE_* mapping when looking at the
discriminator; but so far, only the testsuite was even using the
C struct of an alternate types. I considered the possibility of
keeping the internal enum FooKind, but initialized differently
than most generated arrays, as in:
typedef enum FooKind {
FOO_KIND_A = QTYPE_QDICT,
FOO_KIND_B = QTYPE_QINT,
} FooKind;
to create nicer aliases for knowing when to use foo->a or foo->b
when inspecting foo->type; but it turned out to add too much
complexity, especially without a client.
There is a user-visible side effect to this change, but I
consider it to be an improvement. Previously,
the invalid QMP command:
{"execute":"blockdev-add", "arguments":{"options":
{"driver":"raw", "id":"a", "file":true}}}
failed with:
{"error": {"class": "GenericError",
"desc": "Invalid parameter type for 'file', expected: QDict"}}
(visit_get_next_type() succeeded, and the error comes from the
visit_type_BlockdevOptions() expecting {}; there is no mention of
the fact that a string would also work). Now it fails with:
{"error": {"class": "GenericError",
"desc": "Invalid parameter type for 'file', expected: BlockdevRef"}}
(the error when the next type doesn't match any expected types for
the overall alternate).
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1449033659-25497-5-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
What's more meta than using qapi to define qapi? :)
Convert QType into a full-fledged[*] builtin qapi enum type, so
that a subsequent patch can then use it as the discriminator
type of qapi alternate types. Fortunately, the judicious use of
'prefix' in the qapi definition avoids churn to the spelling of
the enum constants.
To avoid circular definitions, we have to flip the order of
inclusion between "qobject.h" vs. "qapi-types.h". Back in commit
28770e0, we had the latter include the former, so that we could
use 'QObject *' for our implementation of 'any'. But that usage
also works with only a forward declaration, whereas the
definition of QObject requires QType to be a complete type.
[*] The type has to be builtin, rather than declared in
qapi/common.json, because we want to use it for alternates even
when common.json is not included. But since it is the first
builtin enum type, we have to add special cases to qapi-types
and qapi-visit to only emit definitions once, even when two
qapi files are being compiled into the same binary (the way we
already handled builtin list types like 'intList'). We may
need to revisit how multiple qapi files share common types,
but that's a project for another day.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1449033659-25497-4-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
The name QType matches our CODING_STYLE conventions for type names
in CamelCase. It also matches the fact that we are already naming
all the enum members with a prefix of QTYPE, not QTYPE_CODE. And
doing the rename will also make it easier for the next patch to use
QAPI for providing the enum, which also wants CamelCase type names.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1449033659-25497-3-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
When munging enum values, the fact that we were passing the entire
prefix + value through camel_to_upper() meant that enum values
spelled with CamelCase could be turned into CAMEL_CASE. However,
this provides a potential collision (both OneTwo and One-Two would
munge into ONE_TWO) for enum types, when the same two names are
valid side-by-side as QAPI member names. By changing the generation
of enum constants to always be prefix + '_' + c_name(value,
False).upper(), and ensuring that there are no case collisions (in
the next patches), we no longer have to worry about names that
would be distinct as QAPI members but collide as variant tag names,
without having to think about what munging the heuristics in
camel_to_upper() will actually perform on an enum value.
Making the change will affect enums that did not follow coding
conventions, using 'CamelCase' rather than desired 'lower-case'.
Thankfully, there are only two culprits: InputButton and ErrorClass.
We already tweaked ErrorClass to make it an alias of QapiErrorClass,
where only the alias needs changing rather than the whole tree. So
the bulk of this change is modifying INPUT_BUTTON_WHEEL_UP to the
new INPUT_BUTTON_WHEELUP (and likewise for WHEELDOWN). That part
of this commit may later need reverting if we rename the enum
constants from 'WheelUp' to 'wheel-up' as part of moving
x-input-send-event to a stable interface; but at least we have
documentation bread crumbs in place to remind us (commit 513e7cd),
and it matches the fact that SDL constants are also spelled
SDL_BUTTON_WHEELUP.
Suggested by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1447836791-369-27-git-send-email-eblake@redhat.com>
[Commit message tweaked]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Now that we no longer collide with an implicit _MAX enum member,
we no longer need to reject it in the ad hoc parser, and can
remove several tests that are no longer needed.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1447836791-369-24-git-send-email-eblake@redhat.com>
[Commit message tweaked]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Now that we guarantee the user doesn't have any enum values
beginning with a single underscore, we can use that for our
own purposes. Renaming ENUM_MAX to ENUM__MAX makes it obvious
that the sentinel is generated.
This patch was mostly generated by applying a temporary patch:
|diff --git a/scripts/qapi.py b/scripts/qapi.py
|index e6d014b..b862ec9 100644
|--- a/scripts/qapi.py
|+++ b/scripts/qapi.py
|@@ -1570,6 +1570,7 @@ const char *const %(c_name)s_lookup[] = {
| max_index = c_enum_const(name, 'MAX', prefix)
| ret += mcgen('''
| [%(max_index)s] = NULL,
|+// %(max_index)s
| };
| ''',
| max_index=max_index)
then running:
$ cat qapi-{types,event}.c tests/test-qapi-types.c |
sed -n 's,^// \(.*\)MAX,s|\1MAX|\1_MAX|g,p' > list
$ git grep -l _MAX | xargs sed -i -f list
The only things not generated are the changes in scripts/qapi.py.
Rejecting enum members named 'MAX' is now useless, and will be dropped
in the next patch.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1447836791-369-23-git-send-email-eblake@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
[Rebased to current master, commit message tweaked]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
We already documented that qapi names should match specific
patterns (such as starting with a letter unless it was an enum
value or a downstream extension). Tighten that from a suggestion
into a hard requirement, which frees up names beginning with a
single underscore for qapi internal usage.
The tighter regex doesn't forbid everything insane that a user
could provide (for example, a user could name a type 'Foo-lookup'
to collide with the generated 'Foo_lookup[]' for an enum 'Foo'),
but does a good job at protecting the most obvious uses, and
also happens to reserve single leading underscore for later use.
The handling of enum values starting with a digit is tricky:
commit 9fb081e introduced a subtle bug by using c_name() on
a munged value, which would allow an enum to include the
member 'q-int' in spite of our reservation. Furthermore,
munging with a leading '_' would fail our tighter regex. So
fix it by only munging for leading digits (which are never
ticklish in c_name()) and by using a different prefix (I
picked 'D', although any letter should do).
Add new tests, reserved-member-underscore and reserved-enum-q,
to demonstrate the tighter checking.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1447836791-369-22-git-send-email-eblake@redhat.com>
Message-Id: <1447883135-18020-1-git-send-email-eblake@redhat.com>
[Eric's fixup squashed in]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
The method c_name() is supposed to do two different actions: munge
'-' into '_', and add a 'q_' prefix to ticklish names. But it did
these steps out of order, making it possible to submit input that
is not ticklish until after munging, where the output then lacked
the desired prefix.
The failure is exposed easily if you have a compiler that recognizes
C11 keywords, and try to name a member '_Thread-local', as it would
result in trying to compile the declaration 'uint64_t _Thread_local;'
which is not valid. However, this name violates our conventions
(ultimately, want to enforce that no qapi names start with single
underscore), so the test is slightly weaker by instead testing
'wchar-t'; the declaration 'uint64_t wchar_t;' is valid in C (where
wchar_t is only a typedef) but would fail with a C++ compiler (where
it is a keyword).
Fix things by reversing the order of actions within c_name().
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1447836791-369-18-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Detect attempts to declare two object members that would result
in the same C member name, by keying the 'seen' dictionary off
of the C name rather than the qapi name. It also requires passing
info through the check_clash() methods.
This addresses a TODO and fixes the previously-broken
args-name-clash test. The resulting error message demonstrates
the utility of the .describe() method added previously. No change
to generated code.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1447836791-369-17-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Future commits will migrate semantic checking away from parsing
and over to the various QAPISchema*.check() methods. But to
report an error message about an incorrect semantic use of a
member of an object type, it helps to know which type, command,
or event owns the member. In particular, when a member is
inherited from a base type, it is desirable to associate the
member name with the base type (and not the type calling
member.check()).
Rather than packing additional information into the seen array
passed to each member.check() (as in seen[m.name] = {'member':m,
'owner':type}), it is easier to have each member track the name
of the owner type in the first place (keeping things simpler
with the existing seen[m.name] = m). The new member.owner field
is set via a new set_owner() method, called when registering
the members and variants arrays with an object or variant type.
Track only a name, and not the actual type object, to avoid
creating a circular python reference chain.
Note that Variants.set_owner() method does not set the owner
for the tag_member field; this field is set earlier either as
part of an object's non-variant members, or explicitly by
alternates.
The source information is intended for human consumption in
error messages, and a new describe() method is added to access
the resulting information. For example, given the qapi:
{ 'command': 'foo', 'data': { 'string': 'str' } }
an implementation of visit_command() that calls
arg_type.members[0].describe()
will see "'string' (parameter of foo)".
To make the human-readable name of implicit types work without
duplicating efforts, the describe() method has to reverse the
name of implicit types, via the helper _pretty_owner().
No change to generated code.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1447836791-369-16-git-send-email-eblake@redhat.com>
[Incorrect & unused -wrapper case in _pretty_owner() dropped]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Now that branches are in a separate C namespace, we can remove
the restrictions in the parser that claim a branch name would
collide with QMP, and delete the negative tests that are no
longer problematic. A separate patch can then add positive
tests to qapi-schema-test to test that any corner cases will
compile correctly.
This reverts the scripts/qapi.py portion of commit 7b2a5c2,
now that the assertions that it plugged are no longer possible.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1447836791-369-15-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Checking that a given QAPISchemaObjectTypeVariant.name is a
member of the corresponding QAPISchemaEnumType of the owning
QAPISchemaObjectTypeVariants.tag_member ensures that there are
no collisions in the generated C union for those tag values
(since the enum itself should have no collisions).
However, ever since its introduction in f51d8c3d, this was the
only additional action of of Variant.check(), beyond calling
the superclass Member.check(). This forces a difference in
.check() signatures, just to pass the enum type down.
Simplify things by instead doing the tag name check as part of
Variants.check(), at which point we can rely on inheritance
instead of overriding Variant.check().
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1447836791-369-14-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Consolidate two common sequences of clash detection into a
new QAPISchemaObjectType.check_clash() helper method.
No change to generated code.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1447836791-369-13-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Right now, our ad hoc parser ensures that we cannot have a
flat union that introduces any members that would clash with
non-variant members inherited from the union's base type (see
flat-union-clash-member.json). We want QAPISchemaObjectType.check()
to make the same check, so we can later reduce some of the ad
hoc checks.
We already have a map 'seen' of all non-variant members. We
still need to check for collisions between each variant type's
members and the non-variant ones.
To know the variant type's members, we need to call
variant.type.check(). This also detects when a type contains
itself in a variant, exactly like the existing base.check()
detects when a type contains itself as a base. (Except that
we currently forbid anything but a struct as the type of a
variant, so we can't actually trigger this type of loop yet.)
Slight complication: an alternate's variant can have arbitrary
type, but only an object type's check() may be called outside
QAPISchema.check(). We could either skip the call for variants
of alternates, or skip it for non-object types. For now, do
the latter, because it's easier.
Then we call each variant member's check_clash() with the
appropriate 'seen' map. Since members of different variants
can't clash, we have to clone a fresh seen for each variant.
Wrap this in a new helper method
QAPISchemaObjectTypeVariants.check_clash().
Note that cloning 'seen' inside .check_clash() resembles
the one we just removed from .check() in 'qapi: Drop
obsolete tag value collision assertions'; the difference here is
that we are now checking for clashes among the qapi members of
the variant type, rather than for a single clash with the variant
tag name itself.
Note that, by construction, collisions can't actually happen for
simple unions: each variant's type is a wrapper with a single
member 'data', which will never collide with the only non-variant
member 'type'.
For alternates, there's nothing for a variant object type's
members to clash with, and therefore no need to call the new
variants.check_clash().
No change to generated code.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1447836791-369-12-git-send-email-eblake@redhat.com>
[Commit message tweaked]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reduce the ugly flat union / simple union conditional by doing just
the essential work here, namely setting self.tag_member.
Move the rest to callers.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1446559499-26984-7-git-send-email-armbru@redhat.com>
[rebase to earlier changes that moved tag_member.check() of
alternate types, and tweak commit title and wording]
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1447836791-369-11-git-send-email-eblake@redhat.com>
While there, stick in a TODO change key of seen from QAPI name to C
name. Can't do it right away, because it would fail the assertion for
tests/qapi-schema/args-has-clash.json.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1446559499-26984-6-git-send-email-armbru@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1447836791-369-10-git-send-email-eblake@redhat.com>
We can use seen.values() instead if we make it an OrderedDict.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1446559499-26984-5-git-send-email-armbru@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1447836791-369-9-git-send-email-eblake@redhat.com>
This hunk
@@ -964,6 +965,7 @@ class QAPISchemaObjectType(QAPISchemaType):
members = []
seen = {}
for m in members:
+ assert c_name(m.name) not in seen
seen[m.name] = m
for m in self.local_members:
m.check(schema, members, seen)
is plainly broken.
Asserting the members inherited from base don't clash is somewhat
redundant, because self.base.check() just checked that. But it
doesn't hurt.
The idea to use c_name(m.name) instead of m.name for collision
checking is sound, because we need to catch clashes between the m.name
and between the c_name(m.name), and when two m.name clash, then their
c_name() also clash.
However, using c_name(m.name) instead of m.name in one of several
places doesn't work. See the very next line.
Keep the assertion, but drop the c_name() for now. A future commit
will bring it back.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1446559499-26984-4-git-send-email-armbru@redhat.com>
[change TABs in commit message to space]
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1447836791-369-8-git-send-email-eblake@redhat.com>
QAPISchemaObjectTypeVariants.check() parameter members and
QAPISchemaObjectTypeVariant.check() parameter seen are no longer used,
drop them.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1446559499-26984-3-git-send-email-armbru@redhat.com>
[rebase to earlier changes that moved tag_member.check() of
alternate types]
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1447836791-369-7-git-send-email-eblake@redhat.com>
QAPISchemaObjectTypeMember.check() currently does four things:
1. Compute self.type
2. Accumulate members in all_members
Only one caller cares: QAPISchemaObjectType.check() uses it to
compute self.members. The other callers pass a throw-away
accumulator.
3. Accumulate a map from names to members in seen
Only one caller cares: QAPISchemaObjectType.check() uses it to
compute its local variable seen, for self.variants.check(), which
uses it to compute self.variants.tag_member from
self.variants.tag_name. The other callers pass a throw-away
accumulator.
4. Check for collisions
This piggybacks on 3: before adding a new entry, we assert it's new.
Only one caller cares: QAPISchemaObjectType.check() uses it to
assert non-variant members don't clash.
Simplify QAPISchemaObjectType.check(): move 2.-4. to
QAPISchemaObjectType.check(), and drop parameters all_members and
seen.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1446559499-26984-2-git-send-email-armbru@redhat.com>
[rebase to earlier changes that moved tag_member.check() of
alternate types, commit message typo fix]
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1447836791-369-6-git-send-email-eblake@redhat.com>
Union tag values can't clash with member names in generated C anymore
since commit e4ba22b, but QAPISchemaObjectTypeVariants.check() still
asserts they don't. Drop it.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1446559499-26984-1-git-send-email-armbru@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1447836791-369-5-git-send-email-eblake@redhat.com>
Simplify gen_struct_fields() back to a single iteration over a
list of fields (like it was prior to commit f87ab7f9), by moving
the generated comments to gen_object(). Then, inline
gen_struct_field() into its only caller.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1447836791-369-4-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
These two methods are now close enough that we can finally merge
them, relying on the fact that simple unions now provide a
reasonable local_members. Change gen_struct() to gen_object()
that handles all forms of QAPISchemaObjectType, and rename and
shrink gen_union() to gen_variants() to handle the portion of
gen_object() needed when variants are present.
gen_struct_fields() now has a single caller, so it no longer
needs an optional parameter; however, I did not choose to inline
it into the caller.
No difference to generated code.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1447836791-369-3-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
We were previously creating all unions with an empty list for
local_members. However, it will make it easier to unify struct
and union generation if we include the generated tag member in
local_members. That way, we can have a common code pattern:
visit the base (if any), visit the local members (if any), visit
the variants (if any). The local_members of a flat union
remains empty (because the discriminator is already visited as
part of the base). Then, by visiting tag_member.check() during
AlternateType.check(), we no longer need to call it during
Variants.check().
The various front end entities now exist as follows:
struct: optional base, optional local_members, no variants
simple union: no base, one-element local_members, variants with tag_member
from local_members
flat union: base, no local_members, variants with tag_member from base
alternate: no base, no local_members, variants
With the new local members, we require a bit of finesse to
avoid assertions in the clients.
No change to generated code.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1447836791-369-2-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
For the sake of humans reading introspection output, it is nice
to have the name of implicit array types be recognizable as
arrays of the underlying type. However, while this patch allows
humans to skip from a command with return type "[123]" straight
to the definition of type "123" without having to first inspect
type "[123]", document that this shortcut should not be taken by
client apps.
This makes the resulting introspection string slightly larger by
default (just over 200 bytes), but it's in the noise (less than
0.3% of the overall 70k size of 'query-qmp-capabilities').
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1446791754-23823-12-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Our generated list visitors have the same problem as has been
mentioned elsewhere (see commit 2f52e20): they allocate data
even on failure. An upcoming patch will correct things to
provide saner guarantees, but first we need to expose the
behavior in the testsuite to ensure we aren't introducing any
memory usage bugs.
There are more test cases throughout the test-qmp-input-* tests
that already deal with partial allocation; a later commit will
clean up all visit_type_FOO(), without marking all of the tests
with FIXME at this time.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1446791754-23823-10-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Commit 61964 "Add configuration section" broke the analyze-migration.py script
which terminates due to the unrecognised section. Fix the script by parsing
the contents of the configuration section directly into a new
ConfigurationSection object (although nothing is done with it yet).
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Juan Quintela <quintela@redhat.com>al3
Signed-off-by: Juan Quintela <quintela@redhat.com>al3
Latest perl now deprecates "{" literal in regex and print warnings like
"unescaped left brace in regex is deprecated". Add escapes to keep it
happy.
Signed-off-by: Fam Zheng <famz@redhat.com>
Message-Id: <1445326726-16031-1-git-send-email-famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Rather than having all callers pass a name, type, and optional
flag, have them instead pass a QAPISchemaObjectTypeMember which
already has all that information.
No change to generated code.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1445898903-12082-25-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Now that we have separated union tag values from colliding with
non-variant C names, by naming the union 'u', we should reserve
this name for our use. Note that we want to forbid 'u' even in
a struct with no variants, because it is possible for a future
qemu release to extend QMP in a backwards-compatible manner while
converting from a struct to a flat union. Fortunately, no
existing clients were using this member name. If we ever find
the need for QMP to have a member 'u', we could at that time
relax things, perhaps by having c_name() munge the QMP member to
'q_u'.
Note that we cannot forbid 'u' everywhere (by adding the
rejection code to check_name()), because the existing QKeyCode
enum already uses it; therefore we only reserve it as a struct
type member name.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1445898903-12082-24-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
We have two issues with our qapi union layout:
1) Even though the QMP wire format spells the tag 'type', the
C code spells it 'kind', requiring some hacks in the generator.
2) The C struct uses an anonymous union, which places all tag
values in the same namespace as all non-variant members. This
leads to spurious collisions if a tag value matches a non-variant
member's name.
This patch is the back end for a series that converts to a
saner qapi union layout. Now that all clients have been
converted to use 'type' and 'obj->u.value', we can drop the
temporary parallel support for 'kind' and 'obj->value'.
Given a simple union qapi type:
{ 'union':'Foo', 'data': { 'a':'int', 'b':'bool' } }
this is the overall effect, when compared to the state before
this series of patches:
| struct Foo {
|- FooKind kind;
|- union { /* union tag is @kind */
|+ FooKind type;
|+ union { /* union tag is @type */
| void *data;
| int64_t a;
| bool b;
|- };
|+ } u;
| };
The testsuite still contains some examples of artificial restrictions
(see flat-union-clash-type.json, for example) that are no longer
technically necessary, now that there is no longer a collision between
enum tag values and non-variant member names; but fixing this will be
done in later patches, in part because some further changes are required
to keep QAPISchema*.check() from asserting. Also, a later patch will
add a reservation for the member name 'u' to avoid a collision between a
user's non-variant names and our internal choice of C union name.
Note, however, that we do not rename the generated enum, which
is still 'FooKind'. A further patch could generate implicit
enums as 'FooType', but while the generator already reserved
the '*Kind' namespace (commit 4dc2e69), there are already QMP
constructs with '*Type' naming, which means changing our
reservation namespace would have lots of churn to C code to
deal with a forced name change.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1445898903-12082-23-git-send-email-eblake@redhat.com>
[Commit message tweaked]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
We have two issues with our qapi union layout:
1) Even though the QMP wire format spells the tag 'type', the
C code spells it 'kind', requiring some hacks in the generator.
2) The C struct uses an anonymous union, which places all tag
values in the same namespace as all non-variant members. This
leads to spurious collisions if a tag value matches a non-variant
member's name.
Make the conversion to the new layout for qapi-visit.py.
Generated code changes look like:
|@@ -4912,16 +4912,16 @@ void visit_type_MemoryDeviceInfo(Visitor
| if (!*obj) {
| goto out_obj;
| }
|- visit_type_MemoryDeviceInfoKind(v, &(*obj)->kind, "type", &err);
|+ visit_type_MemoryDeviceInfoKind(v, &(*obj)->type, "type", &err);
| if (err) {
| goto out_obj;
| }
|- if (!visit_start_union(v, !!(*obj)->data, &err) || err) {
|+ if (!visit_start_union(v, !!(*obj)->u.data, &err) || err) {
| goto out_obj;
| }
|- switch ((*obj)->kind) {
|+ switch ((*obj)->type) {
| case MEMORY_DEVICE_INFO_KIND_DIMM:
|- visit_type_PCDIMMDeviceInfo(v, &(*obj)->dimm, "data", &err);
|+ visit_type_PCDIMMDeviceInfo(v, &(*obj)->u.dimm, "data", &err);
| break;
| default:
| abort();
|@@ -4930,7 +4930,7 @@ out_obj:
| error_propagate(errp, err);
| err = NULL;
| if (*obj) {
|- visit_end_union(v, !!(*obj)->data, &err);
|+ visit_end_union(v, !!(*obj)->u.data, &err);
| }
| error_propagate(errp, err);
| err = NULL;
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1445898903-12082-14-git-send-email-eblake@redhat.com>
[Commit message tweaked slightly]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
We have two issues with our qapi union layout:
1) Even though the QMP wire format spells the tag 'type', the
C code spells it 'kind', requiring some hacks in the generator.
2) The C struct uses an anonymous union, which places all tag
values in the same namespace as all non-variant members. This
leads to spurious collisions if a tag value matches a non-variant
member's name.
This patch is the front end for a series that converts to a
saner qapi union layout. By the end of the series, we will no
longer have the type/kind mismatch, and all tag values will be
under a named union, which requires clients to access
'obj->u.value' instead of 'obj->value'. But since the
conversion touches a number of files, it is easiest if we
temporarily support BOTH layouts simultaneously.
Given a simple union qapi type:
{ 'union':'Foo', 'data': { 'a':'int', 'b':'bool' } }
make the following changes in generated qapi-types.h:
| struct Foo {
|- FooKind kind;
|- union { /* union tag is @kind */
|+ union {
|+ FooKind kind;
|+ FooKind type;
|+ };
|+ union { /* union tag is @type */
| void *data;
| int64_t a;
| bool b;
|+ union { /* union tag is @type */
|+ void *data;
|+ int64_t a;
|+ bool b;
|+ } u;
| };
| };
Flat unions do not need the anonymous union for the tag member,
as we already fixed that to use the member name instead of 'kind'
back in commit 0f61af3e.
One additional change is needed in qapi.py: check_union() now
needs to check for collisions with 'type' in addition to those
with 'kind'.
Later, when the conversions are complete, we will remove the
duplication hacks, and also drop the check_union() restrictions.
Note, however, that we do not rename the generated enum, which
is still 'FooKind'. A further patch could generate implicit
enums as 'FooType', but while the generator already reserved
the '*Kind' namespace (commit 4dc2e69), there are already QMP
constructs with '*Type' naming, which means changing our
reservation namespace would have lots of churn to C code to
deal with a forced name change.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1445898903-12082-13-git-send-email-eblake@redhat.com>
[Commit message tweaked slightly]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
The code for visiting the base class of a child struct created
visit_type_Base_fields() which covers all fields of Base; while
the code for visiting the base class of a flat union created
visit_type_Union_fields() covering all fields of the base
except the discriminator. But since the base class includes
the discriminator of a flat union, we can just visit the entire
base, without needing a separate visit of the discriminator.
Not only is consistently visiting all fields easier to
understand, it lets us share code.
The generated code in qapi-visit.c loses several now-unused
visit_type_UNION_fields(), along with changes like:
|@@ -1654,11 +1557,7 @@ void visit_type_BlockdevOptions(Visitor
| if (!*obj) {
| goto out_obj;
| }
|- visit_type_BlockdevOptions_fields(v, obj, &err);
|- if (err) {
|- goto out_obj;
|- }
|- visit_type_BlockdevDriver(v, &(*obj)->driver, "driver", &err);
|+ visit_type_BlockdevOptionsBase_fields(v, (BlockdevOptionsBase **)obj, &err);
| if (err) {
| goto out_obj;
| }
and forward declarations where needed. Note that the cast of obj
to BASE ** is necessary to call visit_type_BASE_fields() (and we
can't use our upcast wrappers, because those work on pointers while
we have a pointer-to-pointer).
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1445898903-12082-12-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Eric Blake
Daniel P. Berrange
Lluís Vilanova
Gerd Hoffmann
Peter Maydell
Fam Zheng
Markus Armbruster
Leonid Bloch
Paolo Bonzini
Janosch Frank
Amit Shah
Jason J. Herne
Igor Mammedov
Yang Wei
Mark Cave-Ayland