summaryrefslogtreecommitdiff
path: root/scripts
AgeCommit message (Collapse)AuthorFilesLines
2016-01-26scripts/kvm/kvm_stat: Encapsulate filters variableJanosch Frank1-9/+14
The variable was only used in one class but still was defined globally. Additionaly the detect_platform routine which prepares the data that goes into the variable was called on each start of the script, no matter if the class was needed. To make the variable local to the TracepointProvider class, a new function that calls detect_platform and returns the filters was introduced. Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com> Message-Id: <1452525484-32309-22-git-send-email-frankja@linux.vnet.ibm.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-01-26scripts/kvm/kvm_stat: Cleanup cpu list retrievalJanosch Frank1-11/+20
Reading /sys/devices/system/cpu/online makes opening the cpu directories unnecessary and works on more/older systems. Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com> Message-Id: <1452525484-32309-21-git-send-email-frankja@linux.vnet.ibm.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-01-26scripts/kvm/kvm_stat: Cleanup of TracepointProviderJanosch Frank1-19/+21
Variables with bad names like f and m were renamed to their full name, so it is clearer which data they contain. Unneeded variables were removed and the field generating code was moved in an own function. dict.iteritems() was removed as directly iterating over a dictionary also yields the needed keys. Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com> Message-Id: <1452525484-32309-20-git-send-email-frankja@linux.vnet.ibm.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-01-26scripts/kvm/kvm_stat: Introduce properties for providersJanosch Frank1-8/+19
As previous commit authors used a mixture of setters/getters and direct access to class variables consolidating them the python way improved readability. Properties allow us to assign a value to a class variable through a setter without the need to call the setter ourselves. Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com> Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com> Message-Id: <1452525484-32309-19-git-send-email-frankja@linux.vnet.ibm.com> [prop.setter is new in Python 2.6, which is the earliest supported version. - Paolo] Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-01-26scripts/kvm/kvm_stat: Rename _perf_event_openJanosch Frank1-2/+2
The underscore in front of the function name does not comply with the python coding guidelines. Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com> Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com> Message-Id: <1452525484-32309-18-git-send-email-frankja@linux.vnet.ibm.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-01-26scripts/kvm/kvm_stat: Make cpu detection a functionJanosch Frank1-15/+15
The online cpus detection method is in the Stats class but does not use any class variables. Moving it out of the class to the platform detection function makes the Stats class more readable. Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com> Message-Id: <1452525484-32309-17-git-send-email-frankja@linux.vnet.ibm.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-01-26scripts/kvm/kvm_stat: Cleanup of platform detectionJanosch Frank1-16/+13
s390 machines can also be detected via uname -m, i.e. python's os.uname, no need for more complicated checks. Calling uname once and saving its value for multiple checks is perfectly sufficient. We don't expect the machine's architecture to change when the script is running anyway. On multi-cpu systems x86_init currently will get called multiple times, returning makes sure we don't waste cicles on that. Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com> Message-Id: <1452525484-32309-16-git-send-email-frankja@linux.vnet.ibm.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-01-26scripts/kvm/kvm_stat: Set sensible no. files rlimitJanosch Frank1-2/+9
As num cpus * 1000 is NOT a sensible rlimit, we need to calculate a more accurate rlimit. The number of open files is directly dependent on the cpu count and on the number of trace points per cpu. A additional constant works as a buffer for files that are needed by python or do get opened when the script runs. Hence we have: cpus * traces + constant Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com> Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com> Message-Id: <1452525484-32309-15-git-send-email-frankja@linux.vnet.ibm.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-01-26scripts/kvm/kvm_stat: Fixup syscall error reportingJanosch Frank1-5/+4
In 2008 a patch was written that introduced ctypes.get_errno() and set_errno() as official interfaces to the libc errno variable. Using them we can avoid accessing private libc variables. The patch was included in python 2.6. Also we need to raise the right exception, with the right parameters and a helpful message. Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com> Message-Id: <1452525484-32309-14-git-send-email-frankja@linux.vnet.ibm.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-01-26scripts/kvm/kvm_stat: Moved DebugfsProviderJanosch Frank1-12/+12
When it is next to the TracepointProvider less scrolling is needed to change related, surrounding code. Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com> Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com> Message-Id: <1452525484-32309-13-git-send-email-frankja@linux.vnet.ibm.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-01-26scripts/kvm/kvm_stat: Rename variables that redefine globalsJanosch Frank1-11/+12
Filter, id and byte are builtin python modules which should not be redefined by local variables. Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com> Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com> Message-Id: <1452525484-32309-12-git-send-email-frankja@linux.vnet.ibm.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-01-26scripts/kvm/kvm_stat: Fix spaces around keyword assignmentsJanosch Frank1-31/+31
Keyword assignments should not not have spaces around the equal character according to PEP8. Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com> Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com> Message-Id: <1452525484-32309-11-git-send-email-frankja@linux.vnet.ibm.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-01-26scripts/kvm/kvm_stat: Introduce main functionJanosch Frank1-71/+82
The main function should be the main location for initialization and helps encapsulating variables into a scope. This way they don't have to be global and might be mistaken for local ones. As the providers variable is scoped now it can't be accessed from within the Stats class. Hence, the global access to the variable was changed to a local one. Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com> Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com> Message-Id: <1452525484-32309-10-git-send-email-frankja@linux.vnet.ibm.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-01-26scripts/kvm/kvm_stat: Improve debugfs access checkingJanosch Frank1-5/+11
Access checking with F_OK was replaced with the better readable os.path.exists(). On Linux exists() returns False when the user doesn't have sufficient permissions for statting the directory. Therefore the error message now states that sufficient rights are needed when the check fails. Also added check for /sys/kernel/debug/tracing/. Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com> Message-Id: <1452525484-32309-9-git-send-email-frankja@linux.vnet.ibm.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-01-26scripts/kvm/kvm_stat: Cleanup of path variablesJanosch Frank1-7/+7
Paths to debugfs and trace dirs are now specified globally to remove redundancies in the code. Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com> Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com> Message-Id: <1452525484-32309-8-git-send-email-frankja@linux.vnet.ibm.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-01-26scripts/kvm/kvm_stat: Invert dictionariesJanosch Frank1-176/+173
The exit reasons dictionaries were defined number -> value but later on were accessed the other way around. Therefore a invert function inverted them. Defining them the right way removes the need to invert them and therefore also speeds up the script's setup process. Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com> Message-Id: <1452525484-32309-7-git-send-email-frankja@linux.vnet.ibm.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-01-26scripts/kvm/kvm_stat: Mark globals in functionsJanosch Frank1-19/+22
Updating globals over the globals().update() method is not the standard way of changing globals. Marking variables as global and modifying them the standard way is better readable. Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com> Message-Id: <1452525484-32309-6-git-send-email-frankja@linux.vnet.ibm.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-01-26scripts/kvm/kvm_stat: Removed unneeded PERF constantsJanosch Frank1-26/+2
Only two of the constants are actually needed to set up the events, so the others were removed. All variables that used them were also removed. Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com> Message-Id: <1452525484-32309-5-git-send-email-frankja@linux.vnet.ibm.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-01-26scripts/kvm/kvm_stat: Make constants uppercaseJanosch Frank1-32/+32
Constants should be uppercase with separating underscores, as requested in PEP8. This helps identifying them when reading the code. Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com> Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com> Message-Id: <1452525484-32309-4-git-send-email-frankja@linux.vnet.ibm.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-01-26scripts/kvm/kvm_stat: Replaced os.listdir with os.walkJanosch Frank1-4/+11
Os.walk gives back lists of directories and files, no need to filter directories from the list that listdir gives back. To make it better understandable a wrapper with docstring was introduced. Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com> Message-Id: <1452525484-32309-3-git-send-email-frankja@linux.vnet.ibm.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-01-26scripts/kvm/kvm_stat: Cleanup of multiple importsJanosch Frank1-15/+11
Removed multiple imports of the same module and moved all imports to the top. It is not necessary to import a module each time one of its functions/classes is used. For readability each import should get its own line. Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com> Message-Id: <1452525484-32309-2-git-send-email-frankja@linux.vnet.ibm.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-01-18scripts: Add new clean-includes script to fix C include directivesPeter Maydell1-0/+109
Add a new scripts/clean-includes, which can be used to automatically ensure that a C source file includes qemu/osdep.h first and doesn't then include any headers which osdep.h provides already. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Daniel P. Berrange <berrange@redhat.com> Message-id: 1449505425-32022-2-git-send-email-peter.maydell@linaro.org
2016-01-13checkpatch: Detect newlines in error_report and other error functionsJason J. Herne1-0/+36
We don't want newlines embedded in error messages. This seems to be a common problem with new code so let's try to catch it with checkpatch. This will not catch cases where newlines are inserted into the middle of an existing multi-line statement. But those cases should be rare. Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com> Message-Id: <1449858642-24267-1-git-send-email-jjherne@linux.vnet.ibm.com> [Rephrased "Error function text" to "Error messages", dropped error_vprintf, error_printf, error_printf from $qemu_error_funcs, because they may legitimately print newlines] Signed-off-by: Markus Armbruster <armbru@redhat.com>
2016-01-11Merge remote-tracking branch ↵Peter Maydell1-13/+0
'remotes/mjt/tags/pull-trivial-patches-2016-01-11' into staging trivial patches for 2016-01-11 # gpg: Signature made Mon 11 Jan 2016 08:39:32 GMT using RSA key ID A4C3D7DB # gpg: Good signature from "Michael Tokarev <mjt@tls.msk.ru>" # gpg: aka "Michael Tokarev <mjt@corpit.ru>" # gpg: aka "Michael Tokarev <mjt@debian.org>" * remotes/mjt/tags/pull-trivial-patches-2016-01-11: hw/s390x: Remove superfluous return statements hw/core/qdev: Remove superfluous return statement hw/acpi: Remove superfluous return statement hw/ide: Remove superfluous return statements osdep.h: Include glib-compat.h in osdep.h rather than qemu-common.h scripts/checkpatch.pl: Don't allow special cases of unspaced operators PCI Bonito: QOMify and cleanup SH PCI Host: convert to realize() gt64120: convert to realize() Add missing syscall nrs. according to more recent Linux kernels hw/misc/edu: Convert to realize() configure: fix trace backend check xen/Makefile.objs: simplify crypto: Fix typo in example MAINTAINERS: Add the correct device_tree.h file iscsi: fix readcapacity error message net: convert qemu_log to error_report, fix message linux-user: enable sigaltstack for all architectures unicore32: convert get_sp_from_cpustate from macro to inline Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2016-01-11scripts/checkpatch.pl: Don't allow special cases of unspaced operatorsPeter Maydell1-13/+0
The checkpatch.pl script has a special case to permit the following operators to have no spaces around them: << >> & ^ | + - * / % QEMU style prefers all operators to consistently have spacing around them, so remove this special case handling. This avoids reviewers having to manually note it during code review. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Stefan Weil <sw@weilnetz.de> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
2016-01-09pc: acpi: remove unused ASL templates and related blobs/utilsIgor Mammedov3-422/+0
QEMU now uses internally composed DSDT so drop now empty *.dsl templates and related *.generated binary blobs. Also since templates are not used anymore/obolete remove utility scripts used for extracting/patching AML blobs compiled by IASL and for updating them in git tree. Signed-off-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
2015-12-22scripts/gdb: Fix a python exception in mtree.pyYang Wei1-5/+5
The following exception is threw: Python Exception <class 'NameError'> name 'long' is not defined: Error occurred in Python command: name 'long' is not defined Python 2.4+, int()/long() have been unified, so replace long with int. Signed-off-by: Yang Wei <w90p710@gmail.com> Message-id: 1449316340-4030-1-git-send-email-w90p710@gmail.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2015-12-18io: add QIOChannelSocket classDaniel P. Berrange1-0/+9
Implement a QIOChannel subclass that supports sockets I/O. The implementation is able to manage a single socket file descriptor, whether a TCP/UNIX listener, TCP/UNIX connection, or a UDP datagram. It provides APIs which can listen and connect either asynchronously or synchronously. Since there is no asynchronous DNS lookup API available, it uses the QIOTask helper for spawning a background thread to ensure non-blocking operation. Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2015-12-17coverity: Model g_memdup()Markus Armbruster1-0/+17
We model all the non-deprecated memory allocation functions from https://developer.gnome.org/glib/stable/glib-Memory-Allocation.html except for g_memdup(), g_clear_pointer(), g_steal_pointer(). We don't use the latter two. Model the former. Coverity now reports an OVERRUN vl.c:2317: alloc_strlen: Allocating insufficient memory for the terminating null of the string. Correct, but we omit the terminating null intentionally there. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <1448901152-11716-1-git-send-email-armbru@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2015-12-17coverity: Model g_poll()Markus Armbruster1-0/+9
In my testing, Coverity reported two more CHECKED_RETURN: * qemu-char.c:1248: fixed in commit c1f2448: "qemu-char: retry g_poll on EINTR". * migration/qemu-file-unix.c:75: harmless, cleaned up in commit 4e39f57 "migration: Clean up use of g_poll() in socket_writev_buffer() Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <1450336833-27710-1-git-send-email-armbru@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2015-12-17linux-headers: update from kvm/nextPaolo Bonzini1-0/+3
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2015-12-17qapi: Detect base class loopsEric Blake1-1/+3
It should be fairly obvious that qapi base classes need to form an acyclic graph, since QMP cannot specify the same key more than once, while base classes are included as flat members alongside other members added by the child. But the old check_member_clash() parser function was not prepared to check for this, and entered an infinite recursion (at least until Python gives up, complaining about nesting too deep). Now that check_member_clash() has been recently removed, attempts at self-inheritance trigger an assertion failure introduced by commit ac88219a. The obvious fix is to turn the assertion into a conditional. This patch includes both the tests (base-cycle-direct and base-cycle-indirect) and the fix, since the .err file output for the unfixed case is not useful (particularly when it was warning about unbounded recursion, as that limit may be platform-specific). We don't need to worry about cycles in flat unions (neither the base type nor the type of a variant can be a union) nor in alternates (alternate branches cannot themselves be an alternate). But if we later allow a union type as a variant, we will still be okay, as QAPISchemaObjectTypeVariants.check() triggers the same QAPISchemaObjectType.check() that will detect any loops. Likewise, we need not worry about the case of diamond inheritance where the same class is used for a flat union base class and one of its variants; either both uses will introduce a collision in trying to insert the same member name twice, or the shared type is empty and changes nothing. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1449033659-25497-16-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Move duplicate collision checks to schema check()Eric Blake1-50/+1
With the recent commit 'qapi: Detect collisions in C member names', we have two different locations for detecting clashes - one at parse time, and another at QAPISchema*.check() time. Remove all of the ad hoc parser checks, and delete associated code (for example, the global check_member_clash() method is no longer needed). Testing this showed that the test union-bad-branch wasn't adding much: union-clash-branches also exposes the error message when branches collide, and we've recently fixed things to avoid an implicit collision with max. Likewise, the error for enum-clash-member changes to report our new detection of upper case in a value name, unless we modify the test to use all lower case. The wording of several error messages has changed, but the change is generally an improvement rather than a regression. No change to generated code. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1449033659-25497-15-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Enforce (or whitelist) case conventions on qapi membersEric Blake1-0/+17
We document that members of enums and objects should be 'lower-case', although we were not enforcing it. We have to whitelist a few pre-existing entities that violate the norms. Add three new tests to expose the new error message, each of which first uses the whitelisted name 'UuidInfo' to prove the whitelist works, then triggers the failure (this is the same pattern used in the existing returns-whitelist.json test). Note that by adding this check, we have effectively forbidden an entity with a case-insensitive clash of member names, for any entity that is not on the whitelist (although there is still the possibility to clash via '-' vs. '_'). Not done here: a future patch should also add naming convention support and whitelist exceptions for command, event, and type names. The additions to QAPISchemaMember.check_clash() check whether info['name'] is in the whitelist (the top-most entity name at the point 'info' tracks), rather than self.owner (the type, possibly implicit, that directly owns the member), because it is easier to maintain the whitelist by the names actually in the user's .json file, rather than worrying about the names of implicit types. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1449033659-25497-14-git-send-email-eblake@redhat.com> [Simplified a bit as per discussion with Eric] Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Track enum values by QAPISchemaMember, not stringEric Blake1-11/+26
Rather than using just an array of strings, make enum.values be an array of the new QAPISchemaMember type, and add a helper member_names() method to get back at the original list of names. Likewise, creating an enum requires wrapping strings, via a new QAPISchema._make_enum_members() method. The benefit of wrapping enum members in a QAPISchemaMember Python object is that we now share the existing code for C name clash detection (although the code is not yet active until a later commit removes the earlier ad hoc parser checks). In a related change, the QAPISchemaMember._pretty_owner() method needs to learn about one more implicit type name: the generated enum associated with a simple union. In the interest of keeping the changes of this patch local to one file, the visitor interface still passes just a list of names rather than the full list of QAPISchemaMember instances. We may want to revisit this in the future, if the consistency with visit_object_type() is worth it. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1449033659-25497-12-git-send-email-eblake@redhat.com> [Eric's simplifying followup squashed in] Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Prepare new QAPISchemaMember base classEric Blake1-12/+17
We want to share some clash detection code between enum values and object type members. To assist with that, split off part of QAPISchemaObjectTypeMember into a new base class QAPISchemaMember that tracks name, owner, and common clash detection code; while the former keeps the additional fields for type and optional flag. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1449033659-25497-11-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Shorter visits of optional fieldsEric Blake1-2/+1
For less code, reflect the determined boolean value of an optional visit back to the caller instead of making the caller read the boolean after the fact. The resulting generated code has the following diff: |- visit_optional(v, &has_fdset_id, "fdset-id"); |- if (has_fdset_id) { |+ if (visit_optional(v, &has_fdset_id, "fdset-id")) { | visit_type_int(v, &fdset_id, "fdset-id", &err); | if (err) { | goto out; | } | } Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1449033659-25497-10-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Simplify visits of optional fieldsEric Blake1-6/+2
None of the visitor callbacks would set an error when testing if an optional field was present; make this part of the interface contract by eliminating the errp argument. The resulting generated code has a nice diff: |- visit_optional(v, &has_fdset_id, "fdset-id", &err); |- if (err) { |- goto out; |- } |+ visit_optional(v, &has_fdset_id, "fdset-id"); | if (has_fdset_id) { | visit_type_int(v, &fdset_id, "fdset-id", &err); | if (err) { | goto out; | } | } Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1449033659-25497-9-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Fix alternates that accept 'number' but not 'int'Eric Blake1-4/+7
The QMP input visitor allows integral values to be assigned by promotion to a QTYPE_QFLOAT. However, when parsing an alternate, we did not take this into account, such that an alternate that accepts 'number' and some other type, but not 'int', would reject integral values. With this patch, we now have the following desirable table: alternate has case selected for 'int' 'number' QTYPE_QINT QTYPE_QFLOAT no no error error no yes 'number' 'number' yes no 'int' error yes yes 'int' 'number' While it is unlikely that we will ever use 'number' in an alternate other than in the testsuite, it never hurts to be more precise in what we allow. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1449033659-25497-8-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Inline _make_implicit_tag()Eric Blake1-6/+3
Now that alternates no longer use an implicit tag, we can inline _make_implicit_tag() into its one caller, _def_union_type(). No change to generated code. Suggested-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1449033659-25497-7-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi-types: Drop unnedeed ._fwdefnEric Blake1-5/+1
Previously, the generated code in qapi-types.c initialized all enum lookup tables first, prior to any other definitions. But there are no topological sorting requirements that mandate this layout, so we can drop the QAPISchemaGenTypeVisitor._fwdefn field and just generate all definitions in visitation order. The generated code shows some churn due to reordering, but it is still fairly straightforward to follow (all the deletions occur in one hunk, and all the deleted lines are re-inserted in the same order later in the same files, just spread across multiple insertion points). Suggested-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1449033659-25497-6-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Simplify visiting of alternate typesEric Blake3-44/+23
Previously, working with alternates required two lookup arrays and some indirection: for type Foo, we created Foo_qtypes[] which maps each qtype to a value of the generated FooKind enum, then look up that value in FooKind_lookup[] like we do for other union types. This has a couple of subtle bugs. First, the generator was creating a call with a parameter '(int *) &(*obj)->type' where type is an enum type; this is unsafe if the compiler chooses to store the enum type in a different size than int, where assigning through the wrong size pointer can corrupt data or cause a SIGBUS. Related bug, not not fixed in this patch: qapi-visit.py's gen_visit_enum() generates a cast of its enum * argument to int *. Marked FIXME. Second, since the values of the FooKind enum start at zero, all entries of the Foo_qtypes[] array that were not explicitly initialized will map to the same branch of the union as the first member of the alternate, rather than triggering a desired failure in visit_get_next_type(). Fortunately, the bug seldom bites; the very next thing the input visitor does is try to parse the incoming JSON with the wrong parser, which normally fails; the output visitor is not used with a C struct in that state, and the dealloc visitor has nothing to clean up (so there is no leak). However, the second bug IS observable in one case: parsing an integer causes unusual behavior in an alternate that contains at least a 'number' member but no 'int' member, because the 'number' parser accepts QTYPE_QINT in addition to the expected QTYPE_QFLOAT (that is, since 'int' is not a member, the type QTYPE_QINT accidentally maps to FooKind 0; if this enum value is the 'number' branch the integer parses successfully, but if the 'number' branch is not first, some other branch tries to parse the integer and rejects it). A later patch will worry about fixing alternates to always parse all inputs that a non-alternate 'number' would accept, for now this is still marked FIXME in the updated test-qmp-input-visitor.c, to merely point out that new undesired behavior of 'ans' matches the existing undesired behavior of 'asn'. This patch fixes the default-initialization bug by deleting the indirection, and modifying get_next_type() to directly assign a QTypeCode parameter. This in turn fixes the type-casting bug, as we are no longer casting a pointer to enum to a questionable size. There is no longer a need to generate an implicit FooKind enum associated with the alternate type (since the QMP wire format never uses the stringized counterparts of the C union member names). Since the updated visit_get_next_type() does not know which qtypes are expected, the generated visitor is modified to generate an error statement if an unexpected type is encountered. Callers now have to know the QTYPE_* mapping when looking at the discriminator; but so far, only the testsuite was even using the C struct of an alternate types. I considered the possibility of keeping the internal enum FooKind, but initialized differently than most generated arrays, as in: typedef enum FooKind { FOO_KIND_A = QTYPE_QDICT, FOO_KIND_B = QTYPE_QINT, } FooKind; to create nicer aliases for knowing when to use foo->a or foo->b when inspecting foo->type; but it turned out to add too much complexity, especially without a client. There is a user-visible side effect to this change, but I consider it to be an improvement. Previously, the invalid QMP command: {"execute":"blockdev-add", "arguments":{"options": {"driver":"raw", "id":"a", "file":true}}} failed with: {"error": {"class": "GenericError", "desc": "Invalid parameter type for 'file', expected: QDict"}} (visit_get_next_type() succeeded, and the error comes from the visit_type_BlockdevOptions() expecting {}; there is no mention of the fact that a string would also work). Now it fails with: {"error": {"class": "GenericError", "desc": "Invalid parameter type for 'file', expected: BlockdevRef"}} (the error when the next type doesn't match any expected types for the overall alternate). Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1449033659-25497-5-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Convert QType into QAPI built-in enum typeEric Blake3-6/+27
What's more meta than using qapi to define qapi? :) Convert QType into a full-fledged[*] builtin qapi enum type, so that a subsequent patch can then use it as the discriminator type of qapi alternate types. Fortunately, the judicious use of 'prefix' in the qapi definition avoids churn to the spelling of the enum constants. To avoid circular definitions, we have to flip the order of inclusion between "qobject.h" vs. "qapi-types.h". Back in commit 28770e0, we had the latter include the former, so that we could use 'QObject *' for our implementation of 'any'. But that usage also works with only a forward declaration, whereas the definition of QObject requires QType to be a complete type. [*] The type has to be builtin, rather than declared in qapi/common.json, because we want to use it for alternates even when common.json is not included. But since it is the first builtin enum type, we have to add special cases to qapi-types and qapi-visit to only emit definitions once, even when two qapi files are being compiled into the same binary (the way we already handled builtin list types like 'intList'). We may need to revisit how multiple qapi files share common types, but that's a project for another day. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1449033659-25497-4-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qobject: Rename qtype_code to QTypeEric Blake1-1/+1
The name QType matches our CODING_STYLE conventions for type names in CamelCase. It also matches the fact that we are already naming all the enum members with a prefix of QTYPE, not QTYPE_CODE. And doing the rename will also make it easier for the next patch to use QAPI for providing the enum, which also wants CamelCase type names. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1449033659-25497-3-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Change munging of CamelCase enum valuesEric Blake1-1/+1
When munging enum values, the fact that we were passing the entire prefix + value through camel_to_upper() meant that enum values spelled with CamelCase could be turned into CAMEL_CASE. However, this provides a potential collision (both OneTwo and One-Two would munge into ONE_TWO) for enum types, when the same two names are valid side-by-side as QAPI member names. By changing the generation of enum constants to always be prefix + '_' + c_name(value, False).upper(), and ensuring that there are no case collisions (in the next patches), we no longer have to worry about names that would be distinct as QAPI members but collide as variant tag names, without having to think about what munging the heuristics in camel_to_upper() will actually perform on an enum value. Making the change will affect enums that did not follow coding conventions, using 'CamelCase' rather than desired 'lower-case'. Thankfully, there are only two culprits: InputButton and ErrorClass. We already tweaked ErrorClass to make it an alias of QapiErrorClass, where only the alias needs changing rather than the whole tree. So the bulk of this change is modifying INPUT_BUTTON_WHEEL_UP to the new INPUT_BUTTON_WHEELUP (and likewise for WHEELDOWN). That part of this commit may later need reverting if we rename the enum constants from 'WheelUp' to 'wheel-up' as part of moving x-input-send-event to a stable interface; but at least we have documentation bread crumbs in place to remind us (commit 513e7cd), and it matches the fact that SDL constants are also spelled SDL_BUTTON_WHEELUP. Suggested by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1447836791-369-27-git-send-email-eblake@redhat.com> [Commit message tweaked] Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Remove obsolete tests for MAX collisionEric Blake1-5/+3
Now that we no longer collide with an implicit _MAX enum member, we no longer need to reject it in the ad hoc parser, and can remove several tests that are no longer needed. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1447836791-369-24-git-send-email-eblake@redhat.com> [Commit message tweaked] Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Don't let implicit enum MAX member collideEric Blake1-3/+3
Now that we guarantee the user doesn't have any enum values beginning with a single underscore, we can use that for our own purposes. Renaming ENUM_MAX to ENUM__MAX makes it obvious that the sentinel is generated. This patch was mostly generated by applying a temporary patch: |diff --git a/scripts/qapi.py b/scripts/qapi.py |index e6d014b..b862ec9 100644 |--- a/scripts/qapi.py |+++ b/scripts/qapi.py |@@ -1570,6 +1570,7 @@ const char *const %(c_name)s_lookup[] = { | max_index = c_enum_const(name, 'MAX', prefix) | ret += mcgen(''' | [%(max_index)s] = NULL, |+// %(max_index)s | }; | ''', | max_index=max_index) then running: $ cat qapi-{types,event}.c tests/test-qapi-types.c | sed -n 's,^// \(.*\)MAX,s|\1MAX|\1_MAX|g,p' > list $ git grep -l _MAX | xargs sed -i -f list The only things not generated are the changes in scripts/qapi.py. Rejecting enum members named 'MAX' is now useless, and will be dropped in the next patch. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1447836791-369-23-git-send-email-eblake@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> [Rebased to current master, commit message tweaked] Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Tighten the regex on valid namesEric Blake1-5/+7
We already documented that qapi names should match specific patterns (such as starting with a letter unless it was an enum value or a downstream extension). Tighten that from a suggestion into a hard requirement, which frees up names beginning with a single underscore for qapi internal usage. The tighter regex doesn't forbid everything insane that a user could provide (for example, a user could name a type 'Foo-lookup' to collide with the generated 'Foo_lookup[]' for an enum 'Foo'), but does a good job at protecting the most obvious uses, and also happens to reserve single leading underscore for later use. The handling of enum values starting with a digit is tricky: commit 9fb081e introduced a subtle bug by using c_name() on a munged value, which would allow an enum to include the member 'q-int' in spite of our reservation. Furthermore, munging with a leading '_' would fail our tighter regex. So fix it by only munging for leading digits (which are never ticklish in c_name()) and by using a different prefix (I picked 'D', although any letter should do). Add new tests, reserved-member-underscore and reserved-enum-q, to demonstrate the tighter checking. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1447836791-369-22-git-send-email-eblake@redhat.com> Message-Id: <1447883135-18020-1-git-send-email-eblake@redhat.com> [Eric's fixup squashed in] Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Fix c_name() mungingEric Blake1-1/+2
The method c_name() is supposed to do two different actions: munge '-' into '_', and add a 'q_' prefix to ticklish names. But it did these steps out of order, making it possible to submit input that is not ticklish until after munging, where the output then lacked the desired prefix. The failure is exposed easily if you have a compiler that recognizes C11 keywords, and try to name a member '_Thread-local', as it would result in trying to compile the declaration 'uint64_t _Thread_local;' which is not valid. However, this name violates our conventions (ultimately, want to enforce that no qapi names start with single underscore), so the test is slightly weaker by instead testing 'wchar-t'; the declaration 'uint64_t wchar_t;' is valid in C (where wchar_t is only a typedef) but would fail with a C++ compiler (where it is a keyword). Fix things by reversing the order of actions within c_name(). Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1447836791-369-18-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-12-17qapi: Detect collisions in C member namesEric Blake1-12/+19
Detect attempts to declare two object members that would result in the same C member name, by keying the 'seen' dictionary off of the C name rather than the qapi name. It also requires passing info through the check_clash() methods. This addresses a TODO and fixes the previously-broken args-name-clash test. The resulting error message demonstrates the utility of the .describe() method added previously. No change to generated code. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1447836791-369-17-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>