aboutsummaryrefslogtreecommitdiff
path: root/scripts
AgeCommit message (Collapse)Author
2016-03-05qapi: Drop useless 'data' member of unionsEric Blake
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>
2016-03-05qapi-visit: Expose visit_type_FOO_members()Eric Blake
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>
2016-03-05qapi: Rename 'fields' to 'members' in generated C codeEric Blake
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>
2016-03-05qapi: Rename 'fields' to 'members' in generatorEric Blake
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>
2016-03-04qmp-shell: fix pretty printing of JSON responsesDaniel P. Berrange
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 1ceca07e48ead0dd2e41576c81d40e6a91cafefd 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>
2016-03-01trace: Add a proper API to manage auto-generated events from the 'tcg' propertyLluís Vilanova
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>
2016-03-01trace: Add 'vcpu' event property to trace guest vCPULluís Vilanova
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>
2016-03-01trace: Add helper function to cast event argumentsLluís Vilanova
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu> Message-id: 145641860680.30295.1873612736245870753.stgit@localhost Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2016-03-01trace: Remove unnecessary intermediate event copiesLluís Vilanova
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>
2016-03-01trace: Extend API to manage event argumentsLluís Vilanova
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>
2016-03-01qapi: rename InputAxis values.Gerd Hoffmann
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>
2016-03-01qapi: rename input buttonsGerd Hoffmann
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>
2016-02-25Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into stagingPeter Maydell
* Asynchronous dump-guest-memory from Peter * improved logging with -D -daemonize from Dimitris * more address_space_* optimization from Gonglei * TCG xsave/xrstor thinko fix * chardev bugfix and documentation patch # gpg: Signature made Thu 25 Feb 2016 15:12:27 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: target-i386: fix confusion in xcr0 bit position vs. mask chardev: Properly initialize ChardevCommon components memory: Remove unreachable return statement memory: optimize qemu_get_ram_ptr and qemu_ram_ptr_length exec: store RAMBlock pointer into memory region log: Redirect stderr to logfile if deamonized dump-guest-memory: add qmp event DUMP_COMPLETED Dump: add hmp command "info dump" Dump: add qmp command "query-dump" DumpState: adding total_size and written_size fields dump-guest-memory: add "detach" support dump-guest-memory: disable dump when in INMIGRATE state dump-guest-memory: introduce dump_process() helper function. dump-guest-memory: add dump_in_progress() helper function dump-guest-memory: using static DumpState, add DumpStatus dump-guest-memory: add "detach" flag for QMP/HMP interfaces. dump-guest-memory: cleanup: removing dump_{error|cleanup}(). scripts/kvm/kvm_stat: Fix missing right parantheses and ".format(...)" qemu-options.hx: Improve documentation of chardev multiplexing mode Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2016-02-23Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20160223' into stagingPeter Maydell
Queued TCG patches # gpg: Signature made Tue 23 Feb 2016 18:27:44 GMT using RSA key ID 4DD0279B # gpg: Good signature from "Richard Henderson <rth7680@gmail.com>" # gpg: aka "Richard Henderson <rth@redhat.com>" # gpg: aka "Richard Henderson <rth@twiddle.net>" * remotes/rth/tags/pull-tcg-20160223: tcg: Remove unnecessary osdep.h includes from tcg-target.inc.c scripts/clean-includes: Ignore .inc.c files tcg: Rename tcg-target.c to tcg-target.inc.c target-sparc: Use global registers for the register window target-sparc: Tidy global register initialization tcg: Allocate indirect_base temporaries in a different order tcg: Implement indirect memory registers tcg: Work around clang bug wrt enum ranges, part 2 Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2016-02-23scripts/clean-includes: Ignore .inc.c filesPeter Maydell
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>
2016-02-23tracetool: Include osdep.h in generated-ust.cPeter Maydell
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>
2016-02-23scripts/clean-includes: Add --all optionPeter Maydell
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>
2016-02-23scripts/clean-includes: Enhance to handle header filesPeter Maydell
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>
2016-02-22scripts/kvm/kvm_stat: Fix missing right parantheses and ".format(...)"Fam Zheng
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>
2016-02-19qapi: Change visit_start_implicit_struct to visit_start_alternateEric Blake
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>
2016-02-19qapi: Don't box branches of flat unionsEric Blake
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>
2016-02-19qapi: Don't box struct branch of alternateEric Blake
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>
2016-02-19qapi-visit: Use common idiom in gen_visit_fields_decl()Eric Blake
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>
2016-02-19qapi: Emit structs used as variants in topological orderEric Blake
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>
2016-02-19qapi: Adjust layout of FooList typesEric Blake
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>
2016-02-19qapi-visit: Less indirection in visit_type_Foo_fields()Eric Blake
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>
2016-02-19qapi-visit: Unify struct and union visitMarkus Armbruster
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>
2016-02-19qapi: Visit variants in visit_type_FOO_fields()Eric Blake
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>
2016-02-19qapi-visit: Simplify how we visit common union membersMarkus Armbruster
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]
2016-02-19qapi: Forbid 'any' inside an alternateEric Blake
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>
2016-02-19qapi: Forbid empty unions and useless alternatesEric Blake
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>
2016-02-19qapi-visit: Honor prefix of discriminator enumEric Blake
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>
2016-02-16Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into stagingPeter Maydell
* 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>
2016-02-16scripts/tracetool: Include qemu/osdep.h in generated .c filesPeter Maydell
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>
2016-02-16scripts/feature_to_c.sh: Include qemu/osdep.h rather than config.hPeter Maydell
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>
2016-02-16qapi: Clean up includes in generated filesEric Blake
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>
2016-02-15checkpatch: Eliminate false positive in case of space before square bracket ↵Leonid Bloch
in a definition 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>
2016-02-15checkpatch: Eliminate false positive in case of comma-space-square bracketLeonid Bloch
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>
2016-02-09get_maintainer.pl: fall back to git if only lists are foundPaolo Bonzini
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>
2016-02-09scripts/kvm/kvm_stat: Fix tracefs access checkingJanosch Frank
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>
2016-02-08qapi: Fix compilation failure on MIPS and SPARCEric Blake
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>
2016-02-08qapi: Drop unused error argument for list and implicit structEric Blake
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>
2016-02-08qapi: Drop unused 'kind' for struct/enum visitEric Blake
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>
2016-02-08qapi: Swap visit_* arguments for consistent 'name' placementEric Blake
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>
2016-02-08qapi: Don't cast Enum* to int*Eric Blake
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>
2016-02-08qapi-visit: Kill unused visit_end_union()Eric Blake
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>
2016-02-08qapi: Track all failures between visit_start/stopEric Blake
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>
2016-02-08qapi: Improve generated event use of qapi visitorEric Blake
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>
2016-02-08qapi: Drop dead parameter in gen_params()Eric Blake
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>
2016-02-08Revert "tracetool: use Python 2.4-compatible exception handling syntax"Markus Armbruster
This reverts commit 662da3854e3f490223373b40afdcfcc339d14aa5. 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>