aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2020-10-10qapi/source.py: add type hint annotationsJohn Snow
Annotations do not change runtime behavior. This commit *only* adds annotations. A note on typing of __init__: mypy requires init functions with no parameters to document a return type of None to be considered fully typed. In the case when there are input parameters, None may be omitted. Since __init__ may never return any value, it is preferred to omit the return annotation whenever possible. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Cleber Rosa <crosa@redhat.com> Tested-by: Cleber Rosa <crosa@redhat.com> Message-Id: <20201009161558.107041-25-jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10qapi/commands.py: add type hint annotationsJohn Snow
Annotations do not change runtime behavior. This commit *only* adds annotations. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Cleber Rosa <crosa@redhat.com> Message-Id: <20201009161558.107041-23-jsnow@redhat.com> Message-Id: <20201009161558.107041-24-jsnow@redhat.com> [mypy.ini update squashed in] Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10qapi/commands.py: Don't re-bind to variable of different typeJohn Snow
Mypy isn't a fan of rebinding a variable with a new data type. It's easy enough to avoid. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Cleber Rosa <crosa@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20201009161558.107041-22-jsnow@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10qapi/events.py: Move comments into docstringsJohn Snow
Clarify them while we're here. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Cleber Rosa <crosa@redhat.com> Message-Id: <20201009161558.107041-21-jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10qapi/events.py: add type hint annotationsJohn Snow
Annotations do not change runtime behavior. This commit *only* adds annotations. Note: __init__ does not need its return type annotated, as it is special. https://mypy.readthedocs.io/en/stable/class_basics.html#annotating-init-methods Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Cleber Rosa <crosa@redhat.com> Message-Id: <20201009161558.107041-20-jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10qapi: establish mypy type-checking baselineJohn Snow
Fix a minor typing issue, and then establish a mypy type-checking baseline. Like pylint, this should be run from the folder above: > mypy --config-file=qapi/mypy.ini qapi/ This is designed and tested for mypy 0.770 or greater. Signed-off-by: John Snow <jsnow@redhat.com> Tested-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Cleber Rosa <crosa@redhat.com> Tested-by: Cleber Rosa <crosa@redhat.com> Message-Id: <20201009161558.107041-19-jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10qapi/common.py: move build_params into gen.pyJohn Snow
Including it in common.py creates a circular import dependency; schema relies on common, but common.build_params requires a type annotation from schema. To type this properly, it needs to be moved outside the cycle. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Cleber Rosa <crosa@redhat.com> Message-Id: <20201009161558.107041-18-jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10qapi/common.py: Convert comments into docstrings, and elaborateJohn Snow
As docstrings, they'll show up in documentation and IDE help. The docstring style being targeted is the Sphinx documentation style. Sphinx uses an extension of ReST with "domains". We use the (implicit) Python domain, which supports a number of custom "info fields". Those info fields are documented here: https://www.sphinx-doc.org/en/master/usage/restructuredtext/domains.html#info-field-lists Primarily, we use `:param X: descr`, `:return[s]: descr`, and `:raise[s] Z: when`. Everything else is the Sphinx dialect of ReST. (No, nothing checks or enforces this style that I am aware of. Sphinx either chokes or succeeds, but does not enforce a standard of what is otherwise inside the docstring. Pycharm does highlight when your param fields are not aligned with the actual fields present. It does not highlight missing return or exception statements. There is no existing style guide I am aware of that covers a standard for a minimally acceptable docstring. I am debating writing one.) Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Cleber Rosa <crosa@redhat.com> Message-Id: <20201009161558.107041-17-jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10qapi/common.py: add type hint annotationsJohn Snow
Annotations do not change runtime behavior. This commit *only* adds annotations. Note that build_params() cannot be fully annotated due to import dependency issues. The commit after next will take care of it. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Cleber Rosa <crosa@redhat.com> Message-Id: <20201009161558.107041-16-jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10qapi/common.py: check with pylintJohn Snow
Remove qapi/common.py from the pylintrc ignore list. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Cleber Rosa <crosa@redhat.com> Tested-by: Cleber Rosa <crosa@redhat.com> Tested-by: Eduardo Habkost <ehabkost@redhat.com> Message-Id: <20201009161558.107041-15-jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10qapi/common.py: Replace one-letter 'c' variableJohn Snow
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Cleber Rosa <crosa@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20201009161558.107041-14-jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10qapi/common.py: delint with pylintJohn Snow
At this point, that just means using a consistent strategy for constant names. constants get UPPER_CASE and names not used externally get a leading underscore. As a preference, while renaming constants to be UPPERCASE, move them to the head of the file. Generally, it's nice to be able to audit the code that runs on import in one central place. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Cleber Rosa <crosa@redhat.com> Message-Id: <20201009161558.107041-13-jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10qapi/common.py: Add indent managerJohn Snow
Code style tools really dislike the use of global keywords, because it generally involves re-binding the name at runtime which can have strange effects depending on when and how that global name is referenced in other modules. Make a little indent level manager instead. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Cleber Rosa <crosa@redhat.com> Message-Id: <20201009161558.107041-12-jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10qapi/common.py: Remove python compatibility workaroundJohn Snow
Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Cleber Rosa <crosa@redhat.com> Message-Id: <20201009161558.107041-11-jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10qapi: add pylintrcJohn Snow
Using `pylint --generate-rcfile > pylintrc`, generate a skeleton pylintrc file. Sections that are not presently relevant (by the end of this series) are removed leaving just the empty section as a search engine / documentation hint to future authors. I am targeting pylint 2.6.0. In the future (and hopefully before 5.2 is released), I aim to have gitlab CI running the specific targeted versions of pylint, mypy, flake8, etc in a job. 2.5.x will work if you additionally pass --disable=bad-whitespace. This warning was removed from 2.6.x, for lack of consistent support. Right now, quite a few modules are ignored as they are known to fail as of this commit. modules will be removed from the known-bad list throughout this and following series as they are repaired. Note: Normally, pylintrc would go in the folder above the module, but as that folder is shared by many things, it is going inside the module folder (for now). Due to a bug in pylint 2.5+, pylint does not correctly recognize when it is being run from "inside" a package, and must be run *outside* of the package. Therefore, to run it, you must: > pylint scripts/qapi/ --rcfile=scripts/qapi/pylintrc Signed-off-by: John Snow <jsnow@redhat.com> Tested-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Cleber Rosa <crosa@redhat.com> Tested-by: Cleber Rosa <crosa@redhat.com> Message-Id: <20201009161558.107041-10-jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10qapi: delint using flake8John Snow
Petty style guide fixes and line length enforcement. Not a big win, not a big loss, but flake8 passes 100% on the qapi module, which gives us an easy baseline to enforce hereafter. A note on the flake8 exception: flake8 will warn on *any* bare except, but pylint's is context-aware and will suppress the warning if you re-raise the exception. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Cleber Rosa <crosa@redhat.com> Message-Id: <20201009161558.107041-9-jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10qapi: enforce import order/styling with isortJohn Snow
While we're mucking around with imports, we might as well formalize the style we use. Let's use isort to do it for us. lines_after_imports=2: Use two lines after imports, to match PEP8's desire to have "two lines before and after" class definitions, which are likely to start immediately after imports. force_sort_within_sections: Intermingles "from x" and "import x" style statements, such that sorting is always performed strictly on the module name itself. force_grid_wrap=4: Four or more imports from a single module will force the one-per-line style that's more git-friendly. This will generally happen for 'typing' imports. multi_line_output=3: Uses the one-per-line indented style for long imports. include_trailing_comma: Adds a comma to the last import in a group, which makes git conflicts nicer to deal with, generally. line_length: 72 is chosen to match PEP8's "docstrings and comments" line length limit. If you have a single line import that exceeds 72 characters, your names are too long! Suggested-by: Cleber Rosa <crosa@redhat.com> Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Cleber Rosa <crosa@redhat.com> Tested-by: Cleber Rosa <crosa@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20201009161558.107041-8-jsnow@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10qapi: Remove wildcard includesJohn Snow
Wildcard includes become hard to manage when refactoring and dealing with circular dependencies with strictly typed mypy. flake8 also flags each one as a warning, as it is not smart enough to know which names exist in the imported file. Remove them and include things explicitly by name instead. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Cleber Rosa <crosa@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20201009161558.107041-7-jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10qapi: Prefer explicit relative importsJohn Snow
All of the QAPI include statements are changed to be package-aware, as explicit relative imports. A quirk of Python packages is that the name of the package exists only *outside* of the package. This means that to a module inside of the qapi folder, there is inherently no such thing as the "qapi" package. The reason these imports work is because the "qapi" package exists in the context of the caller -- the execution shim, where sys.path includes a directory that has a 'qapi' folder in it. When we write "from qapi import sibling", we are NOT referencing the folder 'qapi', but rather "any package named qapi in sys.path". If you should so happen to have a 'qapi' package in your path, it will use *that* package. When we write "from .sibling import foo", we always reference explicitly our sibling module; guaranteeing consistency in *where* we are importing these modules from. This can be useful when working with virtual environments and packages in development mode. In development mode, a package is installed as a series of symlinks that forwards to your same source files. The problem arises because code quality checkers will follow "import qapi.x" to the "installed" version instead of the sibling file and -- even though they are the same file -- they have different module paths, and this causes cyclic import problems, false positive type mismatch errors, and more. It can also be useful when dealing with hierarchical packages, e.g. if we allow qemu.core.qmp, qemu.qapi.parser, etc. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Cleber Rosa <crosa@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20201009161558.107041-6-jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10qapi: move generator entrypoint into packageJohn Snow
As part of delinting and adding type hints to the QAPI generator, it's helpful for the entrypoint to be part of the package, only leaving a very tiny entrypoint shim outside of the package. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Cleber Rosa <crosa@redhat.com> Tested-by: Cleber Rosa <crosa@redhat.com> Message-Id: <20201009161558.107041-5-jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> [invalid_char() renamed to invalid_prefix_char()] Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10qapi-gen: Separate arg-parsing from generationJohn Snow
This is a minor re-work of the entrypoint script. It isolates a generate() method from the actual command-line mechanism. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Cleber Rosa <crosa@redhat.com> Tested-by: Cleber Rosa <crosa@redhat.com> Message-Id: <20201009161558.107041-4-jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> [invalid_char() renamed to invalid_prefix_char()] Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10qapi: modify docstrings to be sphinx-compatibleJohn Snow
A precise style guide and a package-wide overhaul is forthcoming pending further discussion and consensus. For now, merely avoid obvious errors that cause Sphinx documentation build problems, using a style loosely based on PEP 257 and Sphinx Autodoc. It is chosen for interoperability with our existing Sphinx framework, and because it has loose recognition in the Pycharm IDE. See also: https://www.python.org/dev/peps/pep-0257/ https://www.sphinx-doc.org/en/master/usage/restructuredtext/domains.html#info-field-lists Signed-off-by: John Snow <jsnow@redhat.com> Message-Id: <20201009161558.107041-3-jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-10docs: repair broken referencesJohn Snow
In two different places, we are not making a cross-reference to some resource correctly. Signed-off-by: John Snow <jsnow@redhat.com> Message-Id: <20201009161558.107041-2-jsnow@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-09Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-5.2-20201009' ↵Peter Maydell
into staging ppc patch queue 2020-10-09 Here's the next set of ppc related patches for qemu-5.2. There are two main things here: * Cleanups to error handling in spapr from Greg Kurz * Improvements to NUMA handling for spapr from Daniel Barboza There are also a handful of other bugfixes. # gpg: Signature made Fri 09 Oct 2020 07:02:29 BST # gpg: using RSA key 75F46586AE61A66CC44E87DC6C38CACA20D9B392 # gpg: Good signature from "David Gibson <david@gibson.dropbear.id.au>" [full] # gpg: aka "David Gibson (Red Hat) <dgibson@redhat.com>" [full] # gpg: aka "David Gibson (ozlabs.org) <dgibson@ozlabs.org>" [full] # gpg: aka "David Gibson (kernel.org) <dwg@kernel.org>" [unknown] # Primary key fingerprint: 75F4 6586 AE61 A66C C44E 87DC 6C38 CACA 20D9 B392 * remotes/dgibson/tags/ppc-for-5.2-20201009: specs/ppc-spapr-numa: update with new NUMA support spapr_numa: consider user input when defining associativity spapr_numa: change reference-points and maxdomain settings spapr_numa: forbid asymmetrical NUMA setups spapr: add spapr_machine_using_legacy_numa() helper ppc/pnv: Increase max firmware size spapr: Add a return value to spapr_check_pagesize() spapr: Add a return value to spapr_nvdimm_validate() spapr: Simplify error handling in spapr_cpu_core_realize() spapr: Add a return value to spapr_set_vcpu_id() spapr: Simplify error handling in prop_get_fdt() spapr: Add a return value to spapr_drc_attach() spapr: Simplify error handling in spapr_vio_busdev_realize() spapr: Simplify error handling in do_client_architecture_support() spapr: Get rid of cas_check_pvr() error reporting spapr: Simplify error handling in callers of ppc_set_compat() ppc: Fix return value in cpu_post_load() error path ppc: Add a return value to ppc_set_compat() and ppc_set_compat_all() spapr: Fix error leak in spapr_realize_vcpu() spapr: Handle HPT allocation failure in nested guest Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2020-10-09Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2020-10-09' ↵Peter Maydell
into staging Error reporting patches for 2020-10-09 # gpg: Signature made Fri 09 Oct 2020 07:45:17 BST # gpg: using RSA key 354BC8B3D7EB2A6B68674E5F3870B400EB918653 # gpg: issuer "armbru@redhat.com" # gpg: Good signature from "Markus Armbruster <armbru@redhat.com>" [full] # gpg: aka "Markus Armbruster <armbru@pond.sub.org>" [full] # Primary key fingerprint: 354B C8B3 D7EB 2A6B 6867 4E5F 3870 B400 EB91 8653 * remotes/armbru/tags/pull-error-2020-10-09: error: Use error_fatal to simplify obvious fatal errors (again) error: Remove NULL checks on error_propagate() calls (again) Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2020-10-09Merge remote-tracking branch 'remotes/armbru/tags/pull-monitor-2020-10-09' ↵Peter Maydell
into staging Monitor patches for 2020-10-09 # gpg: Signature made Fri 09 Oct 2020 06:16:51 BST # gpg: using RSA key 354BC8B3D7EB2A6B68674E5F3870B400EB918653 # gpg: issuer "armbru@redhat.com" # gpg: Good signature from "Markus Armbruster <armbru@redhat.com>" [full] # gpg: aka "Markus Armbruster <armbru@pond.sub.org>" [full] # Primary key fingerprint: 354B C8B3 D7EB 2A6B 6867 4E5F 3870 B400 EB91 8653 * remotes/armbru/tags/pull-monitor-2020-10-09: block: Convert 'block_resize' to coroutine block: Add bdrv_lock()/unlock() block: Add bdrv_co_enter()/leave() util/async: Add aio_co_reschedule_self() hmp: Add support for coroutine command handlers qmp: Move dispatcher to a coroutine qapi: Add a 'coroutine' flag for commands monitor: Make current monitor a per-coroutine property qmp: Call monitor_set_cur() only in qmp_dispatch() qmp: Assert that no other monitor is active hmp: Update current monitor only in handle_hmp_command() monitor: Use getter/setter functions for cur_mon monitor: Add Monitor parameter to monitor_get_cpu_index() monitor: Add Monitor parameter to monitor_set_cpu() Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2020-10-09error: Use error_fatal to simplify obvious fatal errors (again)Markus Armbruster
Patch created mechanically by rerunning: $ spatch --in-place --sp-file scripts/coccinelle/use-error_fatal.cocci \ --macro-file scripts/cocci-macro-file.h --use-gitgrep . Variables now unused dropped manually. Cc: Eric Auger <eric.auger@redhat.com> Cc: David Hildenbrand <david@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200722084048.1726105-5-armbru@redhat.com> Reviewed-by: David Hildenbrand <david@redhat.com>
2020-10-09error: Remove NULL checks on error_propagate() calls (again)Markus Armbruster
Patch created mechanically by rerunning: $ spatch --sp-file scripts/coccinelle/error_propagate_null.cocci \ --macro-file scripts/cocci-macro-file.h \ --use-gitgrep . Cc: Jens Freimann <jfreimann@redhat.com> Cc: Hailiang Zhang <zhang.zhanghailiang@huawei.com> Cc: Juan Quintela <quintela@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200722084048.1726105-4-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2020-10-09block: Convert 'block_resize' to coroutineKevin Wolf
block_resize performs some I/O that could potentially take quite some time, so use it as an example for the new 'coroutine': true annotation in the QAPI schema. bdrv_truncate() requires that we're already in the right AioContext for the BlockDriverState if called in coroutine context. So instead of just taking the AioContext lock, move the QMP handler coroutine to the context. Call blk_unref() only after switching back because blk_unref() may only be called in the main thread. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20201005155855.256490-15-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-09block: Add bdrv_lock()/unlock()Kevin Wolf
Inside of coroutine context, we can't directly use aio_context_acquire() for the AioContext of a block node because we already own the lock of the current AioContext and we need to avoid double locking to prevent deadlocks. This provides helper functions to lock the AioContext of a node only if it's not the same as the current AioContext. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20201005155855.256490-14-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-09block: Add bdrv_co_enter()/leave()Kevin Wolf
Add a pair of functions to temporarily move the current coroutine to the AioContext of a given BlockDriverState. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20201005155855.256490-13-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-09util/async: Add aio_co_reschedule_self()Kevin Wolf
Add a function that can be used to move the currently running coroutine to a different AioContext (and therefore potentially a different thread). Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20201005155855.256490-12-kwolf@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-09hmp: Add support for coroutine command handlersKevin Wolf
Often, QMP command handlers are not only called to handle QMP commands, but also from a corresponding HMP command handler. In order to give them a consistent environment, optionally run HMP command handlers in a coroutine, too. The implementation is a lot simpler than in QMP because for HMP, we still block the VM while the coroutine is running. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Message-Id: <20201005155855.256490-11-kwolf@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-09qmp: Move dispatcher to a coroutineKevin Wolf
This moves the QMP dispatcher to a coroutine and runs all QMP command handlers that declare 'coroutine': true in coroutine context so they can avoid blocking the main loop while doing I/O or waiting for other events. For commands that are not declared safe to run in a coroutine, the dispatcher drops out of coroutine context by calling the QMP command handler from a bottom half. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20201005155855.256490-10-kwolf@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-09qapi: Add a 'coroutine' flag for commandsKevin Wolf
This patch adds a new 'coroutine' flag to QMP command definitions that tells the QMP dispatcher that the command handler is safe to be run in a coroutine. The documentation of the new flag pretends that this flag is already used as intended, which it isn't yet after this patch. We'll implement this in another patch in this series. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20201005155855.256490-9-kwolf@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-09monitor: Make current monitor a per-coroutine propertyKevin Wolf
This way, a monitor command handler will still be able to access the current monitor, but when it yields, all other code code will correctly get NULL from monitor_cur(). This uses a hash table to map the coroutine pointer to the current monitor of that coroutine. Outside of coroutine context, we associate the current monitor with the leader coroutine of the current thread. Approaches to implement some form of coroutine local storage directly in the coroutine core code have been considered and discarded because they didn't end up being much more generic than the hash table and their performance impact on coroutines not using coroutine local storage was unclear. As the block layer uses a coroutine per I/O request, this is a fast path and we have to be careful. It's safest to just stay out of this path with code only used by the monitor. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20201005155855.256490-8-kwolf@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-09qmp: Call monitor_set_cur() only in qmp_dispatch()Kevin Wolf
The correct way to set the current monitor for a coroutine handler will be different than for a blocking handler, so monitor_set_cur() needs to be called in qmp_dispatch(). Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20201005155855.256490-7-kwolf@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-09qmp: Assert that no other monitor is activeKevin Wolf
monitor_qmp_dispatch() is never supposed to be called in the context of another monitor, so assert that monitor_cur() is NULL instead of saving and restoring it. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20201005155855.256490-6-kwolf@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-09hmp: Update current monitor only in handle_hmp_command()Kevin Wolf
The current monitor is updated relatively early in the command handling code even though only the command handler actually needs it. The current monitor will become coroutine-local later, so we can only update it when we know in which coroutine the command will be exectued. Move it to handle_hmp_command() where this information will be available. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20201005155855.256490-5-kwolf@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-09monitor: Use getter/setter functions for cur_monKevin Wolf
cur_mon really needs to be coroutine-local as soon as we move monitor command handlers to coroutines and let them yield. As a first step, just remove all direct accesses to cur_mon so that we can implement this in the getter function later. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20201005155855.256490-4-kwolf@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-09monitor: Add Monitor parameter to monitor_get_cpu_index()Kevin Wolf
Most callers actually don't have to rely on cur_mon, but already know for which monitor they call monitor_get_cpu_index(). Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20201005155855.256490-3-kwolf@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-09monitor: Add Monitor parameter to monitor_set_cpu()Kevin Wolf
Most callers actually don't have to rely on cur_mon, but already know for which monitor they call monitor_set_cpu(). Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20201005155855.256490-2-kwolf@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-09specs/ppc-spapr-numa: update with new NUMA supportDaniel Henrique Barboza
This update provides more in depth information about the choices and drawbacks of the new NUMA support for the spapr machine. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> Message-Id: <20201007172849.302240-6-danielhb413@gmail.com> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2020-10-09spapr_numa: consider user input when defining associativityDaniel Henrique Barboza
A new function called spapr_numa_define_associativity_domains() is created to calculate the associativity domains and change the associativity arrays considering user input. This is how the associativity domain between two NUMA nodes A and B is calculated: - get the distance D between them - get the correspondent NUMA level 'n_level' for D. This is done via a helper called spapr_numa_get_numa_level() - all associativity arrays were initialized with their own numa_ids, and we're calculating the distance in node_id ascending order, starting from node id 0 (the first node retrieved by numa_state). This will have a cascade effect in the algorithm because the associativity domains that node 0 defines will be carried over to other nodes, and node 1 associativities will be carried over after taking node 0 associativities into account, and so on. This happens because we'll assign assoc_src as the associativity domain of dst as well, for all NUMA levels beyond and including n_level. The PPC kernel expects the associativity domains of the first node (node id 0) to be always 0 [1], and this algorithm will grant that by default. Ultimately, all of this results in a best effort approximation for the actual NUMA distances the user input in the command line. Given the nature of how PAPR itself interprets NUMA distances versus the expectations risen by how ACPI SLIT works, there might be better algorithms but, in the end, it'll also result in another way to approximate what the user really wanted. To keep this commit message no longer than it already is, the next patch will update the existing documentation in ppc-spapr-numa.rst with more in depth details and design considerations/drawbacks. [1] https://lore.kernel.org/linuxppc-dev/5e8fbea3-8faf-0951-172a-b41a2138fbcf@gmail.com/ Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> Message-Id: <20201007172849.302240-5-danielhb413@gmail.com> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2020-10-09spapr_numa: change reference-points and maxdomain settingsDaniel Henrique Barboza
This is the first guest visible change introduced in spapr_numa.c. The previous settings of both reference-points and maxdomains were too restrictive, but enough for the existing associativity we're setting in the resources. We'll change that in the following patches, populating the associativity arrays based on user input. For those changes to be effective, reference-points and maxdomains must be more flexible. After this patch, we'll have 4 distinct levels of NUMA (0x4, 0x3, 0x2, 0x1) and maxdomains will allow for any type of configuration the user intends to do - under the scope and limitations of PAPR itself, of course. Reviewed-by: Greg Kurz <groug@kaod.org> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> Message-Id: <20201007172849.302240-4-danielhb413@gmail.com> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2020-10-09spapr_numa: forbid asymmetrical NUMA setupsDaniel Henrique Barboza
The pSeries machine does not support asymmetrical NUMA configurations. This doesn't make much of a different since we're not using user input for pSeries NUMA setup, but this will change in the next patches. To avoid breaking existing setups, gate this change by checking for legacy NUMA support. Reviewed-by: Greg Kurz <groug@kaod.org> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> Message-Id: <20201007172849.302240-3-danielhb413@gmail.com> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2020-10-09spapr: add spapr_machine_using_legacy_numa() helperDaniel Henrique Barboza
The changes to come to NUMA support are all guest visible. In theory we could just create a new 5_1 class option flag to avoid the changes to cascade to 5.1 and under. The reality is that these changes are only relevant if the machine has more than one NUMA node. There is no need to change guest behavior that has been around for years needlesly. This new helper will be used by the next patches to determine whether we should retain the (soon to be) legacy NUMA behavior in the pSeries machine. The new behavior will only be exposed if: - machine is pseries-5.2 and newer; - more than one NUMA node is declared in NUMA state. Reviewed-by: Greg Kurz <groug@kaod.org> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> Message-Id: <20201007172849.302240-2-danielhb413@gmail.com> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2020-10-09ppc/pnv: Increase max firmware sizeCédric Le Goater
Builds enabling GCOV can be bigger than 4MB and the limit on FSP systems is 16MB. Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20201002091440.1349326-1-clg@kaod.org> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2020-10-09spapr: Add a return value to spapr_check_pagesize()Greg Kurz
As recommended in "qapi/error.h", return true on success and false on failure. This allows to reduce error propagation overhead in the callers. Signed-off-by: Greg Kurz <groug@kaod.org> Message-Id: <20200914123505.612812-14-groug@kaod.org> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2020-10-09spapr: Add a return value to spapr_nvdimm_validate()Greg Kurz
As recommended in "qapi/error.h", return true on success and false on failure. This allows to reduce error propagation overhead in the callers. Signed-off-by: Greg Kurz <groug@kaod.org> Message-Id: <20200914123505.612812-13-groug@kaod.org> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>