Returning a partial object on error is an invitation for a careless
caller to leak memory. We already fixed things in an earlier
patch to guarantee NULL if visit_start fails ("qapi: Guarantee
NULL obj on input visitor callback error"), but that does not
help the case where visit_start succeeds but some other failure
happens before visit_end, such that we leak a partially constructed
object outside visit_type_FOO(). As no one outside the testsuite
was actually relying on these semantics, it is cleaner to just
document and guarantee that ALL pointer-based visit_type_FOO()
functions always leave a safe value in *obj during an input visitor
(either the new object on success, or NULL if an error is
encountered), so callers can now unconditionally use
qapi_free_FOO() to clean up regardless of whether an error occurred.
The decision is done by adding visit_is_input(), then updating the
generated code to check if additional cleanup is needed based on
the type of visitor in use.
Note that we still leave *obj unchanged after a scalar-based
visit_type_FOO(); I did not feel like auditing all uses of
visit_type_Enum() to see if the callers would tolerate a specific
sentinel value (not to mention having to decide whether it would
be better to use 0 or ENUM__MAX as that sentinel).
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1461879932-9020-25-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
The semantics of the list visit are somewhat baroque, with the
following pseudocode when FooList is used:
start()
for (prev = head; cur = next(prev); prev = &cur) {
visit(&cur->value)
}
Note that these semantics (advance before visit) requires that
the first call to next() return the list head, while all other
calls return the next element of the list; that is, every visitor
implementation is required to track extra state to decide whether
to return the input as-is, or to advance. It also requires an
argument of 'GenericList **' to next(), solely because the first
iteration might need to modify the caller's GenericList head, so
that all other calls have to do a layer of dereferencing.
Thankfully, we only have two uses of list visits in the entire
code base: one in spapr_drc (which completely avoids
visit_next_list(), feeding in integers from a different source
than uint8List), and one in qapi-visit.py. That is, all other
list visitors are generated in qapi-visit.c, and share the same
paradigm based on a qapi FooList type, so we can refactor how
lists are laid out with minimal churn among clients.
We can greatly simplify things by hoisting the special case
into the start() routine, and flipping the order in the loop
to visit before advance:
start(head)
for (tail = *head; tail; tail = next(tail)) {
visit(&tail->value)
}
With the simpler semantics, visitors have less state to track,
the argument to next() is reduced to 'GenericList *', and it
also becomes obvious whether an input visitor is allocating a
FooList during visit_start_list() (rather than the old way of
not knowing if an allocation happened until the first
visit_next_list()). As a minor drawback, we now allocate in
two functions instead of one, and have to pass the size to
both functions (unless we were to tweak the input visitors to
cache the size to start_list for reuse during next_list, but
that defeats the goal of less visitor state).
The signature of visit_start_list() is chosen to match
visit_start_struct(), with the new parameters after 'name'.
The spapr_drc case is a virtual visit, done by passing NULL for
list, similarly to how NULL is passed to visit_start_struct()
when a qapi type is not used in those visits. It was easy to
provide these semantics for qmp-output and dealloc visitors,
and a bit harder for qmp-input (several prerequisite patches
refactored things to make this patch straightforward). But it
turned out that the string and opts visitors munge enough other
state during visit_next_list() to make it easier to just
document and require a GenericList visit for now; an assertion
will remind us to adjust things if we need the semantics in the
future.
Several pre-requisite cleanup patches made the reshuffling of
the various visitors easier; particularly the qmp input visitor.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1461879932-9020-24-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
As mentioned in previous patches, we want to call visit_end_struct()
functions unconditionally, so that visitors can release resources
tied up since the matching visit_start_struct() without also having
to worry about error priority if more than one error occurs.
Even though error_propagate() can be safely used to ignore a second
error during cleanup caused by a first error, it is simpler if the
cleanup cannot set an error. So, split out the error checking
portion (basically, input visitors checking for unvisited keys) into
a new function visit_check_struct(), which can be safely skipped if
any earlier errors are encountered, and leave the cleanup portion
(which never fails, but must be called unconditionally if
visit_start_struct() succeeded) in visit_end_struct().
Generated code in qapi-visit.c has diffs resembling:
|@@ -59,10 +59,12 @@ void visit_type_ACPIOSTInfo(Visitor *v,
| goto out_obj;
| }
| visit_type_ACPIOSTInfo_members(v, obj, &err);
|- error_propagate(errp, err);
|- err = NULL;
|+ if (err) {
|+ goto out_obj;
|+ }
|+ visit_check_struct(v, &err);
| out_obj:
|- visit_end_struct(v, &err);
|+ visit_end_struct(v);
| out:
and in qapi-event.c:
@@ -47,7 +47,10 @@ void qapi_event_send_acpi_device_ost(ACP
| goto out;
| }
| visit_type_q_obj_ACPI_DEVICE_OST_arg_members(v, ¶m, &err);
|- visit_end_struct(v, err ? NULL : &err);
|+ if (!err) {
|+ visit_check_struct(v, &err);
|+ }
|+ visit_end_struct(v);
| if (err) {
| goto out;
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1461879932-9020-20-git-send-email-eblake@redhat.com>
[Conflict with a doc fixup resolved]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
The qmp-input visitor was allowing callers to play rather fast
and loose: when visiting a QDict, you could grab members of the
root dictionary without first pushing into the dict; among the
culprit callers was the generated marshal code on the 'arguments'
dictionary of a QMP command. But we are about to tighten the
input visitor, at which point the generated marshal code MUST
follow the same paradigms as everyone else, of pushing into the
struct before grabbing its keys.
Generated code grows as follows:
|@@ -515,7 +641,12 @@ void qmp_marshal_blockdev_backup(QDict *
| BlockdevBackup arg = {0};
|
| v = qmp_input_get_visitor(qiv);
|+ visit_start_struct(v, NULL, NULL, 0, &err);
|+ if (err) {
|+ goto out;
|+ }
| visit_type_BlockdevBackup_members(v, &arg, &err);
|+ visit_end_struct(v, err ? NULL : &err);
| if (err) {
| goto out;
| }
|@@ -527,7 +715,9 @@ out:
| qmp_input_visitor_cleanup(qiv);
| qdv = qapi_dealloc_visitor_new();
| v = qapi_dealloc_get_visitor(qdv);
|+ visit_start_struct(v, NULL, NULL, 0, NULL);
| visit_type_BlockdevBackup_members(v, &arg, NULL);
|+ visit_end_struct(v, NULL);
| qapi_dealloc_visitor_cleanup(qdv);
| }
The use of 'err ? NULL : &err' is temporary; a later patch will
clean that up when it splits visit_end_struct().
Prior to this patch, the fact that there was no final
visit_end_struct() meant that even though we are using a strict
input visit, the marshalling code was not detecting excess input
at the top level (only in nested levels). Fortunately, we have
code in monitor.c:qmp_check_client_args() that also checks for
no excess arguments at the top level. But as the generated code
is more compact than the manual check, a later patch will clean
up monitor.c to drop the redundancy added here.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1461879932-9020-9-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Rather than having two separate ways to create a QMP input
visitor, where the safer approach has the more verbose name,
it is better to consolidate things into a single function
where the caller must explicitly choose whether to be strict
or to ignore excess input. This patch is the strictly
mechanical conversion; the next patch will then audit which
uses can be made stricter.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1461879932-9020-6-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
In some occasions, a patch [1] can start with a hunk containing a
simple type cast. At the time annotate_values() is run, the type is
unknown and the cast type is misinterpreted as a identifier, resulting
in an error if it is followed with a negative value:
ERROR: spaces required around that '-' (ctx:WxV)
It seems complex to catch all possible types in a cast expression. So,
as a fallback solution, let's add some common qemu types to the
typeList array.
[1] http://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg06741.html
Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
Message-Id: <1459503606-31603-1-git-send-email-clg@fr.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* Chardev fix from Marc-André
* config.status tweak from David
* Header file tweaks from Markus, myself and Veronia (Outreachy candidate)
* get_ticks_per_sec() removal from Rutuja (Outreachy candidate)
* Coverity fix from myself
* PKE implementation from myself, based on rth's XSAVE support
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQEcBAABCAAGBQJW9ErPAAoJEL/70l94x66DJfEH/A/QkMpAhrgNdyVsahzsGrzE
wx5gHFIc1nBYxyr62w4apUb5jPB7zaXu0LA7EAWDeAe0pyP8hZzLT9kJyOEDsuJu
zwKN2QeLSNMtPbnbKN0I/YQ2za2xX1V5ruhSeOJoVslUI214hgnAURaGshhQNzuZ
2CluDT9KgL5cQifAnKs5kJrwhIYShYNQB+1eDC/7wk28dd/EH+sPALIoF+rqrSmt
Zu4Mdqd+9Ns+oKOjA6br9ULq/Hzg0aDfY82J+XLVVqfF3PXQe8rTDmuMf/7jTn+M
Un7ZOcei9oZF2/9vfAfKQpDCcgD9HvOUSbgqV/ubmkPPmN/LNJzeKj0fBhrRN+Y=
=K12D
-----END PGP SIGNATURE-----
Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging
* Log filtering from Alex and Peter
* Chardev fix from Marc-André
* config.status tweak from David
* Header file tweaks from Markus, myself and Veronia (Outreachy candidate)
* get_ticks_per_sec() removal from Rutuja (Outreachy candidate)
* Coverity fix from myself
* PKE implementation from myself, based on rth's XSAVE support
# gpg: Signature made Thu 24 Mar 2016 20:15:11 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)
target-i386: implement PKE for TCG
config.status: Pass extra parameters
char: translate from QIOChannel error to errno
exec: fix error handling in file_ram_alloc
cputlb: modernise the debug support
qemu-log: support simple pid substitution for logs
target-arm: dfilter support for in_asm
qemu-log: dfilter-ise exec, out_asm, op and opt_op
qemu-log: new option -dfilter to limit output
qemu-log: Improve the "exec" TB execution logging
qemu-log: Avoid function call for disabled qemu_log_mask logging
qemu-log: correct help text for -d cpu
tcg: pass down TranslationBlock to tcg_code_gen
util: move declarations out of qemu-common.h
Replaced get_tick_per_sec() by NANOSECONDS_PER_SECOND
hw: explicitly include qemu-common.h and cpu.h
include/crypto: Include qapi-types.h or qemu/bswap.h instead of qemu-common.h
isa: Move DMA_transfer_handler from qemu-common.h to hw/isa/isa.h
Move ParallelIOArg from qemu-common.h to sysemu/char.h
Move QEMU_ALIGN_*() from qemu-common.h to qemu/osdep.h
...
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Conflicts:
scripts/clean-includes
Manually drop redundant includes that scripts/clean-includes misses,
e.g. because they're hidden in generator programs, or they use the
wrong kind of delimiter.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Commit 57cb38b included qapi/error.h into qemu/osdep.h to get the
Error typedef. Since then, we've moved to include qemu/osdep.h
everywhere. Its file comment explains: "To avoid getting into
possible circular include dependencies, this file should not include
any other QEMU headers, with the exceptions of config-host.h,
compiler.h, os-posix.h and os-win32.h, all of which are doing a
similar job to this file and are under similar constraints."
qapi/error.h doesn't do a similar job, and it doesn't adhere to
similar constraints: it includes qapi-types.h. That's in excess of
100KiB of crap most .c files don't actually need.
Add the typedef to qemu/typedefs.h, and include that instead of
qapi/error.h. Include qapi/error.h in .c files that need it and don't
get it now. Include qapi-types.h in qom/object.h for uint16List.
Update scripts/clean-includes accordingly. Update it further to match
reality: replace config.h by config-target.h, add sysemu/os-posix.h,
sysemu/os-win32.h. Update the list of includes in the qemu/osdep.h
comment quoted above similarly.
This reduces the number of objects depending on qapi/error.h from "all
of them" to less than a third. Unfortunately, the number depending on
qapi-types.h shrinks only a little. More work is needed for that one.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
[Fix compilation without the spice devel packages. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
As soon as setjmp.h is included from qemu/osdep.h, those old include
statements are no longer needed.
Add also setjmp.h to the list in scripts/clean-includes.
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Now that the generator supports it, we might as well use an
anonymous base rather than breaking out a single-use Base
structure, for all three of our current QMP flat unions.
Oddly enough, this change does not affect the resulting
introspection output (because we already inline the members of
a base type into an object, and had no independent use of the
base type reachable from a command).
The case_whitelist now has to list the name of an implicit
type; which is not too bad (consider it a feature if it makes
it harder for developers to make the whitelist grow :)
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1458254921-17042-16-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Rather than requiring all flat unions to explicitly create
a separate base struct, we can allow the qapi schema to specify
the common members via an inline dictionary. This is similar to
how commands can specify an inline anonymous type for its 'data'.
We already have several struct types that only exist to serve as
a single flat union's base; the next commit will clean them up.
In particular, this patch's change to the BlockdevOptions example
in qapi-code-gen.txt will actually be done in the real QAPI schema.
Now that anonymous bases are legal, we need to rework the
flat-union-bad-base negative test (as previously written, it
forms what is now valid QAPI; tweak it to now provide coverage
of a new error message path), and add a positive test in
qapi-schema-test to use an anonymous base (making the integer
argument optional, for even more coverage).
Note that this patch only allows anonymous bases for flat unions;
simple unions are already enough syntactic sugar that we do not
want to burden them further. Meanwhile, while it would be easy
to also allow an anonymous base for structs, that would be quite
redundant, as the members can be put right into the struct
instead.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1458254921-17042-15-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Simple unions were carrying a special case that hid their 'data'
QMP member from the resulting C struct, via the hack method
QAPISchemaObjectTypeVariant.simple_union_type(). But by using
the work we started by unboxing flat union and alternate
branches, coupled with the ability to visit the members of an
implicit type, we can now expose the simple union's implicit
type in qapi-types.h:
| struct q_obj_ImageInfoSpecificQCow2_wrapper {
| ImageInfoSpecificQCow2 *data;
| };
|
| struct q_obj_ImageInfoSpecificVmdk_wrapper {
| ImageInfoSpecificVmdk *data;
| };
...
| struct ImageInfoSpecific {
| ImageInfoSpecificKind type;
| union { /* union tag is @type */
| void *data;
|- ImageInfoSpecificQCow2 *qcow2;
|- ImageInfoSpecificVmdk *vmdk;
|+ q_obj_ImageInfoSpecificQCow2_wrapper qcow2;
|+ q_obj_ImageInfoSpecificVmdk_wrapper vmdk;
| } u;
| };
Doing this removes asymmetry between QAPI's QMP side and its
C side (both sides now expose 'data'), and means that the
treatment of a simple union as sugar for a flat union is now
equivalent in both languages (previously the two approaches used
a different layer of dereferencing, where the simple union could
be converted to a flat union with equivalent C layout but
different {} on the wire, or to an equivalent QMP wire form
but with different C representation). Using the implicit type
also lets us get rid of the simple_union_type() hack.
Of course, now all clients of simple unions have to adjust from
using su->u.member to using su->u.member.data; while this touches
a number of files in the tree, some earlier cleanup patches
helped minimize the change to the initialization of a temporary
variable rather than every single member access. The generated
qapi-visit.c code is also affected by the layout change:
|@@ -7393,10 +7393,10 @@ void visit_type_ImageInfoSpecific_member
| }
| switch (obj->type) {
| case IMAGE_INFO_SPECIFIC_KIND_QCOW2:
|- visit_type_ImageInfoSpecificQCow2(v, "data", &obj->u.qcow2, &err);
|+ visit_type_q_obj_ImageInfoSpecificQCow2_wrapper_members(v, &obj->u.qcow2, &err);
| break;
| case IMAGE_INFO_SPECIFIC_KIND_VMDK:
|- visit_type_ImageInfoSpecificVmdk(v, "data", &obj->u.vmdk, &err);
|+ visit_type_q_obj_ImageInfoSpecificVmdk_wrapper_members(v, &obj->u.vmdk, &err);
| break;
| default:
| abort();
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1458254921-17042-13-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Now that we are always bulk-initializing a QAPI C struct to 0
(whether by g_malloc0() or by 'Type arg = {0};'), we no longer
have any clients of c_null() in the generator for per-element
initialization. This patch is easy enough to revert if we find
a use in the future, but in the present, get rid of the dead code.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1458254921-17042-12-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Commit 82ca8e46 noticed that we had multiple implementations of
visiting every member of a struct, and consolidated it into
gen_visit_fields() (now gen_visit_members()) with enough
parameters to cater to slight differences between the clients.
But recent exposure of implicit types has meant that we are now
down to a single use of that method, so we can clean up the
unused conditionals and just inline it into the remaining
caller: gen_visit_object_members().
Likewise, gen_err_check() no longer needs optional parameters,
as the lone use of non-defaults was via gen_visit_members().
No change to generated code.
Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1458254921-17042-11-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Originally, gen_marshal_input_visit() (or gen_visitor_input_block()
before commit f1538019) was factored out to make it easy to do two
passes of a visit to each member of a (possibly-implicit) object,
without duplicating lots of code. But after recent changes, those
visits now occupy a single line of emitted code, and the helper
method has become a series of conditionals both before and after
the one important line, making it rather awkward to see at a glance
what gets emitted on the first (parsing) or second (deallocation)
pass. It's a lot easier to read the generator code if we just
inline both uses directly into gen_marshal(), without all the
conditionals.
Once we've done that, it's easy to notice that gen_marshal_vars()
is used only once, and inlining it too lets us consolidate some
mcgen() calls that used to be split across helpers.
gen_call() remains a single-use helper function, but it has
enough indentation and complexity that inlining it would hamper
legibility.
No change to generated output. The fact that the diffstat shows
a net reduction in lines is an argument in favor of this cleanup.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1458254921-17042-10-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Rather than generate inline per-member visits, take advantage
of the 'visit_type_FOO_members()' function for command
marshalling. This is possible now that implicit structs can be
visited like any other. Generate call arguments from a stack-
allocated struct, rather than a list of local variables:
|@@ -57,26 +57,15 @@ void qmp_marshal_add_fd(QDict *args, QOb
| QmpInputVisitor *qiv = qmp_input_visitor_new_strict(QOBJECT(args));
| QapiDeallocVisitor *qdv;
| Visitor *v;
|- bool has_fdset_id = false;
|- int64_t fdset_id = 0;
|- bool has_opaque = false;
|- char *opaque = NULL;
|+ q_obj_add_fd_arg arg = {0};
|
| v = qmp_input_get_visitor(qiv);
|- if (visit_optional(v, "fdset-id", &has_fdset_id)) {
|- visit_type_int(v, "fdset-id", &fdset_id, &err);
|- if (err) {
|- goto out;
|- }
|- }
|- if (visit_optional(v, "opaque", &has_opaque)) {
|- visit_type_str(v, "opaque", &opaque, &err);
|- if (err) {
|- goto out;
|- }
|+ visit_type_q_obj_add_fd_arg_members(v, &arg, &err);
|+ if (err) {
|+ goto out;
| }
|
|- retval = qmp_add_fd(has_fdset_id, fdset_id, has_opaque, opaque, &err);
|+ retval = qmp_add_fd(arg.has_fdset_id, arg.fdset_id, arg.has_opaque, arg.opaque, &err);
| if (err) {
| goto out;
| }
|@@ -88,12 +77,7 @@ out:
| qmp_input_visitor_cleanup(qiv);
| qdv = qapi_dealloc_visitor_new();
| v = qapi_dealloc_get_visitor(qdv);
|- if (visit_optional(v, "fdset-id", &has_fdset_id)) {
|- visit_type_int(v, "fdset-id", &fdset_id, NULL);
|- }
|- if (visit_optional(v, "opaque", &has_opaque)) {
|- visit_type_str(v, "opaque", &opaque, NULL);
|- }
|+ visit_type_q_obj_add_fd_arg_members(v, &arg, NULL);
| qapi_dealloc_visitor_cleanup(qdv);
| }
This also has the nice side effect of eliminating a chance of
collision between argument QMP names and local variables.
This patch also paves the way for some followup simplifications
in the generator, in subsequent patches.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1458254921-17042-9-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Rather than generate inline per-member visits, take advantage
of the 'visit_type_FOO_members()' function for emitting events.
This is possible now that implicit structs can be visited like
any other. Generated code shrinks accordingly; by initializing
a struct based on parameters, through a new gen_param_var()
helper, like:
|@@ -338,6 +250,9 @@ void qapi_event_send_block_job_error(con
| QMPEventFuncEmit emit = qmp_event_get_func_emit();
| QmpOutputVisitor *qov;
| Visitor *v;
|+ q_obj_BLOCK_JOB_ERROR_arg param = {
|+ (char *)device, operation, action
|+ };
|
| if (!emit) {
| return;
@@ -351,19 +266,7 @@ void qapi_event_send_block_job_error(con
| if (err) {
| goto out;
| }
|- visit_type_str(v, "device", (char **)&device, &err);
|- if (err) {
|- goto out_obj;
|- }
|- visit_type_IoOperationType(v, "operation", &operation, &err);
|- if (err) {
|- goto out_obj;
|- }
|- visit_type_BlockErrorAction(v, "action", &action, &err);
|- if (err) {
|- goto out_obj;
|- }
|-out_obj:
|+ visit_type_q_obj_BLOCK_JOB_ERROR_arg_members(v, ¶m, &err);
| visit_end_struct(v, err ? NULL : &err);
Notice that the initialization of 'param' has to cast away const
(just as the old gen_visit_members() had to do): we can't change
the signature of the user function (which uses 'const char *'), but
have to assign it to a non-const QAPI object (which requires
'char *').
While touching this, document with a FIXME comment that there is
still a potential collision between QMP members and our choice of
local variable names within qapi_event_send_FOO().
This patch also paves the way for some followup simplifications
in the generator, in subsequent patches.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1458254921-17042-8-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
We already have several places that want to visit all the members
of an implicit object within a larger context (simple union variant,
event with anonymous data, command with anonymous arguments struct);
and will be adding another one soon (the ability to declare an
anonymous base for a flat union). Having a C struct declared for
these implicit types, along with a visit_type_FOO_members() helper
function, will make for fewer special cases in our generator.
We do not, however, need qapi_free_FOO() or visit_type_FOO()
functions for implicit types, because they should not be used
directly outside of the generated code. This is done by adding a
conditional in visit_object_type() for both qapi-types.py and
qapi-visit.py based on the object name. The comparison of
"name.startswith('q_')" is a bit hacky (it's basically duplicating
what .is_implicit() already uses), but beats changing the signature
of the visit_object_type() callback to pass a new 'implicit' flag.
The hack should be temporary: we are considering adding a future
patch that consolidates the narrow visit_object_type(..., base,
local_members, variants) and visit_object_type_flat(...,
all_members, variants) [where different sets of information are
already broken out, and the QAPISchemaObjectType is no longer
available] into a broader visit_object_type(obj_type) [where the
visitor can query the needed fields from obj_type directly].
Also, now that we WANT to output C code for implicits, we no longer
need the visit_needed() filter, leaving 'q_empty' as the only object
still needing a special case. Remember, 'q_empty' is the only
built-in generated object, which means that without a special case
it would be emitted in multiple files (the main qapi-types.h and in
qga-qapi-types.h) causing compilation failure due to redefinition.
But since it has no members, it's easier to just avoid an attempt to
visit that particular type; since gen_object() is called recursively,
we also prime the objects_seen set to cover any recursion into the
empty type.
The patch relies on the changed naming of implicit types in the
previous patch. It is a bit unfortunate that the generated struct
names and visit_type_FOO_members() don't match normal naming
conventions, but it's not too bad, since they will only be used in
generated code.
The generated code grows substantially in size: the implicit
'-wrapper' types must be emitted in qapi-types.h before any union
can include an unboxed member of that type. Arguably, the '-args'
types could be emitted in a private header for just qapi-visit.c
and qmp-marshal.c, rather than polluting qapi-types.h; but adding
complexity to the generator to split the output location according
to role doesn't seem worth the maintenance costs.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1458254921-17042-6-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
The original choice of ':obj-' as the prefix for implicit types
made it obvious that we weren't going to clash with any user-defined
names, which cannot contain ':'. But now we want to create structs
for implicit types, to get rid of special cases in the generators,
and our use of ':' in implicit names needs a tweak to produce valid
C code.
We could transliterate ':' to '_', except that C99 mandates that
"identifiers that begin with an underscore are always reserved for
use as identifiers with file scope in both the ordinary and tag name
spaces". So it's time to change our naming convention: we can
instead use the 'q_' prefix that we reserved for ourselves back in
commit 9fb081e0. Technically, since we aren't planning on exposing
the empty type in generated code, we could keep the name ':empty',
but renaming it to 'q_empty' makes the check for startswith('q_')
cover all implicit types, whether or not code is generated for them.
As long as we don't declare 'empty' or 'obj' ticklish, it shouldn't
clash with c_name() prepending 'q_' to the user's ticklish names.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1458254921-17042-5-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
QAPISchemaType.c_type() is a bit awkward: it takes two optional
boolean flags is_param and is_unboxed, and they should never both
be True.
Add a new method for each of the flags, and drop the flags from
c_type().
Most callers pass no flags; they remain unchanged.
One caller passes is_param=True; call the new .c_param_type()
instead.
One caller passes is_unboxed=True, except for simple union types.
This is actually an ugly special case that will go away soon, so
until then, we now have to call either .c_type() or the new
.c_unboxed_type(). Tolerable in the interim.
It requires slightly more Python, but is arguably easier to read.
Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1458254921-17042-4-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
The generator special-cased
{ 'command':'foo', 'data': {} }
to avoid emitting a visitor variable, but failed to see that
{ 'struct':'NamedEmptyType, 'data': {} }
{ 'command':'foo', 'data':'NamedEmptyType' }
needs the same treatment. There, the generator happily generates a
visitor to get no arguments, and a visitor to destroy no arguments;
and the compiler isn't happy with that, as demonstrated by the updated
qapi-schema-test.json:
tests/test-qmp-marshal.c: In function ‘qmp_marshal_user_def_cmd0’:
tests/test-qmp-marshal.c:264:14: error: variable ‘v’ set but not used [-Werror=unused-but-set-variable]
Visitor *v;
^
No change to generated code except for the testsuite addition.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1458254921-17042-3-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
We are getting closer to the point where we could use one union
as the base or variant type within another union type (as long
as there are no collisions between any possible combination of
member names allowed across all discriminator choices). But
until we get to that point, it is worth asserting that variants
are not present in places where we are not prepared to handle
them: when exploding a type into a parameter list, we do not
expect variants. The qapi.py code is already checking this,
via the older check_type() method; but someday we hope to get
rid of that and move checking into QAPISchema*.check(). The
two asserts added here make sure any refactoring still catches
problems, and makes it locally obvious why we can iterate over
only type.members without worrying about type.variants.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1458254921-17042-2-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
We have a fake linux/types.h which we create in update-linux-headers.h.
Now that every QEMU source file includes osdep.h, this fake header
doesn't need to include anything at all.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-id: 1456237112-32662-4-git-send-email-peter.maydell@linaro.org
include/config.h just includes config-target.h (and used to also
include config-host.h).
It is now obsolete and unused, because osdep.h does this job, so
remove it.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-id: 1456237112-32662-3-git-send-email-peter.maydell@linaro.org
userfailtfd.h is used by post-copy migration so include it to
the update-linux-headers.sh as we want it updated altogether with
other kernel headers.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Message-Id: <1455512381-15271-1-git-send-email-aik@ozlabs.ru>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
All references to mr->ram_addr are replaced by
memory_region_get_ram_addr(mr) (except for a few assertions that are
replaced with mr->ram_block).
Reviewed-by: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
Message-Id: <1456813104-25902-5-git-send-email-famz@redhat.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
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>
