Re: [PATCH 0/2] scsi: to fix issue on passing host_status to the guest kernel
ping? Thank you very much! Dongli Zhang On 12/10/21 6:16 AM, Dongli Zhang wrote: > This patchset fixes the issue on passing 'host_status' to the guest kernel. > > The 1st patch fixes the erroneous usage of req->host_status. > > The 2nd patch is to pass the SCSI_HOST_ERROR to the guest kernel when the > req->bus->info->fail() is not implemented. I do not add 'Fixes:' because I > am not sure if to not pass SCSI_HOST_ERROR was on purpose, especially for > security reason. > > Thank you very much! > > Dongli Zhang > > >
Re: [PATCH v3 00/31] Python: delete synchronous qemu.qmp package
On Mon, Jan 10, 2022 at 6:29 PM John Snow wrote: > Based-on: <20220110232521.1922962-1-js...@redhat.com> > (jsnow/python staging branch) > Sorry, I goofed. This series accidentally re-includes these patches. You can ignore the first four patches, or apply directly on top of origin/master. Sorry for the inconvenience. --js > GitLab: https://gitlab.com/jsnow/qemu/-/commits/python-qmp-legacy-switch > CI: https://gitlab.com/jsnow/qemu/-/pipelines/445163212 > > Hi, this series is part of an effort to publish the qemu.qmp package on > PyPI. It is the first of three series to complete this work: > > --> (1) Switch the new Async QMP library in to python/qemu/qmp > (2) Fork python/qemu/qmp out into its own repository, > with updated GitLab CI/CD targets to build packages. > (3) Update qemu.git to install qemu.qmp from PyPI, > and then delete python/qemu/qmp. > > This series swaps out qemu.qmp for qemu.aqmp permanently, instead of > hiding it behind an environment variable toggle. This leaves us with > just one QMP library to worry about. It also implements the rename of > "qemu.aqmp" to "qemu.qmp". > > I suspect the most potential disruption to iotest and avocado > maintainers, as those two subsystems rely on the QMP features the > most. Would appreciate at least an ACK from each of those camps if > you're willing to give benefit-of-the-doubt on the actual Python code. > > V3: > - Rebased on top of jsnow/python (For GitLab CI fixes) > - Added a new patch 001 to fix a typo Vladimir found. > - Tiny change in 006 due to the new patch 001 > - Reworded subject of patch 007 > - Changed import statement in patch 013 (Vladimir) > - Rebase-related changes in patch 021 > - Removed 'aqmp' from internal variable names in 026 > - Added new patch to rename aqmp-tui to qmp-tui in 027 > > V2: > - Integrate the renaming of qemu.aqmp to qemu.qmp in this series > - Minor bits and pieces. > > John Snow (30): > python/aqmp: use absolute import statement > Python/aqmp: fix type definitions for mypy 0.920 > python: update type hints for mypy 0.930 > python/aqmp: fix docstring typo > python/aqmp: add __del__ method to legacy interface > python/aqmp: handle asyncio.TimeoutError on execute() > python/aqmp: copy type definitions from qmp > python/aqmp: add SocketAddrT to package root > python/aqmp: rename AQMPError to QMPError > python/qemu-ga-client: don't use deprecated CLI syntax in usage > comment > python/qmp: switch qemu-ga-client to AQMP > python/qmp: switch qom tools to AQMP > python/qmp: switch qmp-shell to AQMP > python: move qmp utilities to python/qemu/utils > python: move qmp-shell under the AQMP package > python/machine: permanently switch to AQMP > scripts/cpu-x86-uarch-abi: fix CLI parsing > scripts/cpu-x86-uarch-abi: switch to AQMP > scripts/render-block-graph: switch to AQMP > scripts/bench-block-job: switch to AQMP > iotests/mirror-top-perms: switch to AQMP > iotests: switch to AQMP > python: temporarily silence pylint duplicate-code warnings > python/aqmp: take QMPBadPortError and parse_address from qemu.qmp > python/aqmp: fully separate from qmp.QEMUMonitorProtocol > python/aqmp: copy qmp docstrings to qemu.aqmp.legacy > python: remove the old QMP package > python: re-enable pylint duplicate-code warnings > python: rename qemu.aqmp to qemu.qmp > python: rename 'aqmp-tui' to 'qmp-tui' > > Stefan Weil (1): > simplebench: Fix Python syntax error (reported by LGTM) > > python/qemu/qmp/README.rst| 9 - > python/qemu/aqmp/__init__.py | 51 -- > python/qemu/aqmp/legacy.py| 138 -- > python/qemu/aqmp/py.typed | 0 > python/qemu/machine/machine.py| 18 +- > python/qemu/machine/qtest.py | 2 +- > python/qemu/qmp/__init__.py | 441 ++ > python/qemu/{aqmp => qmp}/error.py| 12 +- > python/qemu/{aqmp => qmp}/events.py | 6 +- > python/qemu/qmp/legacy.py | 319 + > python/qemu/{aqmp => qmp}/message.py | 0 > python/qemu/{aqmp => qmp}/models.py | 0 > python/qemu/{aqmp => qmp}/protocol.py | 33 +- > python/qemu/{aqmp => qmp}/qmp_client.py | 32 +- > python/qemu/qmp/qmp_shell.py | 31 +- > .../qemu/{aqmp/aqmp_tui.py => qmp/qmp_tui.py} | 14 +- > python/qemu/{aqmp => qmp}/util.py | 0 > python/qemu/{qmp => utils}/qemu_ga_client.py | 24 +- > python/qemu/{qmp => utils}/qom.py | 5 +- > python/qemu/{qmp => utils}/qom_common.py | 9 +- > python/qemu/{qmp => utils}/qom_fuse.py| 11 +- > python/setup.cfg | 23 +- > python/tests/protocol.py | 14 +- > scripts/cpu-x86-uarch-abi.py | 7 +- > scripts/device-crash-test | 4 +- >
[PATCH v3 29/31] python: re-enable pylint duplicate-code warnings
With the old library gone, there's nothing duplicated in the tree, so the warning suppression can be removed. Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Beraldo Leal --- python/setup.cfg | 1 - 1 file changed, 1 deletion(-) diff --git a/python/setup.cfg b/python/setup.cfg index 5140a5b322..c341e922c2 100644 --- a/python/setup.cfg +++ b/python/setup.cfg @@ -114,7 +114,6 @@ ignore_missing_imports = True disable=consider-using-f-string, too-many-function-args, # mypy handles this with less false positives. no-member, # mypy also handles this better. -duplicate-code, # To be removed by the end of this patch series. [pylint.basic] # Good variable names which should always be accepted, separated by a comma. -- 2.31.1
[PATCH v3 27/31] python/aqmp: copy qmp docstrings to qemu.aqmp.legacy
Copy the docstrings out of qemu.qmp, adjusting them as necessary to more accurately reflect the current state of this class. Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Beraldo Leal --- python/qemu/aqmp/legacy.py | 110 ++--- 1 file changed, 102 insertions(+), 8 deletions(-) diff --git a/python/qemu/aqmp/legacy.py b/python/qemu/aqmp/legacy.py index 8f38e7d912..6c250cd46a 100644 --- a/python/qemu/aqmp/legacy.py +++ b/python/qemu/aqmp/legacy.py @@ -1,9 +1,23 @@ """ -Sync QMP Wrapper +(Legacy) Sync QMP Wrapper -This class pretends to be qemu.qmp.QEMUMonitorProtocol. +This module provides the `QEMUMonitorProtocol` class, which is a +synchronous wrapper around `QMPClient`. + +Its design closely resembles that of the original QEMUMonitorProtocol +class, originally written by Luiz Capitulino. """ +# Copyright (C) 2009, 2010, 2021 Red Hat Inc. +# +# Authors: +# Luiz Capitulino +# John Snow +# +# This work is licensed under the terms of the GNU GPL, version 2. +# See the COPYING file in the top-level directory. + + import asyncio from types import TracebackType from typing import ( @@ -39,9 +53,6 @@ # {} is the QMPReturnValue. -# pylint: disable=missing-docstring - - class QMPBadPortError(QMPError): """ Unable to parse socket address: Port was non-numerical. @@ -49,6 +60,21 @@ class QMPBadPortError(QMPError): class QEMUMonitorProtocol: +""" +Provide an API to connect to QEMU via QEMU Monitor Protocol (QMP) +and then allow to handle commands and events. + +:param address: QEMU address, can be either a unix socket path (string) + or a tuple in the form ( address, port ) for a TCP + connection +:param server: Deprecated, ignored. (See 'accept') +:param nickname: Optional nickname used for logging. + +..note:: +No connection is established during `__init__`, this is done by +the `connect()` or `accept()` methods. +""" + def __init__(self, address: SocketAddrT, server: bool = False, # pylint: disable=unused-argument nickname: Optional[str] = None): @@ -108,6 +134,12 @@ def parse_address(cls, address: str) -> SocketAddrT: return address def connect(self, negotiate: bool = True) -> Optional[QMPMessage]: +""" +Connect to the QMP Monitor and perform capabilities negotiation. + +:return: QMP greeting dict, or None if negotiate is false +:raise ConnectError: on connection errors +""" self._aqmp.await_greeting = negotiate self._aqmp.negotiate = negotiate @@ -117,6 +149,16 @@ def connect(self, negotiate: bool = True) -> Optional[QMPMessage]: return self._get_greeting() def accept(self, timeout: Optional[float] = 15.0) -> QMPMessage: +""" +Await connection from QMP Monitor and perform capabilities negotiation. + +:param timeout: +timeout in seconds (nonnegative float number, or None). +If None, there is no timeout, and this may block forever. + +:return: QMP greeting dict +:raise ConnectError: on connection errors +""" self._aqmp.await_greeting = True self._aqmp.negotiate = True @@ -130,6 +172,12 @@ def accept(self, timeout: Optional[float] = 15.0) -> QMPMessage: return ret def cmd_obj(self, qmp_cmd: QMPMessage) -> QMPMessage: +""" +Send a QMP command to the QMP Monitor. + +:param qmp_cmd: QMP command to be sent as a Python dict +:return: QMP response as a Python dict +""" return dict( self._sync( # pylint: disable=protected-access @@ -148,9 +196,9 @@ def cmd(self, name: str, """ Build a QMP command and send it to the QMP Monitor. -@param name: command name (string) -@param args: command arguments (dict) -@param cmd_id: command id (dict, list, string or int) +:param name: command name (string) +:param args: command arguments (dict) +:param cmd_id: command id (dict, list, string or int) """ qmp_cmd: QMPMessage = {'execute': name} if args: @@ -160,6 +208,9 @@ def cmd(self, name: str, return self.cmd_obj(qmp_cmd) def command(self, cmd: str, **kwds: object) -> QMPReturnValue: +""" +Build and send a QMP command to the monitor, report errors if any +""" return self._sync( self._aqmp.execute(cmd, kwds), self._timeout @@ -167,6 +218,19 @@ def command(self, cmd: str, **kwds: object) -> QMPReturnValue: def pull_event(self, wait: Union[bool, float] = False) -> Optional[QMPMessage]: +""" +Pulls a single event. + +:param wait: +If False or 0, do not wait. Return None if no events ready. +
[PATCH v3 26/31] python/aqmp: fully separate from qmp.QEMUMonitorProtocol
After this patch, qemu.aqmp.legacy.QEMUMonitorProtocol no longer inherits from qemu.qmp.QEMUMonitorProtocol. To do this, several inherited methods need to be explicitly re-defined. Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Beraldo Leal --- python/qemu/aqmp/legacy.py | 38 -- 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/python/qemu/aqmp/legacy.py b/python/qemu/aqmp/legacy.py index 76b09671cc..8f38e7d912 100644 --- a/python/qemu/aqmp/legacy.py +++ b/python/qemu/aqmp/legacy.py @@ -5,18 +5,18 @@ """ import asyncio +from types import TracebackType from typing import ( Any, Awaitable, Dict, List, Optional, +Type, TypeVar, Union, ) -import qemu.qmp - from .error import QMPError from .protocol import Runstate, SocketAddrT from .qmp_client import QMPClient @@ -48,9 +48,9 @@ class QMPBadPortError(QMPError): """ -class QEMUMonitorProtocol(qemu.qmp.QEMUMonitorProtocol): +class QEMUMonitorProtocol: def __init__(self, address: SocketAddrT, - server: bool = False, + server: bool = False, # pylint: disable=unused-argument nickname: Optional[str] = None): # pylint: disable=super-init-not-called @@ -74,7 +74,18 @@ def _get_greeting(self) -> Optional[QMPMessage]: return self._aqmp.greeting._asdict() return None -# __enter__ and __exit__ need no changes +def __enter__(self: _T) -> _T: +# Implement context manager enter function. +return self + +def __exit__(self, + # pylint: disable=duplicate-code + # see https://github.com/PyCQA/pylint/issues/3619 + exc_type: Optional[Type[BaseException]], + exc_val: Optional[BaseException], + exc_tb: Optional[TracebackType]) -> None: +# Implement context manager exit function. +self.close() @classmethod def parse_address(cls, address: str) -> SocketAddrT: @@ -131,7 +142,22 @@ def cmd_obj(self, qmp_cmd: QMPMessage) -> QMPMessage: ) ) -# Default impl of cmd() delegates to cmd_obj +def cmd(self, name: str, +args: Optional[Dict[str, object]] = None, +cmd_id: Optional[object] = None) -> QMPMessage: +""" +Build a QMP command and send it to the QMP Monitor. + +@param name: command name (string) +@param args: command arguments (dict) +@param cmd_id: command id (dict, list, string or int) +""" +qmp_cmd: QMPMessage = {'execute': name} +if args: +qmp_cmd['arguments'] = args +if cmd_id: +qmp_cmd['id'] = cmd_id +return self.cmd_obj(qmp_cmd) def command(self, cmd: str, **kwds: object) -> QMPReturnValue: return self._sync( -- 2.31.1
[PATCH v3 25/31] python/aqmp: take QMPBadPortError and parse_address from qemu.qmp
Shift these definitions over from the qmp package to the async qmp package. Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Beraldo Leal --- python/qemu/aqmp/aqmp_tui.py | 3 +-- python/qemu/aqmp/legacy.py | 30 ++ python/qemu/qmp/__init__.py | 26 -- 3 files changed, 27 insertions(+), 32 deletions(-) diff --git a/python/qemu/aqmp/aqmp_tui.py b/python/qemu/aqmp/aqmp_tui.py index f1e926dd75..184a3e4690 100644 --- a/python/qemu/aqmp/aqmp_tui.py +++ b/python/qemu/aqmp/aqmp_tui.py @@ -35,9 +35,8 @@ import urwid import urwid_readline -from qemu.qmp import QEMUMonitorProtocol, QMPBadPortError - from .error import ProtocolError +from .legacy import QEMUMonitorProtocol, QMPBadPortError from .message import DeserializationError, Message, UnexpectedTypeError from .protocol import ConnectError, Runstate from .qmp_client import ExecInterruptedError, QMPClient diff --git a/python/qemu/aqmp/legacy.py b/python/qemu/aqmp/legacy.py index 0890f95b16..76b09671cc 100644 --- a/python/qemu/aqmp/legacy.py +++ b/python/qemu/aqmp/legacy.py @@ -22,9 +22,6 @@ from .qmp_client import QMPClient -# (Temporarily) Re-export QMPBadPortError -QMPBadPortError = qemu.qmp.QMPBadPortError - #: QMPMessage is an entire QMP message of any kind. QMPMessage = Dict[str, Any] @@ -45,6 +42,12 @@ # pylint: disable=missing-docstring +class QMPBadPortError(QMPError): +""" +Unable to parse socket address: Port was non-numerical. +""" + + class QEMUMonitorProtocol(qemu.qmp.QEMUMonitorProtocol): def __init__(self, address: SocketAddrT, server: bool = False, @@ -72,7 +75,26 @@ def _get_greeting(self) -> Optional[QMPMessage]: return None # __enter__ and __exit__ need no changes -# parse_address needs no changes + +@classmethod +def parse_address(cls, address: str) -> SocketAddrT: +""" +Parse a string into a QMP address. + +Figure out if the argument is in the port:host form. +If it's not, it's probably a file path. +""" +components = address.split(':') +if len(components) == 2: +try: +port = int(components[1]) +except ValueError: +msg = f"Bad port: '{components[1]}' in '{address}'." +raise QMPBadPortError(msg) from None +return (components[0], port) + +# Treat as filepath. +return address def connect(self, negotiate: bool = True) -> Optional[QMPMessage]: self._aqmp.await_greeting = negotiate diff --git a/python/qemu/qmp/__init__.py b/python/qemu/qmp/__init__.py index 358c0971d0..4e08641154 100644 --- a/python/qemu/qmp/__init__.py +++ b/python/qemu/qmp/__init__.py @@ -102,12 +102,6 @@ def __init__(self, reply: QMPMessage): self.reply = reply -class QMPBadPortError(QMPError): -""" -Unable to parse socket address: Port was non-numerical. -""" - - class QEMUMonitorProtocol: """ Provide an API to connect to QEMU via QEMU Monitor Protocol (QMP) and then @@ -237,26 +231,6 @@ def __exit__(self, # Implement context manager exit function. self.close() -@classmethod -def parse_address(cls, address: str) -> SocketAddrT: -""" -Parse a string into a QMP address. - -Figure out if the argument is in the port:host form. -If it's not, it's probably a file path. -""" -components = address.split(':') -if len(components) == 2: -try: -port = int(components[1]) -except ValueError: -msg = f"Bad port: '{components[1]}' in '{address}'." -raise QMPBadPortError(msg) from None -return (components[0], port) - -# Treat as filepath. -return address - def connect(self, negotiate: bool = True) -> Optional[QMPMessage]: """ Connect to the QMP Monitor and perform capabilities negotiation. -- 2.31.1
[PATCH v3 21/31] scripts/bench-block-job: switch to AQMP
For this commit, we only need to remove accommodations for the synchronous QMP library. Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Beraldo Leal --- scripts/simplebench/bench_block_job.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/scripts/simplebench/bench_block_job.py b/scripts/simplebench/bench_block_job.py index a403c35b08..af9d1646a4 100755 --- a/scripts/simplebench/bench_block_job.py +++ b/scripts/simplebench/bench_block_job.py @@ -27,7 +27,6 @@ sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python')) from qemu.machine import QEMUMachine -from qemu.qmp import QMPConnectError from qemu.aqmp import ConnectError @@ -50,7 +49,7 @@ def bench_block_job(cmd, cmd_args, qemu_args): vm.launch() except OSError as e: return {'error': 'popen failed: ' + str(e)} -except (QMPConnectError, ConnectError, socket.timeout): +except (ConnectError, socket.timeout): return {'error': 'qemu failed: ' + str(vm.get_log())} try: -- 2.31.1
[PATCH v3 28/31] python: remove the old QMP package
Thank you for your service! Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Beraldo Leal --- python/PACKAGE.rst | 4 +- python/README.rst | 2 +- python/qemu/qmp/README.rst | 9 - python/qemu/qmp/__init__.py | 396 python/qemu/qmp/py.typed| 0 python/setup.cfg| 3 +- 6 files changed, 4 insertions(+), 410 deletions(-) delete mode 100644 python/qemu/qmp/README.rst delete mode 100644 python/qemu/qmp/__init__.py delete mode 100644 python/qemu/qmp/py.typed diff --git a/python/PACKAGE.rst b/python/PACKAGE.rst index b0b86cc4c3..ddfa9ba3f5 100644 --- a/python/PACKAGE.rst +++ b/python/PACKAGE.rst @@ -8,11 +8,11 @@ to change at any time. Usage - -The ``qemu.qmp`` subpackage provides a library for communicating with +The ``qemu.aqmp`` subpackage provides a library for communicating with QMP servers. The ``qemu.machine`` subpackage offers rudimentary facilities for launching and managing QEMU processes. Refer to each package's documentation -(``>>> help(qemu.qmp)``, ``>>> help(qemu.machine)``) +(``>>> help(qemu.aqmp)``, ``>>> help(qemu.machine)``) for more information. Contributing diff --git a/python/README.rst b/python/README.rst index fcf74f69ea..eb5213337d 100644 --- a/python/README.rst +++ b/python/README.rst @@ -3,7 +3,7 @@ QEMU Python Tooling This directory houses Python tooling used by the QEMU project to build, configure, and test QEMU. It is organized by namespace (``qemu``), and -then by package (e.g. ``qemu/machine``, ``qemu/qmp``, etc). +then by package (e.g. ``qemu/machine``, ``qemu/aqmp``, etc). ``setup.py`` is used by ``pip`` to install this tooling to the current environment. ``setup.cfg`` provides the packaging configuration used by diff --git a/python/qemu/qmp/README.rst b/python/qemu/qmp/README.rst deleted file mode 100644 index 5bfb82535f..00 --- a/python/qemu/qmp/README.rst +++ /dev/null @@ -1,9 +0,0 @@ -qemu.qmp package - - -This package provides a library used for connecting to and communicating -with QMP servers. It is used extensively by iotests, vm tests, -avocado tests, and other utilities in the ./scripts directory. It is -not a fully-fledged SDK and is subject to change at any time. - -See the documentation in ``__init__.py`` for more information. diff --git a/python/qemu/qmp/__init__.py b/python/qemu/qmp/__init__.py deleted file mode 100644 index 4e08641154..00 --- a/python/qemu/qmp/__init__.py +++ /dev/null @@ -1,396 +0,0 @@ -""" -QEMU Monitor Protocol (QMP) development library & tooling. - -This package provides a fairly low-level class for communicating to QMP -protocol servers, as implemented by QEMU, the QEMU Guest Agent, and the -QEMU Storage Daemon. This library is not intended for production use. - -`QEMUMonitorProtocol` is the primary class of interest, and all errors -raised derive from `QMPError`. -""" - -# Copyright (C) 2009, 2010 Red Hat Inc. -# -# Authors: -# Luiz Capitulino -# -# This work is licensed under the terms of the GNU GPL, version 2. See -# the COPYING file in the top-level directory. - -import errno -import json -import logging -import socket -import struct -from types import TracebackType -from typing import ( -Any, -Dict, -List, -Optional, -TextIO, -Tuple, -Type, -TypeVar, -Union, -cast, -) - - -#: QMPMessage is an entire QMP message of any kind. -QMPMessage = Dict[str, Any] - -#: QMPReturnValue is the 'return' value of a command. -QMPReturnValue = object - -#: QMPObject is any object in a QMP message. -QMPObject = Dict[str, object] - -# QMPMessage can be outgoing commands or incoming events/returns. -# QMPReturnValue is usually a dict/json object, but due to QAPI's -# 'returns-whitelist', it can actually be anything. -# -# {'return': {}} is a QMPMessage, -# {} is the QMPReturnValue. - - -InternetAddrT = Tuple[str, int] -UnixAddrT = str -SocketAddrT = Union[InternetAddrT, UnixAddrT] - - -class QMPError(Exception): -""" -QMP base exception -""" - - -class QMPConnectError(QMPError): -""" -QMP connection exception -""" - - -class QMPCapabilitiesError(QMPError): -""" -QMP negotiate capabilities exception -""" - - -class QMPTimeoutError(QMPError): -""" -QMP timeout exception -""" - - -class QMPProtocolError(QMPError): -""" -QMP protocol error; unexpected response -""" - - -class QMPResponseError(QMPError): -""" -Represents erroneous QMP monitor reply -""" -def __init__(self, reply: QMPMessage): -try: -desc = reply['error']['desc'] -except KeyError: -desc = reply -super().__init__(desc) -self.reply = reply - - -class QEMUMonitorProtocol: -""" -Provide an API to connect to QEMU via QEMU Monitor Protocol (QMP) and then -allow to handle commands and events. -""" - -#: Logger object for debugging
[PATCH v3 30/31] python: rename qemu.aqmp to qemu.qmp
Now that we are fully switched over to the new QMP library, move it back over the old namespace. This is being done primarily so that we may upload this package simply as "qemu.qmp" without introducing confusion over whether or not "aqmp" is a new protocol or not. The trade-off is increased confusion inside the QEMU developer tree. Sorry! Note: the 'private' member "_aqmp" in legacy.py also changes to "_qmp"; not out of necessity, but just to remove any traces of the "aqmp" name. Signed-off-by: John Snow --- python/PACKAGE.rst| 4 +-- python/README.rst | 4 +-- python/qemu/machine/machine.py| 4 +-- python/qemu/machine/qtest.py | 2 +- python/qemu/{aqmp => qmp}/__init__.py | 6 ++-- python/qemu/{aqmp => qmp}/aqmp_tui.py | 0 python/qemu/{aqmp => qmp}/error.py| 0 python/qemu/{aqmp => qmp}/events.py | 2 +- python/qemu/{aqmp => qmp}/legacy.py | 36 +++ python/qemu/{aqmp => qmp}/message.py | 0 python/qemu/{aqmp => qmp}/models.py | 0 python/qemu/{aqmp => qmp}/protocol.py | 4 +-- python/qemu/{aqmp => qmp}/py.typed| 0 python/qemu/{aqmp => qmp}/qmp_client.py | 16 +- python/qemu/{aqmp => qmp}/qmp_shell.py| 4 +-- python/qemu/{aqmp => qmp}/util.py | 0 python/qemu/utils/qemu_ga_client.py | 4 +-- python/qemu/utils/qom.py | 2 +- python/qemu/utils/qom_common.py | 4 +-- python/qemu/utils/qom_fuse.py | 2 +- python/setup.cfg | 8 ++--- python/tests/protocol.py | 14 - scripts/cpu-x86-uarch-abi.py | 2 +- scripts/device-crash-test | 4 +-- scripts/qmp/qmp-shell | 2 +- scripts/render_block_graph.py | 4 +-- scripts/simplebench/bench_block_job.py| 2 +- tests/qemu-iotests/iotests.py | 2 +- tests/qemu-iotests/tests/mirror-top-perms | 6 ++-- 29 files changed, 69 insertions(+), 69 deletions(-) rename python/qemu/{aqmp => qmp}/__init__.py (87%) rename python/qemu/{aqmp => qmp}/aqmp_tui.py (100%) rename python/qemu/{aqmp => qmp}/error.py (100%) rename python/qemu/{aqmp => qmp}/events.py (99%) rename python/qemu/{aqmp => qmp}/legacy.py (92%) rename python/qemu/{aqmp => qmp}/message.py (100%) rename python/qemu/{aqmp => qmp}/models.py (100%) rename python/qemu/{aqmp => qmp}/protocol.py (99%) rename python/qemu/{aqmp => qmp}/py.typed (100%) rename python/qemu/{aqmp => qmp}/qmp_client.py (97%) rename python/qemu/{aqmp => qmp}/qmp_shell.py (99%) rename python/qemu/{aqmp => qmp}/util.py (100%) diff --git a/python/PACKAGE.rst b/python/PACKAGE.rst index ddfa9ba3f5..b0b86cc4c3 100644 --- a/python/PACKAGE.rst +++ b/python/PACKAGE.rst @@ -8,11 +8,11 @@ to change at any time. Usage - -The ``qemu.aqmp`` subpackage provides a library for communicating with +The ``qemu.qmp`` subpackage provides a library for communicating with QMP servers. The ``qemu.machine`` subpackage offers rudimentary facilities for launching and managing QEMU processes. Refer to each package's documentation -(``>>> help(qemu.aqmp)``, ``>>> help(qemu.machine)``) +(``>>> help(qemu.qmp)``, ``>>> help(qemu.machine)``) for more information. Contributing diff --git a/python/README.rst b/python/README.rst index eb5213337d..9c1fceaee7 100644 --- a/python/README.rst +++ b/python/README.rst @@ -3,7 +3,7 @@ QEMU Python Tooling This directory houses Python tooling used by the QEMU project to build, configure, and test QEMU. It is organized by namespace (``qemu``), and -then by package (e.g. ``qemu/machine``, ``qemu/aqmp``, etc). +then by package (e.g. ``qemu/machine``, ``qemu/qmp``, etc). ``setup.py`` is used by ``pip`` to install this tooling to the current environment. ``setup.cfg`` provides the packaging configuration used by @@ -59,7 +59,7 @@ Package installation also normally provides executable console scripts, so that tools like ``qmp-shell`` are always available via $PATH. To invoke them without installation, you can invoke e.g.: -``> PYTHONPATH=~/src/qemu/python python3 -m qemu.aqmp.qmp_shell`` +``> PYTHONPATH=~/src/qemu/python python3 -m qemu.qmp.qmp_shell`` The mappings between console script name and python module path can be found in ``setup.cfg``. diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py index 21fb4a4f30..6e4bf6520c 100644 --- a/python/qemu/machine/machine.py +++ b/python/qemu/machine/machine.py @@ -40,8 +40,8 @@ TypeVar, ) -from qemu.aqmp import SocketAddrT -from qemu.aqmp.legacy import ( +from qemu.qmp import SocketAddrT +from qemu.qmp.legacy import ( QEMUMonitorProtocol, QMPMessage, QMPReturnValue, diff --git a/python/qemu/machine/qtest.py b/python/qemu/machine/qtest.py index 13e0aaff84..1a1fc6c9b0 100644 --- a/python/qemu/machine/qtest.py +++ b/python/qemu/machine/qtest.py @@ -26,7 +26,7
[PATCH v3 13/31] python/qmp: switch qom tools to AQMP
Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Beraldo Leal --- python/qemu/qmp/qom.py| 5 +++-- python/qemu/qmp/qom_common.py | 3 ++- python/qemu/qmp/qom_fuse.py | 11 ++- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/python/qemu/qmp/qom.py b/python/qemu/qmp/qom.py index 8ff28a8343..bb5d1a78f5 100644 --- a/python/qemu/qmp/qom.py +++ b/python/qemu/qmp/qom.py @@ -32,7 +32,8 @@ import argparse -from . import QMPResponseError +from qemu.aqmp import ExecuteError + from .qom_common import QOMCommand @@ -233,7 +234,7 @@ def _list_node(self, path: str) -> None: rsp = self.qmp.command('qom-get', path=path, property=item.name) print(f" {item.name}: {rsp} ({item.type})") -except QMPResponseError as err: +except ExecuteError as err: print(f" {item.name}: ({item.type})") print('') for item in items: diff --git a/python/qemu/qmp/qom_common.py b/python/qemu/qmp/qom_common.py index 2e4c741f77..e034a6f247 100644 --- a/python/qemu/qmp/qom_common.py +++ b/python/qemu/qmp/qom_common.py @@ -27,7 +27,8 @@ TypeVar, ) -from . import QEMUMonitorProtocol, QMPError +from qemu.aqmp import QMPError +from qemu.aqmp.legacy import QEMUMonitorProtocol class ObjectPropertyInfo: diff --git a/python/qemu/qmp/qom_fuse.py b/python/qemu/qmp/qom_fuse.py index 43f4671fdb..653a76b93b 100644 --- a/python/qemu/qmp/qom_fuse.py +++ b/python/qemu/qmp/qom_fuse.py @@ -48,7 +48,8 @@ import fuse from fuse import FUSE, FuseOSError, Operations -from . import QMPResponseError +from qemu.aqmp import ExecuteError + from .qom_common import QOMCommand @@ -99,7 +100,7 @@ def is_object(self, path: str) -> bool: try: self.qom_list(path) return True -except QMPResponseError: +except ExecuteError: return False def is_property(self, path: str) -> bool: @@ -112,7 +113,7 @@ def is_property(self, path: str) -> bool: if item.name == prop: return True return False -except QMPResponseError: +except ExecuteError: return False def is_link(self, path: str) -> bool: @@ -125,7 +126,7 @@ def is_link(self, path: str) -> bool: if item.name == prop and item.link: return True return False -except QMPResponseError: +except ExecuteError: return False def read(self, path: str, size: int, offset: int, fh: IO[bytes]) -> bytes: @@ -138,7 +139,7 @@ def read(self, path: str, size: int, offset: int, fh: IO[bytes]) -> bytes: try: data = str(self.qmp.command('qom-get', path=path, property=prop)) data += '\n' # make values shell friendly -except QMPResponseError as err: +except ExecuteError as err: raise FuseOSError(EPERM) from err if offset > len(data): -- 2.31.1
[PATCH v3 16/31] python: move qmp-shell under the AQMP package
Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Beraldo Leal --- python/README.rst | 2 +- python/qemu/{qmp => aqmp}/qmp_shell.py | 0 python/setup.cfg | 2 +- scripts/qmp/qmp-shell | 2 +- 4 files changed, 3 insertions(+), 3 deletions(-) rename python/qemu/{qmp => aqmp}/qmp_shell.py (100%) diff --git a/python/README.rst b/python/README.rst index 9c1fceaee7..fcf74f69ea 100644 --- a/python/README.rst +++ b/python/README.rst @@ -59,7 +59,7 @@ Package installation also normally provides executable console scripts, so that tools like ``qmp-shell`` are always available via $PATH. To invoke them without installation, you can invoke e.g.: -``> PYTHONPATH=~/src/qemu/python python3 -m qemu.qmp.qmp_shell`` +``> PYTHONPATH=~/src/qemu/python python3 -m qemu.aqmp.qmp_shell`` The mappings between console script name and python module path can be found in ``setup.cfg``. diff --git a/python/qemu/qmp/qmp_shell.py b/python/qemu/aqmp/qmp_shell.py similarity index 100% rename from python/qemu/qmp/qmp_shell.py rename to python/qemu/aqmp/qmp_shell.py diff --git a/python/setup.cfg b/python/setup.cfg index 78421411d2..168a79c867 100644 --- a/python/setup.cfg +++ b/python/setup.cfg @@ -67,7 +67,7 @@ console_scripts = qom-tree = qemu.utils.qom:QOMTree.entry_point qom-fuse = qemu.utils.qom_fuse:QOMFuse.entry_point [fuse] qemu-ga-client = qemu.utils.qemu_ga_client:main -qmp-shell = qemu.qmp.qmp_shell:main +qmp-shell = qemu.aqmp.qmp_shell:main aqmp-tui = qemu.aqmp.aqmp_tui:main [tui] [flake8] diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell index 4a20f97db7..31b19d73e2 100755 --- a/scripts/qmp/qmp-shell +++ b/scripts/qmp/qmp-shell @@ -4,7 +4,7 @@ import os import sys sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python')) -from qemu.qmp import qmp_shell +from qemu.aqmp import qmp_shell if __name__ == '__main__': -- 2.31.1
[PATCH v3 22/31] iotests/mirror-top-perms: switch to AQMP
Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Beraldo Leal --- Note: I still need to adjust the logging. The problem now is that the logging messages include the PID of the test process, so they need to be filtered out. I'll investigate that for a follow-up. I could just add yet another filtering function somewhere, but I think it's getting out of hand with how many filters and loggers there are, so I want to give it a slightly more serious treatment instead of a hackjob. Signed-off-by: John Snow --- tests/qemu-iotests/tests/mirror-top-perms | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/qemu-iotests/tests/mirror-top-perms b/tests/qemu-iotests/tests/mirror-top-perms index 0a51a613f3..f394931a00 100755 --- a/tests/qemu-iotests/tests/mirror-top-perms +++ b/tests/qemu-iotests/tests/mirror-top-perms @@ -23,7 +23,6 @@ import os from qemu.aqmp import ConnectError from qemu.machine import machine -from qemu.qmp import QMPConnectError import iotests from iotests import change_log_level, qemu_img @@ -101,13 +100,13 @@ class TestMirrorTopPerms(iotests.QMPTestCase): self.vm_b.add_device('virtio-blk,drive=drive0,share-rw=on') try: # Silence AQMP errors temporarily. -# TODO: Remove this and just allow the errors to be logged when -# AQMP fully replaces QMP. +# TODO: Remove change_log_level and allow the errors to be logged. +# This necessitates a PID filter on *all* logging output. with change_log_level('qemu.aqmp'): self.vm_b.launch() print('ERROR: VM B launched successfully, ' 'this should not have happened') -except (QMPConnectError, ConnectError): +except ConnectError: assert 'Is another process using the image' in self.vm_b.get_log() result = self.vm.qmp('block-job-cancel', -- 2.31.1
[PATCH v3 15/31] python: move qmp utilities to python/qemu/utils
In order to upload a QMP package to PyPI, I want to remove any scripts that I am not 100% confident I want to support upstream, beyond our castle walls. Move most of our QMP utilities into the utils package so we can split them out from the PyPI upload. Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy --- python/qemu/{qmp => utils}/qemu_ga_client.py | 0 python/qemu/{qmp => utils}/qom.py| 0 python/qemu/{qmp => utils}/qom_common.py | 0 python/qemu/{qmp => utils}/qom_fuse.py | 0 python/setup.cfg | 16 scripts/qmp/qemu-ga-client | 2 +- scripts/qmp/qom-fuse | 2 +- scripts/qmp/qom-get | 2 +- scripts/qmp/qom-list | 2 +- scripts/qmp/qom-set | 2 +- scripts/qmp/qom-tree | 2 +- 11 files changed, 14 insertions(+), 14 deletions(-) rename python/qemu/{qmp => utils}/qemu_ga_client.py (100%) rename python/qemu/{qmp => utils}/qom.py (100%) rename python/qemu/{qmp => utils}/qom_common.py (100%) rename python/qemu/{qmp => utils}/qom_fuse.py (100%) diff --git a/python/qemu/qmp/qemu_ga_client.py b/python/qemu/utils/qemu_ga_client.py similarity index 100% rename from python/qemu/qmp/qemu_ga_client.py rename to python/qemu/utils/qemu_ga_client.py diff --git a/python/qemu/qmp/qom.py b/python/qemu/utils/qom.py similarity index 100% rename from python/qemu/qmp/qom.py rename to python/qemu/utils/qom.py diff --git a/python/qemu/qmp/qom_common.py b/python/qemu/utils/qom_common.py similarity index 100% rename from python/qemu/qmp/qom_common.py rename to python/qemu/utils/qom_common.py diff --git a/python/qemu/qmp/qom_fuse.py b/python/qemu/utils/qom_fuse.py similarity index 100% rename from python/qemu/qmp/qom_fuse.py rename to python/qemu/utils/qom_fuse.py diff --git a/python/setup.cfg b/python/setup.cfg index 417e937839..78421411d2 100644 --- a/python/setup.cfg +++ b/python/setup.cfg @@ -60,13 +60,13 @@ tui = [options.entry_points] console_scripts = -qom = qemu.qmp.qom:main -qom-set = qemu.qmp.qom:QOMSet.entry_point -qom-get = qemu.qmp.qom:QOMGet.entry_point -qom-list = qemu.qmp.qom:QOMList.entry_point -qom-tree = qemu.qmp.qom:QOMTree.entry_point -qom-fuse = qemu.qmp.qom_fuse:QOMFuse.entry_point [fuse] -qemu-ga-client = qemu.qmp.qemu_ga_client:main +qom = qemu.utils.qom:main +qom-set = qemu.utils.qom:QOMSet.entry_point +qom-get = qemu.utils.qom:QOMGet.entry_point +qom-list = qemu.utils.qom:QOMList.entry_point +qom-tree = qemu.utils.qom:QOMTree.entry_point +qom-fuse = qemu.utils.qom_fuse:QOMFuse.entry_point [fuse] +qemu-ga-client = qemu.utils.qemu_ga_client:main qmp-shell = qemu.qmp.qmp_shell:main aqmp-tui = qemu.aqmp.aqmp_tui:main [tui] @@ -80,7 +80,7 @@ python_version = 3.6 warn_unused_configs = True namespace_packages = True -[mypy-qemu.qmp.qom_fuse] +[mypy-qemu.utils.qom_fuse] # fusepy has no type stubs: allow_subclassing_any = True diff --git a/scripts/qmp/qemu-ga-client b/scripts/qmp/qemu-ga-client index 102fd2cad9..56edd0234a 100755 --- a/scripts/qmp/qemu-ga-client +++ b/scripts/qmp/qemu-ga-client @@ -4,7 +4,7 @@ import os import sys sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python')) -from qemu.qmp import qemu_ga_client +from qemu.utils import qemu_ga_client if __name__ == '__main__': diff --git a/scripts/qmp/qom-fuse b/scripts/qmp/qom-fuse index a58c8ef979..d453807b27 100755 --- a/scripts/qmp/qom-fuse +++ b/scripts/qmp/qom-fuse @@ -4,7 +4,7 @@ import os import sys sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python')) -from qemu.qmp.qom_fuse import QOMFuse +from qemu.utils.qom_fuse import QOMFuse if __name__ == '__main__': diff --git a/scripts/qmp/qom-get b/scripts/qmp/qom-get index e4f3e0c013..04ebe052e8 100755 --- a/scripts/qmp/qom-get +++ b/scripts/qmp/qom-get @@ -4,7 +4,7 @@ import os import sys sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python')) -from qemu.qmp.qom import QOMGet +from qemu.utils.qom import QOMGet if __name__ == '__main__': diff --git a/scripts/qmp/qom-list b/scripts/qmp/qom-list index 7a071a54e1..853b85a8d3 100755 --- a/scripts/qmp/qom-list +++ b/scripts/qmp/qom-list @@ -4,7 +4,7 @@ import os import sys sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python')) -from qemu.qmp.qom import QOMList +from qemu.utils.qom import QOMList if __name__ == '__main__': diff --git a/scripts/qmp/qom-set b/scripts/qmp/qom-set index 9ca9e2ba10..06820feec4 100755 --- a/scripts/qmp/qom-set +++ b/scripts/qmp/qom-set @@ -4,7 +4,7 @@ import os import sys sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python')) -from qemu.qmp.qom import QOMSet +from qemu.utils.qom import QOMSet if __name__ == '__main__': diff --git
[PATCH v3 19/31] scripts/cpu-x86-uarch-abi: switch to AQMP
Signed-off-by: John Snow Reviewed-by: Daniel P. Berrangé Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Beraldo Leal --- scripts/cpu-x86-uarch-abi.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/cpu-x86-uarch-abi.py b/scripts/cpu-x86-uarch-abi.py index 8963d90f0b..c262d2f027 100644 --- a/scripts/cpu-x86-uarch-abi.py +++ b/scripts/cpu-x86-uarch-abi.py @@ -6,7 +6,7 @@ # compatibility levels for each CPU model. # -from qemu import qmp +from qemu.aqmp.legacy import QEMUMonitorProtocol import sys if len(sys.argv) != 2: @@ -66,7 +66,7 @@ sock = sys.argv[1] -shell = qmp.QEMUMonitorProtocol(sock) +shell = QEMUMonitorProtocol(sock) shell.connect() models = shell.cmd("query-cpu-definitions") -- 2.31.1
[PATCH v3 31/31] python: rename 'aqmp-tui' to 'qmp-tui'
This is the last vestige of the "aqmp" moniker surviving in the tree; remove it. Signed-off-by: John Snow --- python/qemu/qmp/{aqmp_tui.py => qmp_tui.py} | 12 ++-- python/setup.cfg| 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) rename python/qemu/qmp/{aqmp_tui.py => qmp_tui.py} (98%) diff --git a/python/qemu/qmp/aqmp_tui.py b/python/qemu/qmp/qmp_tui.py similarity index 98% rename from python/qemu/qmp/aqmp_tui.py rename to python/qemu/qmp/qmp_tui.py index 184a3e4690..17dc94e7c3 100644 --- a/python/qemu/qmp/aqmp_tui.py +++ b/python/qemu/qmp/qmp_tui.py @@ -6,13 +6,13 @@ # This work is licensed under the terms of the GNU GPL, version 2 or # later. See the COPYING file in the top-level directory. """ -AQMP TUI +QMP TUI -AQMP TUI is an asynchronous interface built on top the of the AQMP library. +QMP TUI is an asynchronous interface built on top the of the QMP library. It is the successor of QMP-shell and is bought-in as a replacement for it. -Example Usage: aqmp-tui -Full Usage: aqmp-tui --help +Example Usage: qmp-tui +Full Usage: qmp-tui --help """ import argparse @@ -129,7 +129,7 @@ def has_handler_type(logger: logging.Logger, class App(QMPClient): """ -Implements the AQMP TUI. +Implements the QMP TUI. Initializes the widgets and starts the urwid event loop. @@ -612,7 +612,7 @@ def main() -> None: Driver of the whole script, parses arguments, initialize the TUI and the logger. """ -parser = argparse.ArgumentParser(description='AQMP TUI') +parser = argparse.ArgumentParser(description='QMP TUI') parser.add_argument('qmp_server', help='Address of the QMP server. ' 'Format ') parser.add_argument('--num-retries', type=int, default=10, diff --git a/python/setup.cfg b/python/setup.cfg index 911ae02de7..4614521dea 100644 --- a/python/setup.cfg +++ b/python/setup.cfg @@ -51,7 +51,7 @@ devel = fuse = fusepy >= 2.0.4 -# AQMP TUI dependencies +# QMP TUI dependencies tui = urwid >= 2.1.2 urwid-readline >= 0.13 @@ -67,7 +67,7 @@ console_scripts = qom-fuse = qemu.utils.qom_fuse:QOMFuse.entry_point [fuse] qemu-ga-client = qemu.utils.qemu_ga_client:main qmp-shell = qemu.qmp.qmp_shell:main -aqmp-tui = qemu.qmp.aqmp_tui:main [tui] +qmp-tui = qemu.qmp.qmp_tui:main [tui] [flake8] extend-ignore = E722 # Prefer pylint's bare-except checks to flake8's @@ -83,7 +83,7 @@ namespace_packages = True # fusepy has no type stubs: allow_subclassing_any = True -[mypy-qemu.qmp.aqmp_tui] +[mypy-qemu.qmp.qmp_tui] # urwid and urwid_readline have no type stubs: allow_subclassing_any = True -- 2.31.1
[PATCH v3 17/31] python/machine: permanently switch to AQMP
Remove the QEMU_PYTHON_LEGACY_QMP environment variable, making the switch permanent. Update Exceptions and import paths as necessary. Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Beraldo Leal --- python/qemu/machine/machine.py | 18 +++--- python/qemu/machine/qtest.py | 2 +- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py index 67ab06ca2b..21fb4a4f30 100644 --- a/python/qemu/machine/machine.py +++ b/python/qemu/machine/machine.py @@ -40,21 +40,16 @@ TypeVar, ) -from qemu.qmp import ( # pylint: disable=import-error +from qemu.aqmp import SocketAddrT +from qemu.aqmp.legacy import ( +QEMUMonitorProtocol, QMPMessage, QMPReturnValue, -SocketAddrT, ) from . import console_socket -if os.environ.get('QEMU_PYTHON_LEGACY_QMP'): -from qemu.qmp import QEMUMonitorProtocol -else: -from qemu.aqmp.legacy import QEMUMonitorProtocol - - LOG = logging.getLogger(__name__) @@ -710,8 +705,9 @@ def events_wait(self, :param timeout: Optional timeout, in seconds. See QEMUMonitorProtocol.pull_event. -:raise QMPTimeoutError: If timeout was non-zero and no matching events -were found. +:raise asyncio.TimeoutError: +If timeout was non-zero and no matching events were found. + :return: A QMP event matching the filter criteria. If timeout was 0 and no event matched, None. """ @@ -734,7 +730,7 @@ def _match(event: QMPMessage) -> bool: event = self._qmp.pull_event(wait=timeout) if event is None: # NB: None is only returned when timeout is false-ish. -# Timeouts raise QMPTimeoutError instead! +# Timeouts raise asyncio.TimeoutError instead! break if _match(event): return event diff --git a/python/qemu/machine/qtest.py b/python/qemu/machine/qtest.py index f2f9aaa5e5..13e0aaff84 100644 --- a/python/qemu/machine/qtest.py +++ b/python/qemu/machine/qtest.py @@ -26,7 +26,7 @@ TextIO, ) -from qemu.qmp import SocketAddrT # pylint: disable=import-error +from qemu.aqmp import SocketAddrT from .machine import QEMUMachine -- 2.31.1
[PATCH v3 23/31] iotests: switch to AQMP
Simply import the type defition from the new location. Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Beraldo Leal --- tests/qemu-iotests/iotests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index 1e2f2391d1..98bc50cb3a 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -37,7 +37,7 @@ from contextlib import contextmanager from qemu.machine import qtest -from qemu.qmp import QMPMessage +from qemu.aqmp.legacy import QMPMessage # Use this logger for logging messages directly from the iotests module logger = logging.getLogger('qemu.iotests') -- 2.31.1
[PATCH v3 24/31] python: temporarily silence pylint duplicate-code warnings
The next several commits copy some code from qemu.qmp to qemu.aqmp, then delete qemu.qmp. In the interim, to prevent test failures, the duplicate code detection needs to be silenced to prevent bisect problems with CI testing. Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy --- python/setup.cfg | 1 + 1 file changed, 1 insertion(+) diff --git a/python/setup.cfg b/python/setup.cfg index 168a79c867..510df23698 100644 --- a/python/setup.cfg +++ b/python/setup.cfg @@ -115,6 +115,7 @@ ignore_missing_imports = True disable=consider-using-f-string, too-many-function-args, # mypy handles this with less false positives. no-member, # mypy also handles this better. +duplicate-code, # To be removed by the end of this patch series. [pylint.basic] # Good variable names which should always be accepted, separated by a comma. -- 2.31.1
[PATCH v3 20/31] scripts/render-block-graph: switch to AQMP
Creating an instance of qemu.aqmp.ExecuteError is too involved here, so just drop the specificity down to a generic QMPError. Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Beraldo Leal --- scripts/render_block_graph.py | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/scripts/render_block_graph.py b/scripts/render_block_graph.py index da6acf050d..97778927f3 100755 --- a/scripts/render_block_graph.py +++ b/scripts/render_block_graph.py @@ -25,10 +25,8 @@ from graphviz import Digraph sys.path.append(os.path.join(os.path.dirname(__file__), '..', 'python')) -from qemu.qmp import ( -QEMUMonitorProtocol, -QMPResponseError, -) +from qemu.aqmp import QMPError +from qemu.aqmp.legacy import QEMUMonitorProtocol def perm(arr): @@ -105,7 +103,7 @@ def command(self, cmd): reply = json.loads(subprocess.check_output(ar)) if 'error' in reply: -raise QMPResponseError(reply) +raise QMPError(reply) return reply['return'] -- 2.31.1
[PATCH v3 09/31] python/aqmp: add SocketAddrT to package root
It's a commonly needed definition, it can be re-exported by the root. Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Beraldo Leal --- python/qemu/aqmp/__init__.py | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/python/qemu/aqmp/__init__.py b/python/qemu/aqmp/__init__.py index 173556404d..05f467c141 100644 --- a/python/qemu/aqmp/__init__.py +++ b/python/qemu/aqmp/__init__.py @@ -26,7 +26,12 @@ from .error import AQMPError from .events import EventListener from .message import Message -from .protocol import ConnectError, Runstate, StateError +from .protocol import ( +ConnectError, +Runstate, +SocketAddrT, +StateError, +) from .qmp_client import ExecInterruptedError, ExecuteError, QMPClient @@ -48,4 +53,7 @@ 'ConnectError', 'ExecuteError', 'ExecInterruptedError', + +# Type aliases +'SocketAddrT', ) -- 2.31.1
[PATCH v3 18/31] scripts/cpu-x86-uarch-abi: fix CLI parsing
Signed-off-by: John Snow Reviewed-by: Daniel P. Berrangé Reviewed-by: Vladimir Sementsov-Ogievskiy --- scripts/cpu-x86-uarch-abi.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/scripts/cpu-x86-uarch-abi.py b/scripts/cpu-x86-uarch-abi.py index 08acc52a81..8963d90f0b 100644 --- a/scripts/cpu-x86-uarch-abi.py +++ b/scripts/cpu-x86-uarch-abi.py @@ -9,7 +9,7 @@ from qemu import qmp import sys -if len(sys.argv) != 1: +if len(sys.argv) != 2: print("syntax: %s QMP-SOCK\n\n" % __file__ + "Where QMP-SOCK points to a QEMU process such as\n\n" + " # qemu-system-x86_64 -qmp unix:/tmp/qmp,server,nowait " + @@ -66,7 +66,6 @@ sock = sys.argv[1] -cmd = sys.argv[2] shell = qmp.QEMUMonitorProtocol(sock) shell.connect() -- 2.31.1
[PATCH v3 08/31] python/aqmp: copy type definitions from qmp
Copy the remaining type definitions from QMP into the qemu.aqmp.legacy module. Now, users that require the legacy interface don't need to import anything else but qemu.aqmp.legacy wrapper. Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy --- python/qemu/aqmp/legacy.py | 22 -- python/qemu/aqmp/protocol.py | 16 ++-- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/python/qemu/aqmp/legacy.py b/python/qemu/aqmp/legacy.py index 2ccb136b02..9431fe9330 100644 --- a/python/qemu/aqmp/legacy.py +++ b/python/qemu/aqmp/legacy.py @@ -6,7 +6,9 @@ import asyncio from typing import ( +Any, Awaitable, +Dict, List, Optional, TypeVar, @@ -14,13 +16,29 @@ ) import qemu.qmp -from qemu.qmp import QMPMessage, QMPReturnValue, SocketAddrT from .error import AQMPError -from .protocol import Runstate +from .protocol import Runstate, SocketAddrT from .qmp_client import QMPClient +#: QMPMessage is an entire QMP message of any kind. +QMPMessage = Dict[str, Any] + +#: QMPReturnValue is the 'return' value of a command. +QMPReturnValue = object + +#: QMPObject is any object in a QMP message. +QMPObject = Dict[str, object] + +# QMPMessage can be outgoing commands or incoming events/returns. +# QMPReturnValue is usually a dict/json object, but due to QAPI's +# 'returns-whitelist', it can actually be anything. +# +# {'return': {}} is a QMPMessage, +# {} is the QMPReturnValue. + + # pylint: disable=missing-docstring diff --git a/python/qemu/aqmp/protocol.py b/python/qemu/aqmp/protocol.py index c4fbe35a0e..5b4f2f0d0a 100644 --- a/python/qemu/aqmp/protocol.py +++ b/python/qemu/aqmp/protocol.py @@ -46,6 +46,10 @@ _U = TypeVar('_U') _TaskFN = Callable[[], Awaitable[None]] # aka ``async def func() -> None`` +InternetAddrT = Tuple[str, int] +UnixAddrT = str +SocketAddrT = Union[UnixAddrT, InternetAddrT] + class Runstate(Enum): """Protocol session runstate.""" @@ -257,7 +261,7 @@ async def runstate_changed(self) -> Runstate: @upper_half @require(Runstate.IDLE) -async def accept(self, address: Union[str, Tuple[str, int]], +async def accept(self, address: SocketAddrT, ssl: Optional[SSLContext] = None) -> None: """ Accept a connection and begin processing message queues. @@ -275,7 +279,7 @@ async def accept(self, address: Union[str, Tuple[str, int]], @upper_half @require(Runstate.IDLE) -async def connect(self, address: Union[str, Tuple[str, int]], +async def connect(self, address: SocketAddrT, ssl: Optional[SSLContext] = None) -> None: """ Connect to the server and begin processing message queues. @@ -337,7 +341,7 @@ def _set_state(self, state: Runstate) -> None: @upper_half async def _new_session(self, - address: Union[str, Tuple[str, int]], + address: SocketAddrT, ssl: Optional[SSLContext] = None, accept: bool = False) -> None: """ @@ -397,7 +401,7 @@ async def _new_session(self, @upper_half async def _establish_connection( self, -address: Union[str, Tuple[str, int]], +address: SocketAddrT, ssl: Optional[SSLContext] = None, accept: bool = False ) -> None: @@ -424,7 +428,7 @@ async def _establish_connection( await self._do_connect(address, ssl) @upper_half -async def _do_accept(self, address: Union[str, Tuple[str, int]], +async def _do_accept(self, address: SocketAddrT, ssl: Optional[SSLContext] = None) -> None: """ Acting as the transport server, accept a single connection. @@ -482,7 +486,7 @@ async def _client_connected_cb(reader: asyncio.StreamReader, self.logger.debug("Connection accepted.") @upper_half -async def _do_connect(self, address: Union[str, Tuple[str, int]], +async def _do_connect(self, address: SocketAddrT, ssl: Optional[SSLContext] = None) -> None: """ Acting as the transport client, initiate a connection to a server. -- 2.31.1
[PATCH v3 11/31] python/qemu-ga-client: don't use deprecated CLI syntax in usage comment
Cleanup related to commit ccd3b3b8112b670f, "qemu-option: warn for short-form boolean options". Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Daniel P. Berrangé --- python/qemu/qmp/qemu_ga_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/qemu/qmp/qemu_ga_client.py b/python/qemu/qmp/qemu_ga_client.py index 67ac0b4211..b3e1d98c9e 100644 --- a/python/qemu/qmp/qemu_ga_client.py +++ b/python/qemu/qmp/qemu_ga_client.py @@ -5,7 +5,7 @@ Start QEMU with: -# qemu [...] -chardev socket,path=/tmp/qga.sock,server,wait=off,id=qga0 \ +# qemu [...] -chardev socket,path=/tmp/qga.sock,server=on,wait=off,id=qga0 \ -device virtio-serial \ -device virtserialport,chardev=qga0,name=org.qemu.guest_agent.0 -- 2.31.1
[PATCH v3 06/31] python/aqmp: add __del__ method to legacy interface
asyncio can complain *very* loudly if you forget to back out of things gracefully before the garbage collector starts destroying objects that contain live references to asyncio Tasks. The usual fix is just to remember to call aqmp.disconnect(), but for the sake of the legacy wrapper and quick, one-off scripts where a graceful shutdown is not necessarily of paramount imporance, add a courtesy cleanup that will trigger prior to seeing screenfuls of confusing asyncio tracebacks. Note that we can't *always* save you from yourself; depending on when the GC runs, you might just seriously be out of luck. The best we can do in this case is to gently remind you to clean up after yourself. (Still much better than multiple pages of incomprehensible python warnings for the crime of forgetting to put your toys away.) Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Beraldo Leal --- python/qemu/aqmp/legacy.py | 18 ++ 1 file changed, 18 insertions(+) diff --git a/python/qemu/aqmp/legacy.py b/python/qemu/aqmp/legacy.py index 9e7b9fb80b..2ccb136b02 100644 --- a/python/qemu/aqmp/legacy.py +++ b/python/qemu/aqmp/legacy.py @@ -16,6 +16,8 @@ import qemu.qmp from qemu.qmp import QMPMessage, QMPReturnValue, SocketAddrT +from .error import AQMPError +from .protocol import Runstate from .qmp_client import QMPClient @@ -136,3 +138,19 @@ def settimeout(self, timeout: Optional[float]) -> None: def send_fd_scm(self, fd: int) -> None: self._aqmp.send_fd_scm(fd) + +def __del__(self) -> None: +if self._aqmp.runstate == Runstate.IDLE: +return + +if not self._aloop.is_running(): +self.close() +else: +# Garbage collection ran while the event loop was running. +# Nothing we can do about it now, but if we don't raise our +# own error, the user will be treated to a lot of traceback +# they might not understand. +raise AQMPError( +"QEMUMonitorProtocol.close()" +" was not called before object was garbage collected" +) -- 2.31.1
[PATCH v3 14/31] python/qmp: switch qmp-shell to AQMP
We have a replacement for async QMP, but it doesn't have feature parity yet. For now, then, port the old tool onto the new backend. Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy --- python/qemu/aqmp/legacy.py | 3 +++ python/qemu/qmp/qmp_shell.py | 31 +-- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/python/qemu/aqmp/legacy.py b/python/qemu/aqmp/legacy.py index 27df22818a..0890f95b16 100644 --- a/python/qemu/aqmp/legacy.py +++ b/python/qemu/aqmp/legacy.py @@ -22,6 +22,9 @@ from .qmp_client import QMPClient +# (Temporarily) Re-export QMPBadPortError +QMPBadPortError = qemu.qmp.QMPBadPortError + #: QMPMessage is an entire QMP message of any kind. QMPMessage = Dict[str, Any] diff --git a/python/qemu/qmp/qmp_shell.py b/python/qemu/qmp/qmp_shell.py index e7d7eb18f1..d11bf54b00 100644 --- a/python/qemu/qmp/qmp_shell.py +++ b/python/qemu/qmp/qmp_shell.py @@ -95,8 +95,13 @@ Sequence, ) -from qemu import qmp -from qemu.qmp import QMPMessage +from qemu.aqmp import ConnectError, QMPError, SocketAddrT +from qemu.aqmp.legacy import ( +QEMUMonitorProtocol, +QMPBadPortError, +QMPMessage, +QMPObject, +) LOG = logging.getLogger(__name__) @@ -125,7 +130,7 @@ def complete(self, text: str, state: int) -> Optional[str]: return None -class QMPShellError(qmp.QMPError): +class QMPShellError(QMPError): """ QMP Shell Base error class. """ @@ -153,7 +158,7 @@ def visit_Name(cls, # pylint: disable=invalid-name return node -class QMPShell(qmp.QEMUMonitorProtocol): +class QMPShell(QEMUMonitorProtocol): """ QMPShell provides a basic readline-based QMP shell. @@ -161,7 +166,7 @@ class QMPShell(qmp.QEMUMonitorProtocol): :param pretty: Pretty-print QMP messages. :param verbose: Echo outgoing QMP messages to console. """ -def __init__(self, address: qmp.SocketAddrT, +def __init__(self, address: SocketAddrT, pretty: bool = False, verbose: bool = False): super().__init__(address) self._greeting: Optional[QMPMessage] = None @@ -237,7 +242,7 @@ def _parse_value(cls, val: str) -> object: def _cli_expr(self, tokens: Sequence[str], - parent: qmp.QMPObject) -> None: + parent: QMPObject) -> None: for arg in tokens: (key, sep, val) = arg.partition('=') if sep != '=': @@ -403,7 +408,7 @@ class HMPShell(QMPShell): :param pretty: Pretty-print QMP messages. :param verbose: Echo outgoing QMP messages to console. """ -def __init__(self, address: qmp.SocketAddrT, +def __init__(self, address: SocketAddrT, pretty: bool = False, verbose: bool = False): super().__init__(address, pretty, verbose) self._cpu_index = 0 @@ -512,19 +517,17 @@ def main() -> None: try: address = shell_class.parse_address(args.qmp_server) -except qmp.QMPBadPortError: +except QMPBadPortError: parser.error(f"Bad port number: {args.qmp_server}") return # pycharm doesn't know error() is noreturn with shell_class(address, args.pretty, args.verbose) as qemu: try: qemu.connect(negotiate=not args.skip_negotiation) -except qmp.QMPConnectError: -die("Didn't get QMP greeting message") -except qmp.QMPCapabilitiesError: -die("Couldn't negotiate capabilities") -except OSError as err: -die(f"Couldn't connect to {args.qmp_server}: {err!s}") +except ConnectError as err: +if isinstance(err.exc, OSError): +die(f"Couldn't connect to {args.qmp_server}: {err!s}") +die(str(err)) for _ in qemu.repl(): pass -- 2.31.1
[PATCH v3 12/31] python/qmp: switch qemu-ga-client to AQMP
Async QMP always raises a "ConnectError" on any connection error which houses the cause in a second exception. We can check if this root cause was python's ConnectionError to determine a fairly similar condition to the original error check here. Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Beraldo Leal --- python/qemu/qmp/qemu_ga_client.py | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/python/qemu/qmp/qemu_ga_client.py b/python/qemu/qmp/qemu_ga_client.py index b3e1d98c9e..15ed430c61 100644 --- a/python/qemu/qmp/qemu_ga_client.py +++ b/python/qemu/qmp/qemu_ga_client.py @@ -37,8 +37,8 @@ # the COPYING file in the top-level directory. import argparse +import asyncio import base64 -import errno import os import random import sys @@ -50,8 +50,8 @@ Sequence, ) -from qemu import qmp -from qemu.qmp import SocketAddrT +from qemu.aqmp import ConnectError, SocketAddrT +from qemu.aqmp.legacy import QEMUMonitorProtocol # This script has not seen many patches or careful attention in quite @@ -61,7 +61,7 @@ # pylint: disable=missing-docstring -class QemuGuestAgent(qmp.QEMUMonitorProtocol): +class QemuGuestAgent(QEMUMonitorProtocol): def __getattr__(self, name: str) -> Callable[..., Any]: def wrapper(**kwds: object) -> object: return self.command('guest-' + name.replace('_', '-'), **kwds) @@ -149,7 +149,7 @@ def ping(self, timeout: Optional[float]) -> bool: self.qga.settimeout(timeout) try: self.qga.ping() -except TimeoutError: +except asyncio.TimeoutError: return False return True @@ -172,7 +172,7 @@ def suspend(self, mode: str) -> None: try: getattr(self.qga, 'suspend' + '_' + mode)() # On error exception will raise -except TimeoutError: +except asyncio.TimeoutError: # On success command will timed out return @@ -182,7 +182,7 @@ def shutdown(self, mode: str = 'powerdown') -> None: try: self.qga.shutdown(mode=mode) -except TimeoutError: +except asyncio.TimeoutError: pass @@ -277,7 +277,7 @@ def _cmd_reboot(client: QemuGuestAgentClient, args: Sequence[str]) -> None: def send_command(address: str, cmd: str, args: Sequence[str]) -> None: if not os.path.exists(address): -print('%s not found' % address) +print(f"'{address}' not found. (Is QEMU running?)") sys.exit(1) if cmd not in commands: @@ -287,10 +287,10 @@ def send_command(address: str, cmd: str, args: Sequence[str]) -> None: try: client = QemuGuestAgentClient(address) -except OSError as err: +except ConnectError as err: print(err) -if err.errno == errno.ECONNREFUSED: -print('Hint: qemu is not running?') +if isinstance(err.exc, ConnectionError): +print('(Is QEMU running?)') sys.exit(1) if cmd == 'fsfreeze' and args[0] == 'freeze': -- 2.31.1
[PATCH v3 10/31] python/aqmp: rename AQMPError to QMPError
This is in preparation for renaming qemu.aqmp to qemu.qmp. I should have done this from this from the very beginning, but it's a convenient time to make sure this churn is taken care of. Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy --- python/qemu/aqmp/__init__.py | 6 +++--- python/qemu/aqmp/error.py | 12 ++-- python/qemu/aqmp/events.py | 4 ++-- python/qemu/aqmp/legacy.py | 4 ++-- python/qemu/aqmp/protocol.py | 8 python/qemu/aqmp/qmp_client.py | 8 6 files changed, 21 insertions(+), 21 deletions(-) diff --git a/python/qemu/aqmp/__init__.py b/python/qemu/aqmp/__init__.py index 05f467c141..4c22c38079 100644 --- a/python/qemu/aqmp/__init__.py +++ b/python/qemu/aqmp/__init__.py @@ -6,7 +6,7 @@ QEMU Guest Agent, and the QEMU Storage Daemon. `QMPClient` provides the main functionality of this package. All errors -raised by this library derive from `AQMPError`, see `aqmp.error` for +raised by this library derive from `QMPError`, see `aqmp.error` for additional detail. See `aqmp.events` for an in-depth tutorial on managing QMP events. """ @@ -23,7 +23,7 @@ import logging -from .error import AQMPError +from .error import QMPError from .events import EventListener from .message import Message from .protocol import ( @@ -48,7 +48,7 @@ 'Runstate', # Exceptions, most generic to most explicit -'AQMPError', +'QMPError', 'StateError', 'ConnectError', 'ExecuteError', diff --git a/python/qemu/aqmp/error.py b/python/qemu/aqmp/error.py index 781f49b008..24ba4d5054 100644 --- a/python/qemu/aqmp/error.py +++ b/python/qemu/aqmp/error.py @@ -1,21 +1,21 @@ """ -AQMP Error Classes +QMP Error Classes This package seeks to provide semantic error classes that are intended to be used directly by clients when they would like to handle particular semantic failures (e.g. "failed to connect") without needing to know the enumeration of possible reasons for that failure. -AQMPError serves as the ancestor for all exceptions raised by this +QMPError serves as the ancestor for all exceptions raised by this package, and is suitable for use in handling semantic errors from this library. In most cases, individual public methods will attempt to catch and re-encapsulate various exceptions to provide a semantic error-handling interface. -.. admonition:: AQMP Exception Hierarchy Reference +.. admonition:: QMP Exception Hierarchy Reference | `Exception` - |+-- `AQMPError` + |+-- `QMPError` | +-- `ConnectError` | +-- `StateError` | +-- `ExecInterruptedError` @@ -31,11 +31,11 @@ """ -class AQMPError(Exception): +class QMPError(Exception): """Abstract error class for all errors originating from this package.""" -class ProtocolError(AQMPError): +class ProtocolError(QMPError): """ Abstract error class for protocol failures. diff --git a/python/qemu/aqmp/events.py b/python/qemu/aqmp/events.py index 5f7150c78d..f3d4e2b5e8 100644 --- a/python/qemu/aqmp/events.py +++ b/python/qemu/aqmp/events.py @@ -443,7 +443,7 @@ def accept(self, event) -> bool: Union, ) -from .error import AQMPError +from .error import QMPError from .message import Message @@ -451,7 +451,7 @@ def accept(self, event) -> bool: EventFilter = Callable[[Message], bool] -class ListenerError(AQMPError): +class ListenerError(QMPError): """ Generic error class for `EventListener`-related problems. """ diff --git a/python/qemu/aqmp/legacy.py b/python/qemu/aqmp/legacy.py index 9431fe9330..27df22818a 100644 --- a/python/qemu/aqmp/legacy.py +++ b/python/qemu/aqmp/legacy.py @@ -17,7 +17,7 @@ import qemu.qmp -from .error import AQMPError +from .error import QMPError from .protocol import Runstate, SocketAddrT from .qmp_client import QMPClient @@ -168,7 +168,7 @@ def __del__(self) -> None: # Nothing we can do about it now, but if we don't raise our # own error, the user will be treated to a lot of traceback # they might not understand. -raise AQMPError( +raise QMPError( "QEMUMonitorProtocol.close()" " was not called before object was garbage collected" ) diff --git a/python/qemu/aqmp/protocol.py b/python/qemu/aqmp/protocol.py index 5b4f2f0d0a..50e973c2f2 100644 --- a/python/qemu/aqmp/protocol.py +++ b/python/qemu/aqmp/protocol.py @@ -29,7 +29,7 @@ cast, ) -from .error import AQMPError +from .error import QMPError from .util import ( bottom_half, create_task, @@ -65,7 +65,7 @@ class Runstate(Enum): DISCONNECTING = 3 -class ConnectError(AQMPError): +class ConnectError(QMPError): """ Raised when the initial connection process has failed. @@ -90,7 +90,7 @@ def __str__(self) -> str: return f"{self.error_message}: {cause}" -class StateError(AQMPError): +class StateError(QMPError): """ An
[PATCH v3 03/31] python: update type hints for mypy 0.930
Mypy 0.930, released Dec 22, changes the way argparse objects are considered. Crafting a definition that works under Python 3.6 and an older mypy alongside newer versions simultaneously is ... difficult, so... eh. Stub it out with an 'Any' definition to get the CI moving again. Oh well. Signed-off-by: John Snow Message-id: 20220110191349.1841027-4-js...@redhat.com Signed-off-by: John Snow --- python/qemu/qmp/qom_common.py | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/python/qemu/qmp/qom_common.py b/python/qemu/qmp/qom_common.py index a59ae1a2a1..2e4c741f77 100644 --- a/python/qemu/qmp/qom_common.py +++ b/python/qemu/qmp/qom_common.py @@ -30,10 +30,6 @@ from . import QEMUMonitorProtocol, QMPError -# The following is needed only for a type alias. -Subparsers = argparse._SubParsersAction # pylint: disable=protected-access - - class ObjectPropertyInfo: """ Represents the return type from e.g. qom-list. @@ -89,7 +85,7 @@ def __init__(self, args: argparse.Namespace): self.qmp.connect() @classmethod -def register(cls, subparsers: Subparsers) -> None: +def register(cls, subparsers: Any) -> None: """ Register this command with the argument parser. -- 2.31.1
[PATCH v3 07/31] python/aqmp: handle asyncio.TimeoutError on execute()
This exception can be injected into any await statement. If we are canceled via timeout, we want to clear the pending execution record on our way out. Signed-off-by: John Snow Reviewed-by: Beraldo Leal Reviewed-by: Vladimir Sementsov-Ogievskiy --- python/qemu/aqmp/qmp_client.py | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/python/qemu/aqmp/qmp_client.py b/python/qemu/aqmp/qmp_client.py index 8105e29fa8..6a985ffe30 100644 --- a/python/qemu/aqmp/qmp_client.py +++ b/python/qemu/aqmp/qmp_client.py @@ -435,7 +435,11 @@ async def _issue(self, msg: Message) -> Union[None, str]: msg_id = msg['id'] self._pending[msg_id] = asyncio.Queue(maxsize=1) -await self._outgoing.put(msg) +try: +await self._outgoing.put(msg) +except: +del self._pending[msg_id] +raise return msg_id @@ -452,9 +456,9 @@ async def _reply(self, msg_id: Union[str, None]) -> Message: was lost, or some other problem. """ queue = self._pending[msg_id] -result = await queue.get() try: +result = await queue.get() if isinstance(result, ExecInterruptedError): raise result return result -- 2.31.1
[PATCH v3 04/31] simplebench: Fix Python syntax error (reported by LGTM)
From: Stefan Weil Fixes: b2fcb0c5754c2554b8406376e99a75e9e0a6b7bd Signed-off-by: Stefan Weil Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: John Snow Message-id: 20220107153019.504124-1...@weilnetz.de Signed-off-by: John Snow --- scripts/simplebench/bench-example.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/simplebench/bench-example.py b/scripts/simplebench/bench-example.py index 4864435f39..fc370691e0 100644 --- a/scripts/simplebench/bench-example.py +++ b/scripts/simplebench/bench-example.py @@ -25,7 +25,7 @@ def bench_func(env, case): """ Handle one "cell" of benchmarking table. """ -return bench_block_copy(env['qemu_binary'], env['cmd'], {} +return bench_block_copy(env['qemu_binary'], env['cmd'], {}, case['source'], case['target']) -- 2.31.1
[PATCH v3 01/31] python/aqmp: use absolute import statement
pylint's dependency astroid appears to have bugs in 2.9.1 and 2.9.2 (Dec 31 and Jan 3) that appear to erroneously expect the qemu namespace to have an __init__.py file. astroid 2.9.3 (Jan 9) avoids that problem, but appears to not understand a relative import within a namespace package. Update the relative import - it was worth changing anyway, because these packages will eventually be packaged and distributed separately. Signed-off-by: John Snow Message-id: 20220110191349.1841027-2-js...@redhat.com Signed-off-by: John Snow --- python/qemu/aqmp/aqmp_tui.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/python/qemu/aqmp/aqmp_tui.py b/python/qemu/aqmp/aqmp_tui.py index a2929f771c..f1e926dd75 100644 --- a/python/qemu/aqmp/aqmp_tui.py +++ b/python/qemu/aqmp/aqmp_tui.py @@ -35,7 +35,8 @@ import urwid import urwid_readline -from ..qmp import QEMUMonitorProtocol, QMPBadPortError +from qemu.qmp import QEMUMonitorProtocol, QMPBadPortError + from .error import ProtocolError from .message import DeserializationError, Message, UnexpectedTypeError from .protocol import ConnectError, Runstate -- 2.31.1
[PATCH v3 05/31] python/aqmp: fix docstring typo
Reported-by: Vladimir Sementsov-Ogievskiy Signed-off-by: John Snow --- python/qemu/aqmp/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/qemu/aqmp/__init__.py b/python/qemu/aqmp/__init__.py index 880d5b6fa7..173556404d 100644 --- a/python/qemu/aqmp/__init__.py +++ b/python/qemu/aqmp/__init__.py @@ -6,7 +6,7 @@ QEMU Guest Agent, and the QEMU Storage Daemon. `QMPClient` provides the main functionality of this package. All errors -raised by this library dervive from `AQMPError`, see `aqmp.error` for +raised by this library derive from `AQMPError`, see `aqmp.error` for additional detail. See `aqmp.events` for an in-depth tutorial on managing QMP events. """ -- 2.31.1
[PATCH v3 02/31] Python/aqmp: fix type definitions for mypy 0.920
0.920 (Released 2021-12-15) is not entirely happy with the way that I was defining _FutureT: qemu/aqmp/protocol.py:601: error: Item "object" of the upper bound "Optional[Future[Any]]" of type variable "_FutureT" has no attribute "done" Update it with something a little mechanically simpler that works better across a wider array of mypy versions. Signed-off-by: John Snow Message-id: 20220110191349.1841027-3-js...@redhat.com Signed-off-by: John Snow --- python/qemu/aqmp/protocol.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/python/qemu/aqmp/protocol.py b/python/qemu/aqmp/protocol.py index 5190b33b13..c4fbe35a0e 100644 --- a/python/qemu/aqmp/protocol.py +++ b/python/qemu/aqmp/protocol.py @@ -43,8 +43,8 @@ T = TypeVar('T') +_U = TypeVar('_U') _TaskFN = Callable[[], Awaitable[None]] # aka ``async def func() -> None`` -_FutureT = TypeVar('_FutureT', bound=Optional['asyncio.Future[Any]']) class Runstate(Enum): @@ -591,7 +591,8 @@ def _cleanup(self) -> None: """ Fully reset this object to a clean state and return to `IDLE`. """ -def _paranoid_task_erase(task: _FutureT) -> Optional[_FutureT]: +def _paranoid_task_erase(task: Optional['asyncio.Future[_U]'] + ) -> Optional['asyncio.Future[_U]']: # Help to erase a task, ENSURING it is fully quiesced first. assert (task is None) or task.done() return None if (task and task.done()) else task -- 2.31.1
[PATCH v3 00/31] Python: delete synchronous qemu.qmp package
Based-on: <20220110232521.1922962-1-js...@redhat.com> (jsnow/python staging branch) GitLab: https://gitlab.com/jsnow/qemu/-/commits/python-qmp-legacy-switch CI: https://gitlab.com/jsnow/qemu/-/pipelines/445163212 Hi, this series is part of an effort to publish the qemu.qmp package on PyPI. It is the first of three series to complete this work: --> (1) Switch the new Async QMP library in to python/qemu/qmp (2) Fork python/qemu/qmp out into its own repository, with updated GitLab CI/CD targets to build packages. (3) Update qemu.git to install qemu.qmp from PyPI, and then delete python/qemu/qmp. This series swaps out qemu.qmp for qemu.aqmp permanently, instead of hiding it behind an environment variable toggle. This leaves us with just one QMP library to worry about. It also implements the rename of "qemu.aqmp" to "qemu.qmp". I suspect the most potential disruption to iotest and avocado maintainers, as those two subsystems rely on the QMP features the most. Would appreciate at least an ACK from each of those camps if you're willing to give benefit-of-the-doubt on the actual Python code. V3: - Rebased on top of jsnow/python (For GitLab CI fixes) - Added a new patch 001 to fix a typo Vladimir found. - Tiny change in 006 due to the new patch 001 - Reworded subject of patch 007 - Changed import statement in patch 013 (Vladimir) - Rebase-related changes in patch 021 - Removed 'aqmp' from internal variable names in 026 - Added new patch to rename aqmp-tui to qmp-tui in 027 V2: - Integrate the renaming of qemu.aqmp to qemu.qmp in this series - Minor bits and pieces. John Snow (30): python/aqmp: use absolute import statement Python/aqmp: fix type definitions for mypy 0.920 python: update type hints for mypy 0.930 python/aqmp: fix docstring typo python/aqmp: add __del__ method to legacy interface python/aqmp: handle asyncio.TimeoutError on execute() python/aqmp: copy type definitions from qmp python/aqmp: add SocketAddrT to package root python/aqmp: rename AQMPError to QMPError python/qemu-ga-client: don't use deprecated CLI syntax in usage comment python/qmp: switch qemu-ga-client to AQMP python/qmp: switch qom tools to AQMP python/qmp: switch qmp-shell to AQMP python: move qmp utilities to python/qemu/utils python: move qmp-shell under the AQMP package python/machine: permanently switch to AQMP scripts/cpu-x86-uarch-abi: fix CLI parsing scripts/cpu-x86-uarch-abi: switch to AQMP scripts/render-block-graph: switch to AQMP scripts/bench-block-job: switch to AQMP iotests/mirror-top-perms: switch to AQMP iotests: switch to AQMP python: temporarily silence pylint duplicate-code warnings python/aqmp: take QMPBadPortError and parse_address from qemu.qmp python/aqmp: fully separate from qmp.QEMUMonitorProtocol python/aqmp: copy qmp docstrings to qemu.aqmp.legacy python: remove the old QMP package python: re-enable pylint duplicate-code warnings python: rename qemu.aqmp to qemu.qmp python: rename 'aqmp-tui' to 'qmp-tui' Stefan Weil (1): simplebench: Fix Python syntax error (reported by LGTM) python/qemu/qmp/README.rst| 9 - python/qemu/aqmp/__init__.py | 51 -- python/qemu/aqmp/legacy.py| 138 -- python/qemu/aqmp/py.typed | 0 python/qemu/machine/machine.py| 18 +- python/qemu/machine/qtest.py | 2 +- python/qemu/qmp/__init__.py | 441 ++ python/qemu/{aqmp => qmp}/error.py| 12 +- python/qemu/{aqmp => qmp}/events.py | 6 +- python/qemu/qmp/legacy.py | 319 + python/qemu/{aqmp => qmp}/message.py | 0 python/qemu/{aqmp => qmp}/models.py | 0 python/qemu/{aqmp => qmp}/protocol.py | 33 +- python/qemu/{aqmp => qmp}/qmp_client.py | 32 +- python/qemu/qmp/qmp_shell.py | 31 +- .../qemu/{aqmp/aqmp_tui.py => qmp/qmp_tui.py} | 14 +- python/qemu/{aqmp => qmp}/util.py | 0 python/qemu/{qmp => utils}/qemu_ga_client.py | 24 +- python/qemu/{qmp => utils}/qom.py | 5 +- python/qemu/{qmp => utils}/qom_common.py | 9 +- python/qemu/{qmp => utils}/qom_fuse.py| 11 +- python/setup.cfg | 23 +- python/tests/protocol.py | 14 +- scripts/cpu-x86-uarch-abi.py | 7 +- scripts/device-crash-test | 4 +- scripts/qmp/qemu-ga-client| 2 +- scripts/qmp/qom-fuse | 2 +- scripts/qmp/qom-get | 2 +- scripts/qmp/qom-list | 2 +- scripts/qmp/qom-set | 2 +- scripts/qmp/qom-tree | 2 +- scripts/render_block_graph.py | 8 +- scripts/simplebench/bench-example.py
[PULL 4/4] simplebench: Fix Python syntax error (reported by LGTM)
From: Stefan Weil Fixes: b2fcb0c5754c2554b8406376e99a75e9e0a6b7bd Signed-off-by: Stefan Weil Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: John Snow Message-id: 20220107153019.504124-1...@weilnetz.de Signed-off-by: John Snow --- scripts/simplebench/bench-example.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/simplebench/bench-example.py b/scripts/simplebench/bench-example.py index 4864435f39..fc370691e0 100644 --- a/scripts/simplebench/bench-example.py +++ b/scripts/simplebench/bench-example.py @@ -25,7 +25,7 @@ def bench_func(env, case): """ Handle one "cell" of benchmarking table. """ -return bench_block_copy(env['qemu_binary'], env['cmd'], {} +return bench_block_copy(env['qemu_binary'], env['cmd'], {}, case['source'], case['target']) -- 2.31.1
[PULL 0/4] Python patches
The following changes since commit de3f5223fa4cf8bfc5e3fe1fd495ddf468edcdf7: Merge remote-tracking branch 'remotes/vivier/tags/m68k-for-7.0-pull-request' into staging (2022-01-10 14:43:03 +) are available in the Git repository at: https://gitlab.com/jsnow/qemu.git tags/python-pull-request for you to fetch changes up to 9ebfc5a583d8aa94bf1bc37c1f71559187fd809c: simplebench: Fix Python syntax error (reported by LGTM) (2022-01-10 18:23:10 -0500) Python pull request Fixes for the tests that broke during vacation, plus a simple syntax fix for a python script. John Snow (3): python/aqmp: use absolute import statement Python/aqmp: fix type definitions for mypy 0.920 python: update type hints for mypy 0.930 Stefan Weil (1): simplebench: Fix Python syntax error (reported by LGTM) python/qemu/aqmp/aqmp_tui.py | 3 ++- python/qemu/aqmp/protocol.py | 5 +++-- python/qemu/qmp/qom_common.py| 6 +- scripts/simplebench/bench-example.py | 2 +- 4 files changed, 7 insertions(+), 9 deletions(-) -- 2.31.1
[PULL 3/4] python: update type hints for mypy 0.930
Mypy 0.930, released Dec 22, changes the way argparse objects are considered. Crafting a definition that works under Python 3.6 and an older mypy alongside newer versions simultaneously is ... difficult, so... eh. Stub it out with an 'Any' definition to get the CI moving again. Oh well. Signed-off-by: John Snow Reviewed-by: Beraldo Leal Message-id: 20220110191349.1841027-4-js...@redhat.com Signed-off-by: John Snow --- python/qemu/qmp/qom_common.py | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/python/qemu/qmp/qom_common.py b/python/qemu/qmp/qom_common.py index a59ae1a2a1..2e4c741f77 100644 --- a/python/qemu/qmp/qom_common.py +++ b/python/qemu/qmp/qom_common.py @@ -30,10 +30,6 @@ from . import QEMUMonitorProtocol, QMPError -# The following is needed only for a type alias. -Subparsers = argparse._SubParsersAction # pylint: disable=protected-access - - class ObjectPropertyInfo: """ Represents the return type from e.g. qom-list. @@ -89,7 +85,7 @@ def __init__(self, args: argparse.Namespace): self.qmp.connect() @classmethod -def register(cls, subparsers: Subparsers) -> None: +def register(cls, subparsers: Any) -> None: """ Register this command with the argument parser. -- 2.31.1
[PULL 2/4] Python/aqmp: fix type definitions for mypy 0.920
0.920 (Released 2021-12-15) is not entirely happy with the way that I was defining _FutureT: qemu/aqmp/protocol.py:601: error: Item "object" of the upper bound "Optional[Future[Any]]" of type variable "_FutureT" has no attribute "done" Update it with something a little mechanically simpler that works better across a wider array of mypy versions. Signed-off-by: John Snow Message-id: 20220110191349.1841027-3-js...@redhat.com Signed-off-by: John Snow --- python/qemu/aqmp/protocol.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/python/qemu/aqmp/protocol.py b/python/qemu/aqmp/protocol.py index 5190b33b13..c4fbe35a0e 100644 --- a/python/qemu/aqmp/protocol.py +++ b/python/qemu/aqmp/protocol.py @@ -43,8 +43,8 @@ T = TypeVar('T') +_U = TypeVar('_U') _TaskFN = Callable[[], Awaitable[None]] # aka ``async def func() -> None`` -_FutureT = TypeVar('_FutureT', bound=Optional['asyncio.Future[Any]']) class Runstate(Enum): @@ -591,7 +591,8 @@ def _cleanup(self) -> None: """ Fully reset this object to a clean state and return to `IDLE`. """ -def _paranoid_task_erase(task: _FutureT) -> Optional[_FutureT]: +def _paranoid_task_erase(task: Optional['asyncio.Future[_U]'] + ) -> Optional['asyncio.Future[_U]']: # Help to erase a task, ENSURING it is fully quiesced first. assert (task is None) or task.done() return None if (task and task.done()) else task -- 2.31.1
[PULL 1/4] python/aqmp: use absolute import statement
pylint's dependency astroid appears to have bugs in 2.9.1 and 2.9.2 (Dec 31 and Jan 3) that appear to erroneously expect the qemu namespace to have an __init__.py file. astroid 2.9.3 (Jan 9) avoids that problem, but appears to not understand a relative import within a namespace package. Update the relative import - it was worth changing anyway, because these packages will eventually be packaged and distributed separately. Signed-off-by: John Snow Reviewed-by: Beraldo Leal Message-id: 20220110191349.1841027-2-js...@redhat.com Signed-off-by: John Snow --- python/qemu/aqmp/aqmp_tui.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/python/qemu/aqmp/aqmp_tui.py b/python/qemu/aqmp/aqmp_tui.py index a2929f771c..f1e926dd75 100644 --- a/python/qemu/aqmp/aqmp_tui.py +++ b/python/qemu/aqmp/aqmp_tui.py @@ -35,7 +35,8 @@ import urwid import urwid_readline -from ..qmp import QEMUMonitorProtocol, QMPBadPortError +from qemu.qmp import QEMUMonitorProtocol, QMPBadPortError + from .error import ProtocolError from .message import DeserializationError, Message, UnexpectedTypeError from .protocol import ConnectError, Runstate -- 2.31.1
Re: [RFC PATCH v2 2/6] audio/coreaudio: Remove a deprecation warning on macOS 12
On 2022/01/11 6:05, Christian Schoenebeck wrote: On Montag, 10. Januar 2022 21:39:28 CET Akihiko Odaki wrote: On 2022/01/11 5:22, Christian Schoenebeck wrote: On Montag, 10. Januar 2022 20:01:40 CET Akihiko Odaki wrote: On 2022/01/11 3:46, Christian Schoenebeck wrote: On Montag, 10. Januar 2022 19:20:15 CET Akihiko Odaki wrote: On 2022/01/10 22:22, Peter Maydell wrote: On Mon, 10 Jan 2022 at 13:14, Christian Schoenebeck wrote: I'd suggest to use: #if !defined(MAC_OS_VERSION_12_0) || (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_VERSION_12_0) #define kAudioObjectPropertyElementMain kAudioObjectPropertyElementMaster #endif This is also how we do this for existing checks of this sort, like the one in osdep.h for qemu_thread_jit_execute(). -- PMM If I understand correctly, Many macOS-specific codes already no longer complies with GCC because they depend on modern features GCC doesn't provide. The most problematic construction is block; it is extensively used by Apple's ABI and API and you cannot avoid using it even if you try. You mean Obj-C blocks? That's working with GCC for decades. I am not aware about any recent changes to Obj-C block mechanisms by Apple. Also, note that MAC_OS_X_VERSION_MAX_ALLOWED defines the upper bound of the supported version. The lower bound should be preferred here because the usage of the new identifier is applied regardless of the version of the host system. It is in contrary to the usage of MAC_OS_X_VERSION_MAX_ALLOWED in osdep.h where the new interfaces are used only for the newer versions. The lower bound is defined as MAC_OS_X_VERSION_MIN_REQUIRED. Practically there is no difference of the two macros because they have the same value in QEMU and kAudioObjectPropertyElementMain is a constant resolved compile-time, but it is still nice to have the code semantically correct. For this particular enum: no, MAC_OS_X_VERSION_MAX_ALLOWED is the correct one. This is about whether enum kAudioObjectPropertyElementMain is defined in the SDK header files. That's all. And the new enum kAudioObjectPropertyElementMain is pure refactoring of the enum's old name due to social reasons ("Master"). The actual reflected numeric value and semantic of the enum is unchanged and the resulting binary and behaviour are identical. There are a few problems with the usage of MAC_OS_X_VERSION_MAX_ALLOWED: - The deprecation warning is designed to work with MAC_OS_X_VERSION_MIN_REQUIRED. You may verify that with: cc -mmacosx-version-min=12.0 -x c - < int main() { int k = kAudioObjectPropertyElementMaster; } EOF That's actually interesting. On other projects I definitely saw deprecated warnings before on API declarations that were deprecated at a version higher than the project's minimum deployment target. Did they change that? I don't think so. The behavior is documented at: https://clang.llvm.org/docs/AttributeReference.html#availability and the example refers to OS X 10.4, 10.6, 10.7. Probably they haven't changed the behavior for decades. The descriptions is very vague. It sais e.g. "If Clang is instructed to compile code for macOS 10.6 ...". So it is describing it only via singular version per example. We are talking about version ranges however. MacOSX.sdk/System/Library/Frameworks/Kernel.framework/Headers/os/availabilit y.h says manually defining API_TO_BE_DEPRECATED can alter the behavior so that may be the case. - The programmer must be aware whether it is constant or not. - The macro tells about the runtime and not the SDK. There is no way to tell the SDK version and that is why I suggested __is_identifier at the first place. However, now I'm convinced that MAC_OS_X_VERSION_MIN_REQUIRED is the better option because of the above reasons. If you make it dependent on MAC_OS_X_VERSION_MIN_REQUIRED, people with older SDKs (e.g. Xcode <=13.0) would get a compiler error. __is_identifier is the only option if you need a compatibility with the older SDKs while specifying a greater version for MAC_OS_X_VERSION_MIN_REQUIRED. It also applies to MAC_OS_X_VERSION_MAX_ALLOWED; they give the possible runtime versions and not the SDK version. I have never used __is_identifier() for such things. I always used MAC_OS_X_VERSION_MIN_REQUIRED and MAC_OS_X_VERSION_MAX_ALLOWED and it was always doing the job. And for symbols: those are automatically weak linked by the compiler if the project's minimum deployment target is lower than the introductory version of the symbol. That would not happen with older SDKs because they don't know even whether the identifier is a symbol. That is usually not a problem though because such a problem happens only when the version range specified MAC_OS_X_VERSION_MIN_REQUIRED and MAC_OS_X_VERSION_MAX_ALLOWED are not supported by the SDK. You are right about the deprecated warning not being emitted in the example above, currently not sure why, but I still think MAC_OS_X_VERSION_MAX_ALLOWED is the way to go in this case. The
Re: [RFC PATCH v2 2/6] audio/coreaudio: Remove a deprecation warning on macOS 12
On Montag, 10. Januar 2022 21:39:28 CET Akihiko Odaki wrote: > On 2022/01/11 5:22, Christian Schoenebeck wrote: > > On Montag, 10. Januar 2022 20:01:40 CET Akihiko Odaki wrote: > >> On 2022/01/11 3:46, Christian Schoenebeck wrote: > >>> On Montag, 10. Januar 2022 19:20:15 CET Akihiko Odaki wrote: > On 2022/01/10 22:22, Peter Maydell wrote: > > On Mon, 10 Jan 2022 at 13:14, Christian Schoenebeck > > > > wrote: > >> I'd suggest to use: > >> > >> #if !defined(MAC_OS_VERSION_12_0) || > >> > >>(MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_VERSION_12_0) > >> > >> #define kAudioObjectPropertyElementMain > >> kAudioObjectPropertyElementMaster > >> #endif > > > > This is also how we do this for existing checks of this sort, > > like the one in osdep.h for qemu_thread_jit_execute(). > > > > -- PMM > > If I understand correctly, Many macOS-specific codes already no longer > complies with GCC because they depend on modern features GCC doesn't > provide. The most problematic construction is block; it is extensively > used by Apple's ABI and API and you cannot avoid using it even if you > try. > >>> > >>> You mean Obj-C blocks? That's working with GCC for decades. I am not > >>> aware > >>> about any recent changes to Obj-C block mechanisms by Apple. > >>> > Also, note that MAC_OS_X_VERSION_MAX_ALLOWED defines the upper bound of > the supported version. The lower bound should be preferred here because > the usage of the new identifier is applied regardless of the version of > the host system. It is in contrary to the usage of > MAC_OS_X_VERSION_MAX_ALLOWED in osdep.h where the new interfaces are > used only for the newer versions. The lower bound is defined as > MAC_OS_X_VERSION_MIN_REQUIRED. Practically there is no difference of > the > two macros because they have the same value in QEMU and > kAudioObjectPropertyElementMain is a constant resolved compile-time, > but > it is still nice to have the code semantically correct. > >>> > >>> For this particular enum: no, MAC_OS_X_VERSION_MAX_ALLOWED is the > >>> correct > >>> one. This is about whether enum kAudioObjectPropertyElementMain is > >>> defined in the SDK header files. That's all. And the new enum > >>> kAudioObjectPropertyElementMain is pure refactoring of the enum's old > >>> name due to social reasons ("Master"). The actual reflected numeric > >>> value > >>> and semantic of the enum is unchanged and the resulting binary and > >>> behaviour are identical. > >> > >> There are a few problems with the usage of MAC_OS_X_VERSION_MAX_ALLOWED: > >> - The deprecation warning is designed to work with > >> MAC_OS_X_VERSION_MIN_REQUIRED. You may verify that with: > >> cc -mmacosx-version-min=12.0 -x c - < >> #include > >> > >> int main() > >> { > >> > >> int k = kAudioObjectPropertyElementMaster; > >> > >> } > >> EOF > > > > That's actually interesting. On other projects I definitely saw deprecated > > warnings before on API declarations that were deprecated at a version > > higher than the project's minimum deployment target. > > > > Did they change that? > > I don't think so. The behavior is documented at: > https://clang.llvm.org/docs/AttributeReference.html#availability > and the example refers to OS X 10.4, 10.6, 10.7. Probably they haven't > changed the behavior for decades. The descriptions is very vague. It sais e.g. "If Clang is instructed to compile code for macOS 10.6 ...". So it is describing it only via singular version per example. We are talking about version ranges however. > MacOSX.sdk/System/Library/Frameworks/Kernel.framework/Headers/os/availabilit > y.h says manually defining API_TO_BE_DEPRECATED can alter the behavior so > that may be the case. > > >> - The programmer must be aware whether it is constant or not. > >> - The macro tells about the runtime and not the SDK. There is no way to > >> tell the SDK version and that is why I suggested __is_identifier at the > >> first place. However, now I'm convinced that > >> MAC_OS_X_VERSION_MIN_REQUIRED is the better option because of the above > >> reasons. > > > > If you make it dependent on MAC_OS_X_VERSION_MIN_REQUIRED, people with > > older SDKs (e.g. Xcode <=13.0) would get a compiler error. > > __is_identifier is the only option if you need a compatibility with the > older SDKs while specifying a greater version for > MAC_OS_X_VERSION_MIN_REQUIRED. It also applies to > MAC_OS_X_VERSION_MAX_ALLOWED; they give the possible runtime versions > and not the SDK version. I have never used __is_identifier() for such things. I always used MAC_OS_X_VERSION_MIN_REQUIRED and MAC_OS_X_VERSION_MAX_ALLOWED and it was always doing the job. And for symbols: those are automatically weak linked by the compiler if the project's minimum deployment target is lower than the introductory version of the symbol. > > You are
Re: [RFC PATCH v2 2/6] audio/coreaudio: Remove a deprecation warning on macOS 12
On 2022/01/11 5:22, Christian Schoenebeck wrote: On Montag, 10. Januar 2022 20:01:40 CET Akihiko Odaki wrote: On 2022/01/11 3:46, Christian Schoenebeck wrote: On Montag, 10. Januar 2022 19:20:15 CET Akihiko Odaki wrote: On 2022/01/10 22:22, Peter Maydell wrote: On Mon, 10 Jan 2022 at 13:14, Christian Schoenebeck wrote: I'd suggest to use: #if !defined(MAC_OS_VERSION_12_0) || (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_VERSION_12_0) #define kAudioObjectPropertyElementMain kAudioObjectPropertyElementMaster #endif This is also how we do this for existing checks of this sort, like the one in osdep.h for qemu_thread_jit_execute(). -- PMM If I understand correctly, Many macOS-specific codes already no longer complies with GCC because they depend on modern features GCC doesn't provide. The most problematic construction is block; it is extensively used by Apple's ABI and API and you cannot avoid using it even if you try. You mean Obj-C blocks? That's working with GCC for decades. I am not aware about any recent changes to Obj-C block mechanisms by Apple. Also, note that MAC_OS_X_VERSION_MAX_ALLOWED defines the upper bound of the supported version. The lower bound should be preferred here because the usage of the new identifier is applied regardless of the version of the host system. It is in contrary to the usage of MAC_OS_X_VERSION_MAX_ALLOWED in osdep.h where the new interfaces are used only for the newer versions. The lower bound is defined as MAC_OS_X_VERSION_MIN_REQUIRED. Practically there is no difference of the two macros because they have the same value in QEMU and kAudioObjectPropertyElementMain is a constant resolved compile-time, but it is still nice to have the code semantically correct. For this particular enum: no, MAC_OS_X_VERSION_MAX_ALLOWED is the correct one. This is about whether enum kAudioObjectPropertyElementMain is defined in the SDK header files. That's all. And the new enum kAudioObjectPropertyElementMain is pure refactoring of the enum's old name due to social reasons ("Master"). The actual reflected numeric value and semantic of the enum is unchanged and the resulting binary and behaviour are identical. There are a few problems with the usage of MAC_OS_X_VERSION_MAX_ALLOWED: - The deprecation warning is designed to work with MAC_OS_X_VERSION_MIN_REQUIRED. You may verify that with: cc -mmacosx-version-min=12.0 -x c - < int main() { int k = kAudioObjectPropertyElementMaster; } EOF That's actually interesting. On other projects I definitely saw deprecated warnings before on API declarations that were deprecated at a version higher than the project's minimum deployment target. Did they change that? I don't think so. The behavior is documented at: https://clang.llvm.org/docs/AttributeReference.html#availability and the example refers to OS X 10.4, 10.6, 10.7. Probably they haven't changed the behavior for decades. MacOSX.sdk/System/Library/Frameworks/Kernel.framework/Headers/os/availability.h says manually defining API_TO_BE_DEPRECATED can alter the behavior so that may be the case. - The programmer must be aware whether it is constant or not. - The macro tells about the runtime and not the SDK. There is no way to tell the SDK version and that is why I suggested __is_identifier at the first place. However, now I'm convinced that MAC_OS_X_VERSION_MIN_REQUIRED is the better option because of the above reasons. If you make it dependent on MAC_OS_X_VERSION_MIN_REQUIRED, people with older SDKs (e.g. Xcode <=13.0) would get a compiler error. __is_identifier is the only option if you need a compatibility with the older SDKs while specifying a greater version for MAC_OS_X_VERSION_MIN_REQUIRED. It also applies to MAC_OS_X_VERSION_MAX_ALLOWED; they give the possible runtime versions and not the SDK version. You are right about the deprecated warning not being emitted in the example above, currently not sure why, but I still think MAC_OS_X_VERSION_MAX_ALLOWED is the way to go in this case. The page and the header file I referred the above would help understanding the behavior. Regards, Akihiko Odaki Best regards, Christian Schoenebeck
Re: [RFC PATCH v2 2/6] audio/coreaudio: Remove a deprecation warning on macOS 12
On Montag, 10. Januar 2022 20:01:40 CET Akihiko Odaki wrote: > On 2022/01/11 3:46, Christian Schoenebeck wrote: > > On Montag, 10. Januar 2022 19:20:15 CET Akihiko Odaki wrote: > >> On 2022/01/10 22:22, Peter Maydell wrote: > >>> On Mon, 10 Jan 2022 at 13:14, Christian Schoenebeck > >>> > >>> wrote: > I'd suggest to use: > > #if !defined(MAC_OS_VERSION_12_0) || > > (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_VERSION_12_0) > > #define kAudioObjectPropertyElementMain > kAudioObjectPropertyElementMaster > #endif > >>> > >>> This is also how we do this for existing checks of this sort, > >>> like the one in osdep.h for qemu_thread_jit_execute(). > >>> > >>> -- PMM > >> > >> If I understand correctly, Many macOS-specific codes already no longer > >> complies with GCC because they depend on modern features GCC doesn't > >> provide. The most problematic construction is block; it is extensively > >> used by Apple's ABI and API and you cannot avoid using it even if you > >> try. > > > > You mean Obj-C blocks? That's working with GCC for decades. I am not aware > > about any recent changes to Obj-C block mechanisms by Apple. > > > >> Also, note that MAC_OS_X_VERSION_MAX_ALLOWED defines the upper bound of > >> the supported version. The lower bound should be preferred here because > >> the usage of the new identifier is applied regardless of the version of > >> the host system. It is in contrary to the usage of > >> MAC_OS_X_VERSION_MAX_ALLOWED in osdep.h where the new interfaces are > >> used only for the newer versions. The lower bound is defined as > >> MAC_OS_X_VERSION_MIN_REQUIRED. Practically there is no difference of the > >> two macros because they have the same value in QEMU and > >> kAudioObjectPropertyElementMain is a constant resolved compile-time, but > >> it is still nice to have the code semantically correct. > > > > For this particular enum: no, MAC_OS_X_VERSION_MAX_ALLOWED is the correct > > one. This is about whether enum kAudioObjectPropertyElementMain is > > defined in the SDK header files. That's all. And the new enum > > kAudioObjectPropertyElementMain is pure refactoring of the enum's old > > name due to social reasons ("Master"). The actual reflected numeric value > > and semantic of the enum is unchanged and the resulting binary and > > behaviour are identical. > > There are a few problems with the usage of MAC_OS_X_VERSION_MAX_ALLOWED: > - The deprecation warning is designed to work with > MAC_OS_X_VERSION_MIN_REQUIRED. You may verify that with: > cc -mmacosx-version-min=12.0 -x c - < #include > > int main() > { > int k = kAudioObjectPropertyElementMaster; > } > EOF That's actually interesting. On other projects I definitely saw deprecated warnings before on API declarations that were deprecated at a version higher than the project's minimum deployment target. Did they change that? > - The programmer must be aware whether it is constant or not. > - The macro tells about the runtime and not the SDK. There is no way to > tell the SDK version and that is why I suggested __is_identifier at the > first place. However, now I'm convinced that > MAC_OS_X_VERSION_MIN_REQUIRED is the better option because of the above > reasons. If you make it dependent on MAC_OS_X_VERSION_MIN_REQUIRED, people with older SDKs (e.g. Xcode <=13.0) would get a compiler error. You are right about the deprecated warning not being emitted in the example above, currently not sure why, but I still think MAC_OS_X_VERSION_MAX_ALLOWED is the way to go in this case. Best regards, Christian Schoenebeck
Re: [PULL 0/2] SD/MMC patches for 2022-01-08
On 1/10/22 17:02, Peter Maydell wrote: > On Sat, 8 Jan 2022 at 21:59, Philippe Mathieu-Daudé wrote: >> >> Hi Richard, >> >> This is the SD/MMC PR that ought to be sent previously. >> >> The following changes since commit b5a3d8bc9146ba22a25116cb748c97341bf99737: >> >> Merge tag 'pull-misc-20220103' of https://gitlab.com/rth7680/qemu into >> staging (2022-01-03 09:34:41 -0800) >> >> are available in the Git repository at: >> >> https://github.com/philmd/qemu.git tags/sdmmc-20220108 >> >> for you to fetch changes up to b66f73a0cb312c81470433dfd5275d2824bb89de: >> >> hw/sd: Add SDHC support for SD card SPI-mode (2022-01-04 08:50:28 +0100) >> >> >> SD/MMC patches queue >> >> - Add SDHC support for SD card SPI-mode (Frank Chang) >> >> > > Hi; gpg says (my copy of) your key has expired: > > gpg: Signature made Sat 08 Jan 2022 21:56:02 GMT > gpg:using RSA key FAABE75E12917221DCFD6BB2E3E32C2CDEADC0DE > gpg: Good signature from "Philippe Mathieu-Daudé (F4BUG) > " [expired] > gpg: Note: This key has expired! > Primary key fingerprint: FAAB E75E 1291 7221 DCFD 6BB2 E3E3 2C2C DEAD C0DE > > Can you point me at a keyserver I can get an updated version, please? Sorry. It is on pgp.mit.edu and keyserver.ubuntu.com; which keyserver are you using? (so I can upload it there too). Thanks, Phil.
Re: [RFC PATCH v2 2/6] audio/coreaudio: Remove a deprecation warning on macOS 12
On Mon, 10 Jan 2022 at 19:01, Akihiko Odaki wrote: > Assuming the correctness of the use MAC_OS_X_VERSION_MAX_ALLOWED is > irrelevant with the nature of the identifier (constant or not), the same > problem is in ui/cocoa.m: > #ifndef MAC_OS_X_VERSION_10_13 > #define MAC_OS_X_VERSION_10_13 101300 > #endif > > /* 10.14 deprecates NSOnState and NSOffState in favor of > * NSControlStateValueOn/Off, which were introduced in 10.13. > * Define for older versions > */ > #if MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_13 > #define NSControlStateValueOn NSOnState > #define NSControlStateValueOff NSOffState > #endif It's tempting to fix that one by just deleting the ifdeffery, since cocoa.m already doesn't compile on 10.13 (it uses NSPasteboardTypeOwner, which was only introduced in 10.14)... -- PMM
Re: [RFC PATCH v2 2/6] audio/coreaudio: Remove a deprecation warning on macOS 12
On 2022/01/11 3:46, Christian Schoenebeck wrote: On Montag, 10. Januar 2022 19:20:15 CET Akihiko Odaki wrote: On 2022/01/10 22:22, Peter Maydell wrote: On Mon, 10 Jan 2022 at 13:14, Christian Schoenebeck wrote: I'd suggest to use: #if !defined(MAC_OS_VERSION_12_0) || (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_VERSION_12_0) #define kAudioObjectPropertyElementMain kAudioObjectPropertyElementMaster #endif This is also how we do this for existing checks of this sort, like the one in osdep.h for qemu_thread_jit_execute(). -- PMM If I understand correctly, Many macOS-specific codes already no longer complies with GCC because they depend on modern features GCC doesn't provide. The most problematic construction is block; it is extensively used by Apple's ABI and API and you cannot avoid using it even if you try. You mean Obj-C blocks? That's working with GCC for decades. I am not aware about any recent changes to Obj-C block mechanisms by Apple. Also, note that MAC_OS_X_VERSION_MAX_ALLOWED defines the upper bound of the supported version. The lower bound should be preferred here because the usage of the new identifier is applied regardless of the version of the host system. It is in contrary to the usage of MAC_OS_X_VERSION_MAX_ALLOWED in osdep.h where the new interfaces are used only for the newer versions. The lower bound is defined as MAC_OS_X_VERSION_MIN_REQUIRED. Practically there is no difference of the two macros because they have the same value in QEMU and kAudioObjectPropertyElementMain is a constant resolved compile-time, but it is still nice to have the code semantically correct. For this particular enum: no, MAC_OS_X_VERSION_MAX_ALLOWED is the correct one. This is about whether enum kAudioObjectPropertyElementMain is defined in the SDK header files. That's all. And the new enum kAudioObjectPropertyElementMain is pure refactoring of the enum's old name due to social reasons ("Master"). The actual reflected numeric value and semantic of the enum is unchanged and the resulting binary and behaviour are identical. There are a few problems with the usage of MAC_OS_X_VERSION_MAX_ALLOWED: - The deprecation warning is designed to work with MAC_OS_X_VERSION_MIN_REQUIRED. You may verify that with: cc -mmacosx-version-min=12.0 -x c - < int main() { int k = kAudioObjectPropertyElementMaster; } EOF - The programmer must be aware whether it is constant or not. - The macro tells about the runtime and not the SDK. There is no way to tell the SDK version and that is why I suggested __is_identifier at the first place. However, now I'm convinced that MAC_OS_X_VERSION_MIN_REQUIRED is the better option because of the above reasons. There are other cases where MAC_OS_X_VERSION_MIN_REQUIRED (a.k.a. "minimum deployment target") would be used instead: macOS APIs that might be available to only some, but not to the entire macOS version range officially supported by the rolled out binary. Did you see any particular case where this is incorrectly used in QEMU? Best regards, Christian Schoenebeck Assuming the correctness of the use MAC_OS_X_VERSION_MAX_ALLOWED is irrelevant with the nature of the identifier (constant or not), the same problem is in ui/cocoa.m: #ifndef MAC_OS_X_VERSION_10_13 #define MAC_OS_X_VERSION_10_13 101300 #endif /* 10.14 deprecates NSOnState and NSOffState in favor of * NSControlStateValueOn/Off, which were introduced in 10.13. * Define for older versions */ #if MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_13 #define NSControlStateValueOn NSOnState #define NSControlStateValueOff NSOffState #endif Regards, Akihiko Odaki
Re: [PATCH v2] block-backend: prevent dangling BDS pointers across aio_poll()
On 14.12.21 15:35, Stefan Hajnoczi wrote: The BlockBackend root child can change when aio_poll() is invoked. This happens when a temporary filter node is removed upon blockjob completion, for example. Functions in block/block-backend.c must be aware of this when using a blk_bs() pointer across aio_poll() because the BlockDriverState refcnt may reach 0, resulting in a stale pointer. One example is scsi_device_purge_requests(), which calls blk_drain() to wait for in-flight requests to cancel. If the backup blockjob is active, then the BlockBackend root child is a temporary filter BDS owned by the blockjob. The blockjob can complete during bdrv_drained_begin() and the last reference to the BDS is released when the temporary filter node is removed. This results in a use-after-free when blk_drain() calls bdrv_drained_end(bs) on the dangling pointer. By the way, I have a BZ for this, though it’s about block-stream instead of backup (https://bugzilla.redhat.com/show_bug.cgi?id=2036178). But I’m happy to report your patch seems* to fix that problem, too! (Thanks for fixing my BZs! :)) *I’ve written a reproducer in iotest form (https://gitlab.com/hreitz/qemu/-/blob/stefans-fix-and-a-test/tests/qemu-iotests/tests/stream-error-on-reset), and so far I can only assume it indeed reproduces the report, but I found that iotest to indeed be fixed by this patch. (Which made me very happy.) Hanna Explicitly hold a reference to bs across block APIs that invoke aio_poll(). Signed-off-by: Stefan Hajnoczi --- v2: - Audit block/block-backend.c and fix additional cases --- block/block-backend.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/block/block-backend.c b/block/block-backend.c index 12ef80ea17..a40ad7fa92 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -828,10 +828,12 @@ void blk_remove_bs(BlockBackend *blk) notifier_list_notify(>remove_bs_notifiers, blk); if (tgm->throttle_state) { bs = blk_bs(blk); +bdrv_ref(bs); bdrv_drained_begin(bs); throttle_group_detach_aio_context(tgm); throttle_group_attach_aio_context(tgm, qemu_get_aio_context()); bdrv_drained_end(bs); +bdrv_unref(bs); } blk_update_root_state(blk); @@ -1705,6 +1707,7 @@ void blk_drain(BlockBackend *blk) BlockDriverState *bs = blk_bs(blk); if (bs) { +bdrv_ref(bs); bdrv_drained_begin(bs); } @@ -1714,6 +1717,7 @@ void blk_drain(BlockBackend *blk) if (bs) { bdrv_drained_end(bs); +bdrv_unref(bs); } } @@ -2044,10 +2048,13 @@ static int blk_do_set_aio_context(BlockBackend *blk, AioContext *new_context, int ret; if (bs) { +bdrv_ref(bs); + if (update_root_node) { ret = bdrv_child_try_set_aio_context(bs, new_context, blk->root, errp); if (ret < 0) { +bdrv_unref(bs); return ret; } } @@ -2057,6 +2064,8 @@ static int blk_do_set_aio_context(BlockBackend *blk, AioContext *new_context, throttle_group_attach_aio_context(tgm, new_context); bdrv_drained_end(bs); } + +bdrv_unref(bs); } blk->ctx = new_context; @@ -2326,11 +2335,13 @@ void blk_io_limits_disable(BlockBackend *blk) ThrottleGroupMember *tgm = >public.throttle_group_member; assert(tgm->throttle_state); if (bs) { +bdrv_ref(bs); bdrv_drained_begin(bs); } throttle_group_unregister_tgm(tgm); if (bs) { bdrv_drained_end(bs); +bdrv_unref(bs); } }
Re: [RFC PATCH v2 2/6] audio/coreaudio: Remove a deprecation warning on macOS 12
On Montag, 10. Januar 2022 19:20:15 CET Akihiko Odaki wrote: > On 2022/01/10 22:22, Peter Maydell wrote: > > On Mon, 10 Jan 2022 at 13:14, Christian Schoenebeck > > > > wrote: > >> I'd suggest to use: > >> > >> #if !defined(MAC_OS_VERSION_12_0) || > >> > >> (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_VERSION_12_0) > >> > >> #define kAudioObjectPropertyElementMain kAudioObjectPropertyElementMaster > >> #endif > > > > This is also how we do this for existing checks of this sort, > > like the one in osdep.h for qemu_thread_jit_execute(). > > > > -- PMM > > If I understand correctly, Many macOS-specific codes already no longer > complies with GCC because they depend on modern features GCC doesn't > provide. The most problematic construction is block; it is extensively > used by Apple's ABI and API and you cannot avoid using it even if you try. You mean Obj-C blocks? That's working with GCC for decades. I am not aware about any recent changes to Obj-C block mechanisms by Apple. > Also, note that MAC_OS_X_VERSION_MAX_ALLOWED defines the upper bound of > the supported version. The lower bound should be preferred here because > the usage of the new identifier is applied regardless of the version of > the host system. It is in contrary to the usage of > MAC_OS_X_VERSION_MAX_ALLOWED in osdep.h where the new interfaces are > used only for the newer versions. The lower bound is defined as > MAC_OS_X_VERSION_MIN_REQUIRED. Practically there is no difference of the > two macros because they have the same value in QEMU and > kAudioObjectPropertyElementMain is a constant resolved compile-time, but > it is still nice to have the code semantically correct. For this particular enum: no, MAC_OS_X_VERSION_MAX_ALLOWED is the correct one. This is about whether enum kAudioObjectPropertyElementMain is defined in the SDK header files. That's all. And the new enum kAudioObjectPropertyElementMain is pure refactoring of the enum's old name due to social reasons ("Master"). The actual reflected numeric value and semantic of the enum is unchanged and the resulting binary and behaviour are identical. There are other cases where MAC_OS_X_VERSION_MIN_REQUIRED (a.k.a. "minimum deployment target") would be used instead: macOS APIs that might be available to only some, but not to the entire macOS version range officially supported by the rolled out binary. Did you see any particular case where this is incorrectly used in QEMU? Best regards, Christian Schoenebeck
Re: [RFC PATCH v2 2/6] audio/coreaudio: Remove a deprecation warning on macOS 12
On 2022/01/10 22:22, Peter Maydell wrote: On Mon, 10 Jan 2022 at 13:14, Christian Schoenebeck wrote: I'd suggest to use: #if !defined(MAC_OS_VERSION_12_0) || (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_VERSION_12_0) #define kAudioObjectPropertyElementMain kAudioObjectPropertyElementMaster #endif This is also how we do this for existing checks of this sort, like the one in osdep.h for qemu_thread_jit_execute(). -- PMM If I understand correctly, Many macOS-specific codes already no longer complies with GCC because they depend on modern features GCC doesn't provide. The most problematic construction is block; it is extensively used by Apple's ABI and API and you cannot avoid using it even if you try. Also, note that MAC_OS_X_VERSION_MAX_ALLOWED defines the upper bound of the supported version. The lower bound should be preferred here because the usage of the new identifier is applied regardless of the version of the host system. It is in contrary to the usage of MAC_OS_X_VERSION_MAX_ALLOWED in osdep.h where the new interfaces are used only for the newer versions. The lower bound is defined as MAC_OS_X_VERSION_MIN_REQUIRED. Practically there is no difference of the two macros because they have the same value in QEMU and kAudioObjectPropertyElementMain is a constant resolved compile-time, but it is still nice to have the code semantically correct. Regards, Akihiko Odaki
Re: [PATCH v2 4/4] meson: generate trace points for qmp commands
On Thu, Dec 23, 2021 at 12:07:56PM +0100, Vladimir Sementsov-Ogievskiy wrote: > 1. Use --add-trace-points when generate qmp commands > 2. Add corresponding .trace-events files as outputs in qapi_files >custom target > 3. Define global qapi_trace_events list of .trace-events file targets, >to fill in trace/qapi.build and to use in trace/meson.build > 4. In trace/meson.build use the new array as an additional source of >.trace_events files to be processed > > Signed-off-by: Vladimir Sementsov-Ogievskiy > --- > meson.build | 1 + > qapi/meson.build | 9 - > trace/meson.build | 11 --- > 3 files changed, 17 insertions(+), 4 deletions(-) This patch has a meson subdir ordering dependency: qapi/ must be processed before trace/. Please document this in meson.build so anyone editing subdirs knows to preserve this order. signature.asc Description: PGP signature
Re: [PATCH v2 3/4] scripts/qapi-gen.py: add --add-trace-points option
On Thu, Dec 23, 2021 at 12:07:55PM +0100, Vladimir Sementsov-Ogievskiy wrote: > @@ -74,6 +75,8 @@ def main() -> int: > parser.add_argument('-u', '--unmask-non-abi-names', action='store_true', > dest='unmask', > help="expose non-ABI names in introspection") > +parser.add_argument('--add-trace-points', action='store_true', > +help="add trace points to qmp marshals") Please call it --add-trace-events for consistency with QEMU tracing terminology. Thanks, Stefan signature.asc Description: PGP signature
Re: [PATCH v2 2/4] scripts/qapi/commands: gen_commands(): add add_trace_points argument
On Thu, Dec 23, 2021 at 12:07:54PM +0100, Vladimir Sementsov-Ogievskiy wrote: > Add possibility to generate trace points for each qmp command. > > We should generate both trace points and trace-events file, for further > trace point code generation. > > Signed-off-by: Vladimir Sementsov-Ogievskiy > --- > scripts/qapi/commands.py | 84 ++-- > 1 file changed, 73 insertions(+), 11 deletions(-) > > diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py > index 21001bbd6b..9691c11f96 100644 > --- a/scripts/qapi/commands.py > +++ b/scripts/qapi/commands.py > @@ -53,7 +53,8 @@ def gen_command_decl(name: str, > def gen_call(name: str, > arg_type: Optional[QAPISchemaObjectType], > boxed: bool, > - ret_type: Optional[QAPISchemaType]) -> str: > + ret_type: Optional[QAPISchemaType], > + add_trace_points: bool) -> str: Please use the term "trace events" instead of "trace points". That's the term that docs/devel/tracing.rst uses. > @@ -122,10 +167,14 @@ def gen_marshal_decl(name: str) -> str: > proto=build_marshal_proto(name)) > > > +def gen_trace(name: str) -> str: > +return f'qmp_{c_name(name)}(const char *tag, const char *json) "%s%s"\n' This trace event is emitted in 3 different ways: 1. For arguments before calling a QMP command. 2. For the error message when the QMP command fails. 3. For the return value when a QMP command succeeds. This makes parsing the trace akward because you get two events in succession for a single call and they both have the same name. Please generate 2 trace events: 1. qmp_enter_ 2. qmp_exit_ (That's similar to how the syscalls Linux kernel trace events work.) Scripts processing the trace can easily differentiate between enter (args) and exit (return value) events without parsing or keeping state to count the second event. Thanks, Stefan signature.asc Description: PGP signature
Re: [PATCH v2 1/4] jobs: drop qmp_ trace points
On Thu, Dec 23, 2021 at 12:07:53PM +0100, Vladimir Sementsov-Ogievskiy wrote: > diff --git a/block/trace-events b/block/trace-events > index 549090d453..5be3e3913b 100644 > --- a/block/trace-events > +++ b/block/trace-events > @@ -49,15 +49,6 @@ block_copy_read_fail(void *bcs, int64_t start, int ret) > "bcs %p start %"PRId64" > block_copy_write_fail(void *bcs, int64_t start, int ret) "bcs %p start > %"PRId64" ret %d" > block_copy_write_zeroes_fail(void *bcs, int64_t start, int ret) "bcs %p > start %"PRId64" ret %d" > > -# ../blockdev.c > -qmp_block_job_cancel(void *job) "job %p" > -qmp_block_job_pause(void *job) "job %p" > -qmp_block_job_resume(void *job) "job %p" > -qmp_block_job_complete(void *job) "job %p" > -qmp_block_job_finalize(void *job) "job %p" > -qmp_block_job_dismiss(void *job) "job %p" > -qmp_block_stream(void *bs) "bs %p" > - > # file-win32.c > file_paio_submit(void *acb, void *opaque, int64_t offset, int count, int > type) "acb %p opaque %p offset %"PRId64" count %d type %d" > > diff --git a/trace-events b/trace-events > index a637a61eba..1265f1e0cc 100644 > --- a/trace-events > +++ b/trace-events > @@ -79,14 +79,6 @@ job_state_transition(void *job, int ret, const char > *legal, const char *s0, con > job_apply_verb(void *job, const char *state, const char *verb, const char > *legal) "job %p in state %s; applying verb %s (%s)" > job_completed(void *job, int ret) "job %p ret %d" > > -# job-qmp.c > -qmp_job_cancel(void *job) "job %p" > -qmp_job_pause(void *job) "job %p" > -qmp_job_resume(void *job) "job %p" > -qmp_job_complete(void *job) "job %p" > -qmp_job_finalize(void *job) "job %p" > -qmp_job_dismiss(void *job) "job %p" The job pointer argument will be lost. That's not ideal but probably worth getting trace events for all QMP commands. Stefan signature.asc Description: PGP signature
Re: [PULL 0/2] SD/MMC patches for 2022-01-08
On Sat, 8 Jan 2022 at 21:59, Philippe Mathieu-Daudé wrote: > > Hi Richard, > > This is the SD/MMC PR that ought to be sent previously. > > The following changes since commit b5a3d8bc9146ba22a25116cb748c97341bf99737: > > Merge tag 'pull-misc-20220103' of https://gitlab.com/rth7680/qemu into > staging (2022-01-03 09:34:41 -0800) > > are available in the Git repository at: > > https://github.com/philmd/qemu.git tags/sdmmc-20220108 > > for you to fetch changes up to b66f73a0cb312c81470433dfd5275d2824bb89de: > > hw/sd: Add SDHC support for SD card SPI-mode (2022-01-04 08:50:28 +0100) > > > SD/MMC patches queue > > - Add SDHC support for SD card SPI-mode (Frank Chang) > > Hi; gpg says (my copy of) your key has expired: gpg: Signature made Sat 08 Jan 2022 21:56:02 GMT gpg:using RSA key FAABE75E12917221DCFD6BB2E3E32C2CDEADC0DE gpg: Good signature from "Philippe Mathieu-Daudé (F4BUG) " [expired] gpg: Note: This key has expired! Primary key fingerprint: FAAB E75E 1291 7221 DCFD 6BB2 E3E3 2C2C DEAD C0DE Can you point me at a keyserver I can get an updated version, please? thanks -- PMM
iotest 040, 041, intermittent failure in netbsd VM
Just saw this failure of iotests in a netbsd VM (the in-tree tests/vm stuff). Pretty sure it's an intermittent as the pulreq being tested has nothing io or block related. TEST iotest-qcow2: 036 TEST iotest-qcow2: 037 TEST iotest-qcow2: 038 [not run] TEST iotest-qcow2: 039 [not run] TEST iotest-qcow2: 040 [fail] QEMU -- "/home/qemu/qemu-test.MPWquy/build/tests/qemu-iotests/../../qemu-system-aarch64" -nodefaults -display none -accel qtest -machine virt QEMU_IMG -- "/home/qemu/qemu-test.MPWquy/build/tests/qemu-iotests/../../qemu-img" QEMU_IO -- "/home/qemu/qemu-test.MPWquy/build/tests/qemu-iotests/../../qemu-io" --cache writeback --aio threads -f qcow2 QEMU_NBD -- "/home/qemu/qemu-test.MPWquy/build/tests/qemu-iotests/../../qemu-nbd" IMGFMT-- qcow2 IMGPROTO -- file PLATFORM -- NetBSD/amd64 localhost 9.2 TEST_DIR -- /home/qemu/qemu-test.MPWquy/build/tests/qemu-iotests/scratch SOCK_DIR -- /tmp/tmpuniuicbi GDB_OPTIONS -- VALGRIND_QEMU -- PRINT_QEMU_OUTPUT -- --- /home/qemu/qemu-test.MPWquy/src/tests/qemu-iotests/040.out fcntl(): Invalid argument +++ 040.out.bad @@ -1,5 +1,30 @@ -. +ERROR:qemu.aqmp.qmp_client.qemu-7648:Failed to establish connection: concurrent.futures._base.CancelledError +E +== +ERROR: test_top_is_default_active (__main__.TestSingleDrive) +-- +Traceback (most recent call last): + File "/home/qemu/qemu-test.MPWquy/src/tests/qemu-iotests/040", line 94, in setUp +self.vm.launch() + File "/home/qemu/qemu-test.MPWquy/src/python/qemu/machine/machine.py", line 399, in launch +self._launch() + File "/home/qemu/qemu-test.MPWquy/src/python/qemu/machine/machine.py", line 434, in _launch +self._post_launch() + File "/home/qemu/qemu-test.MPWquy/src/python/qemu/machine/qtest.py", line 147, in _post_launch +super()._post_launch() + File "/home/qemu/qemu-test.MPWquy/src/python/qemu/machine/machine.py", line 340, in _post_launch +self._qmp.accept(self._qmp_timer) + File "/home/qemu/qemu-test.MPWquy/src/python/qemu/aqmp/legacy.py", line 69, in accept +timeout + File "/home/qemu/qemu-test.MPWquy/src/python/qemu/aqmp/legacy.py", line 42, in _sync +asyncio.wait_for(future, timeout=timeout) + File "/usr/pkg/lib/python3.7/asyncio/base_events.py", line 587, in run_until_complete +return future.result() + File "/usr/pkg/lib/python3.7/asyncio/tasks.py", line 449, in wait_for +raise futures.TimeoutError() +concurrent.futures._base.TimeoutError + -- Ran 65 tests -OK +FAILED (errors=1) TEST iotest-qcow2: 041 [fail] QEMU -- "/home/qemu/qemu-test.MPWquy/build/tests/qemu-iotests/../../qemu-system-aarch64" -nodefaults -display none -accel qtest -machine virt QEMU_IMG -- "/home/qemu/qemu-test.MPWquy/build/tests/qemu-iotests/../../qemu-img" QEMU_IO -- "/home/qemu/qemu-test.MPWquy/build/tests/qemu-iotests/../../qemu-io" --cache writeback --aio threads -f qcow2 QEMU_NBD -- "/home/qemu/qemu-test.MPWquy/build/tests/qemu-iotests/../../qemu-nbd" IMGFMT-- qcow2 IMGPROTO -- file PLATFORM -- NetBSD/amd64 localhost 9.2 TEST_DIR -- /home/qemu/qemu-test.MPWquy/build/tests/qemu-iotests/scratch SOCK_DIR -- /tmp/tmpuniuicbi GDB_OPTIONS -- VALGRIND_QEMU -- PRINT_QEMU_OUTPUT -- --- /home/qemu/qemu-test.MPWquy/src/tests/qemu-iotests/041.out +++ 041.out.bad @@ -1,5 +1,32 @@ -... +..ERROR:qemu.aqmp.qmp_client.qemu-15252:Failed to establish connection: concurrent.futures._base.CancelledError +E +== +ERROR: test_small_buffer (__main__.TestSingleBlockdev) +-- +Traceback (most recent call last): + File "/home/qemu/qemu-test.MPWquy/src/tests/qemu-iotests/041", line 233, in setUp +TestSingleDrive.setUp(self) + File "/home/qemu/qemu-test.MPWquy/src/tests/qemu-iotests/041", line 54, in setUp +self.vm.launch() + File "/home/qemu/qemu-test.MPWquy/src/python/qemu/machine/machine.py", line 399, in launch +self._launch() + File "/home/qemu/qemu-test.MPWquy/src/python/qemu/machine/machine.py", line 434, in _launch +self._post_launch() + File "/home/qemu/qemu-test.MPWquy/src/python/qemu/machine/qtest.py", line 147, in _post_launch +super()._post_launch() + File "/home/qemu/qemu-test.MPWquy/src/python/qemu/machine/machine.py", line 340, in _post_launch +self._qmp.accept(self._qmp_timer) +
Re: [PATCH 2/2] block/rbd: workaround for ceph issue #53784
On Mon, Jan 10, 2022 at 12:41:54PM +0100, Peter Lieven wrote: librbd had a bug until early 2022 that affected all versions of ceph that supported fast-diff. This bug results in reporting of incorrect offsets if the offset parameter to rbd_diff_iterate2 is not object aligned. Work around this bug by rounding down the offset to object boundaries. Fixes: https://tracker.ceph.com/issues/53784 Cc: qemu-sta...@nongnu.org Signed-off-by: Peter Lieven --- block/rbd.c | 17 - 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/block/rbd.c b/block/rbd.c index 5e9dc91d81..260cb9f4b4 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -1333,6 +1333,7 @@ static int coroutine_fn qemu_rbd_co_block_status(BlockDriverState *bs, int status, r; RBDDiffIterateReq req = { .offs = offset }; uint64_t features, flags; +int64_t head; assert(offset + bytes <= s->image_size); @@ -1360,6 +1361,19 @@ static int coroutine_fn qemu_rbd_co_block_status(BlockDriverState *bs, return status; } +/* + * librbd had a bug until early 2022 that affected all versions of ceph that + * supported fast-diff. This bug results in reporting of incorrect offsets + * if the offset parameter to rbd_diff_iterate2 is not object aligned. + * Work around this bug by rounding down the offset to object boundaries. + * + * See: https://tracker.ceph.com/issues/53784 + */ +head = offset & (s->object_size - 1); +offset -= head; +req.offs -= head; +bytes += head; + r = rbd_diff_iterate2(s->image, NULL, offset, bytes, true, true, qemu_rbd_diff_iterate_cb, ); if (r < 0 && r != QEMU_RBD_EXIT_DIFF_ITERATE2) { @@ -1379,7 +1393,8 @@ static int coroutine_fn qemu_rbd_co_block_status(BlockDriverState *bs, status = BDRV_BLOCK_ZERO | BDRV_BLOCK_OFFSET_VALID; } -*pnum = req.bytes; +assert(req.bytes > head); +*pnum = req.bytes - head; return status; } Thanks for the workaround! I just tested this patch for the issue reported in this BZ [1] and the test now works correctly! Tested-by: Stefano Garzarella [1] https://bugzilla.redhat.com/show_bug.cgi?id=2034791
Re: [RFC PATCH v2 2/6] audio/coreaudio: Remove a deprecation warning on macOS 12
On Mon, 10 Jan 2022 at 13:14, Christian Schoenebeck wrote: > I'd suggest to use: > > #if !defined(MAC_OS_VERSION_12_0) || > (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_VERSION_12_0) > #define kAudioObjectPropertyElementMain kAudioObjectPropertyElementMaster > #endif This is also how we do this for existing checks of this sort, like the one in osdep.h for qemu_thread_jit_execute(). -- PMM
Re: [RFC PATCH v2 2/6] audio/coreaudio: Remove a deprecation warning on macOS 12
On 1/10/22 14:07, Christian Schoenebeck wrote: > On Montag, 10. Januar 2022 13:24:06 CET Philippe Mathieu-Daudé wrote: >> On 1/10/22 09:44, Philippe Mathieu-Daudé wrote: >>> On 1/10/22 09:17, Akihiko Odaki wrote: On 2022/01/10 2:06, Philippe Mathieu-Daudé wrote: > When building on macOS 12 we get: > >audio/coreaudio.c:50:5: error: 'kAudioObjectPropertyElementMaster' > is deprecated: first deprecated in macOS 12.0 > [-Werror,-Wdeprecated-declarations] >kAudioObjectPropertyElementMaster >^ >kAudioObjectPropertyElementMain > > /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frame > works/CoreAudio.framework/Headers/AudioHardwareBase.h:208:5: note: > 'kAudioObjectPropertyElementMaster' has been explicitly marked > deprecated here >kAudioObjectPropertyElementMaster > API_DEPRECATED_WITH_REPLACEMENT("kAudioObjectPropertyElementMain", > macos(10.0, 12.0), ios(2.0, 15.0), watchos(1.0, 8.0), tvos(9.0, 15.0)) > = kAudioObjectPropertyElementMain >^ > > Use kAudioObjectPropertyElementMain (define it to > kAudioObjectPropertyElementMaster on macOS < 12). > > Signed-off-by: Philippe Mathieu-Daudé > --- > audio/coreaudio.c | 16 ++-- > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/audio/coreaudio.c b/audio/coreaudio.c > index d8a21d3e507..c836bc9dd37 100644 > --- a/audio/coreaudio.c > +++ b/audio/coreaudio.c > @@ -44,10 +44,14 @@ typedef struct coreaudioVoiceOut { > bool enabled; > } coreaudioVoiceOut; > +#if !defined(MAC_OS_VERSION_12_0) > +#define kAudioObjectPropertyElementMain > kAudioObjectPropertyElementMaster > +#endif > + Semantically MAC_OS_VERSION_12_0 defines the numeric value of version 12.0 and its existence does not mean that kAudioObjectPropertyElementMain is defined. I suggest the following: #if !__is_identifier(kAudioObjectPropertyElementMain) #define kAudioObjectPropertyElementMain kAudioObjectPropertyElementMaster #endif >> >> Apparently __is_identifier() is Clang specific. It might be acceptable >> since this file is restricted to macOS. Similarly for the other >> block/file-posix.c patch, the section is conditional to __APPLE__ >> being defined. > > Correct, __is_identifier() is a clang extension and does not work with GCC > (tested). I would not use it. People on Mac usually use clang, but there are > also cross compilers for macOS binaries. > > I'd suggest to use: > > #if !defined(MAC_OS_VERSION_12_0) || > (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_VERSION_12_0) > #define kAudioObjectPropertyElementMain kAudioObjectPropertyElementMaster > #endif Uh I just posted v3. I didn't test GCC. I'll wait for more comment on v3 then repost v4 with your suggestion, thanks.
Re: [RFC PATCH v2 2/6] audio/coreaudio: Remove a deprecation warning on macOS 12
On Montag, 10. Januar 2022 13:24:06 CET Philippe Mathieu-Daudé wrote: > On 1/10/22 09:44, Philippe Mathieu-Daudé wrote: > > On 1/10/22 09:17, Akihiko Odaki wrote: > >> On 2022/01/10 2:06, Philippe Mathieu-Daudé wrote: > >>> When building on macOS 12 we get: > >>> > >>>audio/coreaudio.c:50:5: error: 'kAudioObjectPropertyElementMaster' > >>> is deprecated: first deprecated in macOS 12.0 > >>> [-Werror,-Wdeprecated-declarations] > >>>kAudioObjectPropertyElementMaster > >>>^ > >>>kAudioObjectPropertyElementMain > >>> > >>> /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frame > >>> works/CoreAudio.framework/Headers/AudioHardwareBase.h:208:5: note: > >>> 'kAudioObjectPropertyElementMaster' has been explicitly marked > >>> deprecated here > >>>kAudioObjectPropertyElementMaster > >>> API_DEPRECATED_WITH_REPLACEMENT("kAudioObjectPropertyElementMain", > >>> macos(10.0, 12.0), ios(2.0, 15.0), watchos(1.0, 8.0), tvos(9.0, 15.0)) > >>> = kAudioObjectPropertyElementMain > >>>^ > >>> > >>> Use kAudioObjectPropertyElementMain (define it to > >>> kAudioObjectPropertyElementMaster on macOS < 12). > >>> > >>> Signed-off-by: Philippe Mathieu-Daudé > >>> --- > >>> audio/coreaudio.c | 16 ++-- > >>> 1 file changed, 10 insertions(+), 6 deletions(-) > >>> > >>> diff --git a/audio/coreaudio.c b/audio/coreaudio.c > >>> index d8a21d3e507..c836bc9dd37 100644 > >>> --- a/audio/coreaudio.c > >>> +++ b/audio/coreaudio.c > >>> @@ -44,10 +44,14 @@ typedef struct coreaudioVoiceOut { > >>> bool enabled; > >>> } coreaudioVoiceOut; > >>> +#if !defined(MAC_OS_VERSION_12_0) > >>> +#define kAudioObjectPropertyElementMain > >>> kAudioObjectPropertyElementMaster > >>> +#endif > >>> + > >> > >> Semantically MAC_OS_VERSION_12_0 defines the numeric value of version > >> 12.0 and its existence does not mean that > >> kAudioObjectPropertyElementMain is defined. I suggest the following: > >> #if !__is_identifier(kAudioObjectPropertyElementMain) > >> #define kAudioObjectPropertyElementMain kAudioObjectPropertyElementMaster > >> #endif > > Apparently __is_identifier() is Clang specific. It might be acceptable > since this file is restricted to macOS. Similarly for the other > block/file-posix.c patch, the section is conditional to __APPLE__ > being defined. Correct, __is_identifier() is a clang extension and does not work with GCC (tested). I would not use it. People on Mac usually use clang, but there are also cross compilers for macOS binaries. I'd suggest to use: #if !defined(MAC_OS_VERSION_12_0) || (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_VERSION_12_0) #define kAudioObjectPropertyElementMain kAudioObjectPropertyElementMaster #endif Best regards, Christian Schoenebeck
Re: [RFC PATCH v2 2/6] audio/coreaudio: Remove a deprecation warning on macOS 12
On 1/10/22 09:44, Philippe Mathieu-Daudé wrote: > On 1/10/22 09:17, Akihiko Odaki wrote: >> On 2022/01/10 2:06, Philippe Mathieu-Daudé wrote: >>> When building on macOS 12 we get: >>> >>> audio/coreaudio.c:50:5: error: 'kAudioObjectPropertyElementMaster' >>> is deprecated: first deprecated in macOS 12.0 >>> [-Werror,-Wdeprecated-declarations] >>> kAudioObjectPropertyElementMaster >>> ^ >>> kAudioObjectPropertyElementMain >>> >>> /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/CoreAudio.framework/Headers/AudioHardwareBase.h:208:5: >>> note: 'kAudioObjectPropertyElementMaster' has been explicitly marked >>> deprecated here >>> kAudioObjectPropertyElementMaster >>> API_DEPRECATED_WITH_REPLACEMENT("kAudioObjectPropertyElementMain", >>> macos(10.0, 12.0), ios(2.0, 15.0), watchos(1.0, 8.0), tvos(9.0, 15.0)) >>> = kAudioObjectPropertyElementMain >>> ^ >>> >>> Use kAudioObjectPropertyElementMain (define it to >>> kAudioObjectPropertyElementMaster on macOS < 12). >>> >>> Signed-off-by: Philippe Mathieu-Daudé >>> --- >>> audio/coreaudio.c | 16 ++-- >>> 1 file changed, 10 insertions(+), 6 deletions(-) >>> >>> diff --git a/audio/coreaudio.c b/audio/coreaudio.c >>> index d8a21d3e507..c836bc9dd37 100644 >>> --- a/audio/coreaudio.c >>> +++ b/audio/coreaudio.c >>> @@ -44,10 +44,14 @@ typedef struct coreaudioVoiceOut { >>> bool enabled; >>> } coreaudioVoiceOut; >>> +#if !defined(MAC_OS_VERSION_12_0) >>> +#define kAudioObjectPropertyElementMain >>> kAudioObjectPropertyElementMaster >>> +#endif >>> + >> >> Semantically MAC_OS_VERSION_12_0 defines the numeric value of version >> 12.0 and its existence does not mean that >> kAudioObjectPropertyElementMain is defined. I suggest the following: >> #if !__is_identifier(kAudioObjectPropertyElementMain) >> #define kAudioObjectPropertyElementMain kAudioObjectPropertyElementMaster >> #endif Apparently __is_identifier() is Clang specific. It might be acceptable since this file is restricted to macOS. Similarly for the other block/file-posix.c patch, the section is conditional to __APPLE__ being defined.
Re: [PATCH 0/2] block-backend: Retain permissions after migration
On 11/25/2021 9:53 PM, Hanna Reitz wrote: > Hi, > > Peng Liang has reported an issue regarding migration of raw images here: > https://lists.nongnu.org/archive/html/qemu-block/2021-11/msg00673.html > > It turns out that after migrating, all permissions are shared when they > weren’t before. The cause of the problem is that we deliberately delay > restricting the shared permissions until migration is really done (until > the runstate is no longer INMIGRATE) and first share all permissions; > but this causes us to lose the original shared permission mask and > overwrites it with BLK_PERM_ALL, so once we do try to restrict the > shared permissions, we only again share them all. > > Fix this by saving the set of shared permissions through the first > blk_perm_set() call that shares all; and add a regression test. > > > I don’t believe we have to fix this in 6.2, because I think this bug has > existed for four years now. (I.e. it isn’t critical, and it’s no > regression.) > > > Hanna Reitz (2): > block-backend: Retain permissions after migration > iotests/migration-permissions: New test > > block/block-backend.c | 11 ++ > .../qemu-iotests/tests/migration-permissions | 101 ++ > .../tests/migration-permissions.out | 5 + > 3 files changed, 117 insertions(+) > create mode 100755 tests/qemu-iotests/tests/migration-permissions > create mode 100644 tests/qemu-iotests/tests/migration-permissions.out > Hi Hanna, QEMU 6.3 development tree has been opened. Will this fix be merged in 6.3? Thanks, Peng
[PATCH 0/2] block/rbd: fixes for bdrv_co_block_status
Peter Lieven (2): block/rbd: fix handling of holes in .bdrv_co_block_status block/rbd: workaround for ceph issue #53784 block/rbd.c | 72 + 1 file changed, 50 insertions(+), 22 deletions(-) -- 2.25.1
[PATCH 2/2] block/rbd: workaround for ceph issue #53784
librbd had a bug until early 2022 that affected all versions of ceph that supported fast-diff. This bug results in reporting of incorrect offsets if the offset parameter to rbd_diff_iterate2 is not object aligned. Work around this bug by rounding down the offset to object boundaries. Fixes: https://tracker.ceph.com/issues/53784 Cc: qemu-sta...@nongnu.org Signed-off-by: Peter Lieven --- block/rbd.c | 17 - 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/block/rbd.c b/block/rbd.c index 5e9dc91d81..260cb9f4b4 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -1333,6 +1333,7 @@ static int coroutine_fn qemu_rbd_co_block_status(BlockDriverState *bs, int status, r; RBDDiffIterateReq req = { .offs = offset }; uint64_t features, flags; +int64_t head; assert(offset + bytes <= s->image_size); @@ -1360,6 +1361,19 @@ static int coroutine_fn qemu_rbd_co_block_status(BlockDriverState *bs, return status; } +/* + * librbd had a bug until early 2022 that affected all versions of ceph that + * supported fast-diff. This bug results in reporting of incorrect offsets + * if the offset parameter to rbd_diff_iterate2 is not object aligned. + * Work around this bug by rounding down the offset to object boundaries. + * + * See: https://tracker.ceph.com/issues/53784 + */ +head = offset & (s->object_size - 1); +offset -= head; +req.offs -= head; +bytes += head; + r = rbd_diff_iterate2(s->image, NULL, offset, bytes, true, true, qemu_rbd_diff_iterate_cb, ); if (r < 0 && r != QEMU_RBD_EXIT_DIFF_ITERATE2) { @@ -1379,7 +1393,8 @@ static int coroutine_fn qemu_rbd_co_block_status(BlockDriverState *bs, status = BDRV_BLOCK_ZERO | BDRV_BLOCK_OFFSET_VALID; } -*pnum = req.bytes; +assert(req.bytes > head); +*pnum = req.bytes - head; return status; } -- 2.25.1
[PATCH 1/2] block/rbd: fix handling of holes in .bdrv_co_block_status
the assumption that we can't hit a hole if we do not diff against a snapshot was wrong. We can see a hole in an image if we diff against base if there exists an older snapshot of the image and we have discarded blocks in the image where the snapshot has data. Fixes: 0347a8fd4c3faaedf119be04c197804be40a384b Cc: qemu-sta...@nongnu.org Signed-off-by: Peter Lieven --- block/rbd.c | 55 + 1 file changed, 34 insertions(+), 21 deletions(-) diff --git a/block/rbd.c b/block/rbd.c index def96292e0..5e9dc91d81 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -1279,13 +1279,24 @@ static int qemu_rbd_diff_iterate_cb(uint64_t offs, size_t len, RBDDiffIterateReq *req = opaque; assert(req->offs + req->bytes <= offs); -/* - * we do not diff against a snapshot so we should never receive a callback - * for a hole. - */ -assert(exists); -if (!req->exists && offs > req->offs) { +if (req->exists && offs > req->offs + req->bytes) { +/* + * we started in an allocated area and jumped over an unallocated area, + * req->bytes contains the length of the allocated area before the + * unallocated area. stop further processing. + */ +return QEMU_RBD_EXIT_DIFF_ITERATE2; +} +if (req->exists && !exists) { +/* + * we started in an allocated area and reached a hole. req->bytes + * contains the length of the allocated area before the hole. + * stop further processing. + */ +return QEMU_RBD_EXIT_DIFF_ITERATE2; +} +if (!req->exists && exists && offs > req->offs) { /* * we started in an unallocated area and hit the first allocated * block. req->bytes must be set to the length of the unallocated area @@ -1295,17 +1306,19 @@ static int qemu_rbd_diff_iterate_cb(uint64_t offs, size_t len, return QEMU_RBD_EXIT_DIFF_ITERATE2; } -if (req->exists && offs > req->offs + req->bytes) { -/* - * we started in an allocated area and jumped over an unallocated area, - * req->bytes contains the length of the allocated area before the - * unallocated area. stop further processing. - */ -return QEMU_RBD_EXIT_DIFF_ITERATE2; -} +/* + * assert that we caught all cases above and allocation state has not + * changed during callbacks. + */ +assert(exists == req->exists || !req->bytes); +req->exists = exists; -req->bytes += len; -req->exists = true; +/* + * assert that we either return an unallocated block or have got callbacks + * for all allocated blocks present. + */ +assert(!req->exists || offs == req->offs + req->bytes); +req->bytes = offs + len - req->offs; return 0; } @@ -1354,13 +1367,13 @@ static int coroutine_fn qemu_rbd_co_block_status(BlockDriverState *bs, } assert(req.bytes <= bytes); if (!req.exists) { -if (r == 0) { +if (r == 0 && !req.bytes) { /* - * rbd_diff_iterate2 does not invoke callbacks for unallocated - * areas. This here catches the case where no callback was - * invoked at all (req.bytes == 0). + * rbd_diff_iterate2 does not invoke callbacks for unallocated areas + * except for the case where an overlay has a hole where the parent + * or an older snapshot of the image has not. This here catches the + * case where no callback was invoked at all. */ -assert(req.bytes == 0); req.bytes = bytes; } status = BDRV_BLOCK_ZERO | BDRV_BLOCK_OFFSET_VALID; -- 2.25.1
Re: [PATCH v2 6/6] gitlab-ci: Support macOS 12 via cirrus-run
On Sun, Jan 09, 2022 at 06:06:12PM +0100, Philippe Mathieu-Daudé wrote: > Add support for macOS 12 build on Cirrus-CI, similarly to commit > 0e103a65ba1 ("gitlab: support for ... macOS 11 via cirrus-run"). > > Disable deprecation warnings on Objective C to avoid: > > [2789/6622] Compiling Objective-C object libcommon.fa.p/ui_cocoa.m.o > ui/cocoa.m:1411:16: error: 'setAllowedFileTypes:' is deprecated: first > deprecated in macOS 12.0 - Use -allowedContentTypes instead > [-Werror,-Wdeprecated-declarations] > [openPanel setAllowedFileTypes: supportedImageFileTypes]; > ^ > > /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/AppKit.framework/Headers/NSSavePanel.h:215:49: > note: property 'allowedFileTypes' is declared deprecated here > @property (nullable, copy) NSArray *allowedFileTypes > API_DEPRECATED("Use -allowedContentTypes instead", macos(10.3,12.0)); > ^ > > /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/AppKit.framework/Headers/NSSavePanel.h:215:49: > note: 'setAllowedFileTypes:' has been explicitly marked deprecated here > FAILED: libcommon.fa.p/ui_cocoa.m.o > > Signed-off-by: Philippe Mathieu-Daudé > --- > Generated using lcitool from: > https://gitlab.com/libvirt/libvirt-ci/-/merge_requests/210 > --- > .gitlab-ci.d/cirrus.yml | 16 > .gitlab-ci.d/cirrus/macos-12.vars | 16 > 2 files changed, 32 insertions(+) > create mode 100644 .gitlab-ci.d/cirrus/macos-12.vars There's going to be a minor interaction with my patches that integrate lcitool more formally in QEMU. Alex has them queued here: https://lists.gnu.org/archive/html/qemu-devel/2022-01/msg00558.html could you rebase on top of that series - all it will really mean in this case is including the git submodule update on top, and updating the refresh script to include macos-12 too https://lists.gnu.org/archive/html/qemu-devel/2022-01/msg00571.html Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v2 9/9] hw/dma: Let dma_buf_read() / dma_buf_write() propagate MemTxResult
On 1/10/22 09:51, David Hildenbrand wrote: > On 04.01.22 09:54, Philippe Mathieu-Daudé wrote: >> From: Philippe Mathieu-Daudé >> >> Since the previous commit, dma_buf_rw() returns a MemTxResult >> type. Do not discard it, return it to the caller. >> >> Since both dma_buf_read/dma_buf_write functions were previously >> returning the QEMUSGList size not consumed, add an extra argument >> where the unconsummed size can be stored. >> >> Update the few callers. > > I feel like this patch doesn't fit into the context of this series. > Especially as the cover letter mentiones "Finally we replace misuses > with dma_addr_t typedef." > > Am I missing something? OK, I will simply repost it once this series gets merged.
Re: [PATCH v2 8/9] hw/dma: Use dma_addr_t type definition when relevant
On 1/10/22 09:49, David Hildenbrand wrote: > On 04.01.22 09:54, Philippe Mathieu-Daudé wrote: >> From: Philippe Mathieu-Daudé >> >> Update the obvious places where dma_addr_t should be used >> (instead of uint64_t, hwaddr, size_t, int32_t types). >> >> This allows to have _addr_t type portable on 32/64-bit >> hosts. >> >> Move QEMUSGList declaration after dma_addr_t declaration >> so this structure can use the new type. >> >> Signed-off-by: Philippe Mathieu-Daudé >> Signed-off-by: Philippe Mathieu-Daudé >> --- >> include/sysemu/dma.h | 22 +++--- >> hw/nvme/ctrl.c| 2 +- >> hw/rdma/rdma_utils.c | 2 +- >> hw/scsi/megasas.c | 10 +- >> softmmu/dma-helpers.c | 6 +++--- >> 5 files changed, 21 insertions(+), 21 deletions(-) >> >> diff --git a/include/sysemu/dma.h b/include/sysemu/dma.h >> index 0db2478a506..7a8ae4fcd0b 100644 >> --- a/include/sysemu/dma.h >> +++ b/include/sysemu/dma.h >> @@ -15,22 +15,11 @@ >> #include "block/block.h" >> #include "block/accounting.h" >> >> -typedef struct ScatterGatherEntry ScatterGatherEntry; >> - >> typedef enum { >> DMA_DIRECTION_TO_DEVICE = 0, >> DMA_DIRECTION_FROM_DEVICE = 1, >> } DMADirection; >> >> -struct QEMUSGList { >> -ScatterGatherEntry *sg; >> -int nsg; >> -int nalloc; >> -size_t size; >> -DeviceState *dev; >> -AddressSpace *as; >> -}; >> - >> /* >> * When an IOMMU is present, bus addresses become distinct from >> * CPU/memory physical addresses and may be a different size. Because >> @@ -43,6 +32,17 @@ typedef uint64_t dma_addr_t; >> #define DMA_ADDR_BITS 64 >> #define DMA_ADDR_FMT "%" PRIx64 >> >> +typedef struct ScatterGatherEntry ScatterGatherEntry; >> + >> +struct QEMUSGList { >> +ScatterGatherEntry *sg; >> +int nsg; >> +int nalloc; >> +dma_addr_t size; >> +DeviceState *dev; >> +AddressSpace *as; >> +}; > > Changing one member while moving is sneaky. Why the move in this patch? Because dma_addr_t is declared before QEMUSGList. I will add an intermediate patch. > Apart from that and Peters comment > > Reviewed-by: David Hildenbrand Thanks (Peter's comment addressed).
Re: [PATCH v2 8/9] hw/dma: Use dma_addr_t type definition when relevant
On 04.01.22 09:54, Philippe Mathieu-Daudé wrote: > From: Philippe Mathieu-Daudé > > Update the obvious places where dma_addr_t should be used > (instead of uint64_t, hwaddr, size_t, int32_t types). > > This allows to have _addr_t type portable on 32/64-bit > hosts. > > Move QEMUSGList declaration after dma_addr_t declaration > so this structure can use the new type. > > Signed-off-by: Philippe Mathieu-Daudé > Signed-off-by: Philippe Mathieu-Daudé > --- > include/sysemu/dma.h | 22 +++--- > hw/nvme/ctrl.c| 2 +- > hw/rdma/rdma_utils.c | 2 +- > hw/scsi/megasas.c | 10 +- > softmmu/dma-helpers.c | 6 +++--- > 5 files changed, 21 insertions(+), 21 deletions(-) > > diff --git a/include/sysemu/dma.h b/include/sysemu/dma.h > index 0db2478a506..7a8ae4fcd0b 100644 > --- a/include/sysemu/dma.h > +++ b/include/sysemu/dma.h > @@ -15,22 +15,11 @@ > #include "block/block.h" > #include "block/accounting.h" > > -typedef struct ScatterGatherEntry ScatterGatherEntry; > - > typedef enum { > DMA_DIRECTION_TO_DEVICE = 0, > DMA_DIRECTION_FROM_DEVICE = 1, > } DMADirection; > > -struct QEMUSGList { > -ScatterGatherEntry *sg; > -int nsg; > -int nalloc; > -size_t size; > -DeviceState *dev; > -AddressSpace *as; > -}; > - > /* > * When an IOMMU is present, bus addresses become distinct from > * CPU/memory physical addresses and may be a different size. Because > @@ -43,6 +32,17 @@ typedef uint64_t dma_addr_t; > #define DMA_ADDR_BITS 64 > #define DMA_ADDR_FMT "%" PRIx64 > > +typedef struct ScatterGatherEntry ScatterGatherEntry; > + > +struct QEMUSGList { > +ScatterGatherEntry *sg; > +int nsg; > +int nalloc; > +dma_addr_t size; > +DeviceState *dev; > +AddressSpace *as; > +}; Changing one member while moving is sneaky. Why the move in this patch? Apart from that and Peters comment Reviewed-by: David Hildenbrand -- Thanks, David / dhildenb
Re: [PATCH v2 7/9] hw/dma: Fix format string issues using dma_addr_t
On 04.01.22 09:54, Philippe Mathieu-Daudé wrote: > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/ide/ahci.c| 2 +- > hw/rdma/trace-events | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c > index 205dfdc6622..6c727dd0c08 100644 > --- a/hw/ide/ahci.c > +++ b/hw/ide/ahci.c > @@ -1159,7 +1159,7 @@ static void process_ncq_command(AHCIState *s, int port, > const uint8_t *cmd_fis, > ahci_populate_sglist(ad, _tfs->sglist, ncq_tfs->cmdh, size, 0); > > if (ncq_tfs->sglist.size < size) { > -error_report("ahci: PRDT length for NCQ command (0x%zx) " > +error_report("ahci: PRDT length for NCQ command (0x" DMA_ADDR_FMT ") > " > "is smaller than the requested size (0x%zx)", > ncq_tfs->sglist.size, size); > ncq_err(ncq_tfs); > diff --git a/hw/rdma/trace-events b/hw/rdma/trace-events > index 9accb149734..c23175120e1 100644 > --- a/hw/rdma/trace-events > +++ b/hw/rdma/trace-events > @@ -27,5 +27,5 @@ rdma_rm_alloc_qp(uint32_t rm_qpn, uint32_t backend_qpn, > uint8_t qp_type) "rm_qpn > rdma_rm_modify_qp(uint32_t qpn, uint32_t attr_mask, int qp_state, uint8_t > sgid_idx) "qpn=0x%x, attr_mask=0x%x, qp_state=%d, sgid_idx=%d" > > # rdma_utils.c > -rdma_pci_dma_map(uint64_t addr, void *vaddr, uint64_t len) "0x%"PRIx64" -> > %p (len=%" PRId64")" > +rdma_pci_dma_map(uint64_t addr, void *vaddr, uint64_t len) "0x%"PRIx64" -> > %p (len=%" PRIu64")" > rdma_pci_dma_unmap(void *vaddr) "%p" Reviewed-by: David Hildenbrand -- Thanks, David / dhildenb
Re: [PATCH v2 6/9] hw/scsi: Rename SCSIRequest::resid as 'residual'
On 04.01.22 09:54, Philippe Mathieu-Daudé wrote: > From: Philippe Mathieu-Daudé > > The 'resid' field is slightly confusing and could be > interpreted as some ID. Rename it as 'residual' which > is clearer to review. No logical change. > > Signed-off-by: Philippe Mathieu-Daudé > Signed-off-by: Philippe Mathieu-Daudé > --- Reviewed-by: David Hildenbrand -- Thanks, David / dhildenb
Re: [PATCH v2 5/9] hw/rdma/rdma_utils: Rename rdma_pci_dma_map 'len' argument
On 04.01.22 09:54, Philippe Mathieu-Daudé wrote: > From: Philippe Mathieu-Daudé > > Various APIs use 'pval' naming for 'pointer to val'. > rdma_pci_dma_map() uses 'plen' for 'PCI length', but since > 'PCI' is already explicit in the function name, simplify > and rename the argument 'len'. No logical change. > > Signed-off-by: Philippe Mathieu-Daudé > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/rdma/rdma_utils.h | 2 +- > hw/rdma/rdma_utils.c | 14 +++--- > 2 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/hw/rdma/rdma_utils.h b/hw/rdma/rdma_utils.h > index 9fd0efd940b..0c6414e7e0a 100644 > --- a/hw/rdma/rdma_utils.h > +++ b/hw/rdma/rdma_utils.h > @@ -38,7 +38,7 @@ typedef struct RdmaProtectedGSList { > GSList *list; > } RdmaProtectedGSList; > > -void *rdma_pci_dma_map(PCIDevice *dev, dma_addr_t addr, dma_addr_t plen); > +void *rdma_pci_dma_map(PCIDevice *dev, dma_addr_t addr, dma_addr_t len); > void rdma_pci_dma_unmap(PCIDevice *dev, void *buffer, dma_addr_t len); > void rdma_protected_gqueue_init(RdmaProtectedGQueue *list); > void rdma_protected_gqueue_destroy(RdmaProtectedGQueue *list); > diff --git a/hw/rdma/rdma_utils.c b/hw/rdma/rdma_utils.c > index 98df58f6897..61cb8ede0fd 100644 > --- a/hw/rdma/rdma_utils.c > +++ b/hw/rdma/rdma_utils.c > @@ -17,29 +17,29 @@ > #include "trace.h" > #include "rdma_utils.h" > > -void *rdma_pci_dma_map(PCIDevice *dev, dma_addr_t addr, dma_addr_t plen) > +void *rdma_pci_dma_map(PCIDevice *dev, dma_addr_t addr, dma_addr_t len) > { > void *p; > -hwaddr len = plen; > +hwaddr pci_len = len; > > if (!addr) { > rdma_error_report("addr is NULL"); > return NULL; > } > > -p = pci_dma_map(dev, addr, , DMA_DIRECTION_TO_DEVICE); > +p = pci_dma_map(dev, addr, _len, DMA_DIRECTION_TO_DEVICE); > if (!p) { > rdma_error_report("pci_dma_map fail, addr=0x%"PRIx64", len=%"PRId64, > - addr, len); > + addr, pci_len); > return NULL; > } > > -if (len != plen) { > -rdma_pci_dma_unmap(dev, p, len); > +if (pci_len != len) { > +rdma_pci_dma_unmap(dev, p, pci_len); > return NULL; > } > > -trace_rdma_pci_dma_map(addr, p, len); > +trace_rdma_pci_dma_map(addr, p, pci_len); > > return p; > } Reviewed-by: David Hildenbrand -- Thanks, David / dhildenb
Re: [PATCH v2 9/9] hw/dma: Let dma_buf_read() / dma_buf_write() propagate MemTxResult
On 04.01.22 09:54, Philippe Mathieu-Daudé wrote: > From: Philippe Mathieu-Daudé > > Since the previous commit, dma_buf_rw() returns a MemTxResult > type. Do not discard it, return it to the caller. > > Since both dma_buf_read/dma_buf_write functions were previously > returning the QEMUSGList size not consumed, add an extra argument > where the unconsummed size can be stored. > > Update the few callers. I feel like this patch doesn't fit into the context of this series. Especially as the cover letter mentiones "Finally we replace misuses with dma_addr_t typedef." Am I missing something? -- Thanks, David / dhildenb
Re: [PATCH v2 6/6] gitlab-ci: Support macOS 12 via cirrus-run
On 2022/01/10 2:06, Philippe Mathieu-Daudé wrote: Add support for macOS 12 build on Cirrus-CI, similarly to commit 0e103a65ba1 ("gitlab: support for ... macOS 11 via cirrus-run"). Disable deprecation warnings on Objective C to avoid: [2789/6622] Compiling Objective-C object libcommon.fa.p/ui_cocoa.m.o ui/cocoa.m:1411:16: error: 'setAllowedFileTypes:' is deprecated: first deprecated in macOS 12.0 - Use -allowedContentTypes instead [-Werror,-Wdeprecated-declarations] [openPanel setAllowedFileTypes: supportedImageFileTypes]; ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/AppKit.framework/Headers/NSSavePanel.h:215:49: note: property 'allowedFileTypes' is declared deprecated here @property (nullable, copy) NSArray *allowedFileTypes API_DEPRECATED("Use -allowedContentTypes instead", macos(10.3,12.0)); ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/AppKit.framework/Headers/NSSavePanel.h:215:49: note: 'setAllowedFileTypes:' has been explicitly marked deprecated here FAILED: libcommon.fa.p/ui_cocoa.m.o Signed-off-by: Philippe Mathieu-Daudé Globally disabling deprecation warnings sounds bad. I came up with a few alternatives: - Just remove the call of [NSSavePanel allowedFileTypes:]. Actually I think it is harming the usability rather than improving it. An image file, which is being chosen by the panel, can be a raw file and have a variety of file extensions and many are not covered by the provided list (e.g. "udf"). Other platforms like GTK can provide an option to open a file with an extension not listed, but Cocoa can't. It forces the user to rename the file to give an extension in the list. Moreover, Cocoa does not tell which extensions are in the list so the user needs to read the source code, which is pretty bad. - Use a different method to provide the same functionality. - To locally disable the warning, enclose the statement with: #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wdeprecated-declarations" and #pragma clang diagnostic pop Regards, Akihiko Odaki --- Generated using lcitool from: https://gitlab.com/libvirt/libvirt-ci/-/merge_requests/210 --- .gitlab-ci.d/cirrus.yml | 16 .gitlab-ci.d/cirrus/macos-12.vars | 16 2 files changed, 32 insertions(+) create mode 100644 .gitlab-ci.d/cirrus/macos-12.vars diff --git a/.gitlab-ci.d/cirrus.yml b/.gitlab-ci.d/cirrus.yml index 19e6c21401b..719008b13ca 100644 --- a/.gitlab-ci.d/cirrus.yml +++ b/.gitlab-ci.d/cirrus.yml @@ -90,6 +90,22 @@ x64-macos-11-base-build: PKG_CONFIG_PATH: /usr/local/opt/curl/lib/pkgconfig:/usr/local/opt/ncurses/lib/pkgconfig:/usr/local/opt/readline/lib/pkgconfig TEST_TARGETS: check-unit check-block check-qapi-schema check-softfloat check-qtest-x86_64 +x64-macos-12-base-build: + extends: .cirrus_build_job + variables: +NAME: macos-12 +CIRRUS_VM_INSTANCE_TYPE: osx_instance +CIRRUS_VM_IMAGE_SELECTOR: image +CIRRUS_VM_IMAGE_NAME: monterey-base +CIRRUS_VM_CPUS: 12 +CIRRUS_VM_RAM: 24G +UPDATE_COMMAND: brew update +INSTALL_COMMAND: brew install +PATH_EXTRA: /usr/local/opt/ccache/libexec:/usr/local/opt/gettext/bin +PKG_CONFIG_PATH: /usr/local/opt/curl/lib/pkgconfig:/usr/local/opt/ncurses/lib/pkgconfig:/usr/local/opt/readline/lib/pkgconfig +TEST_TARGETS: check-unit check-block check-qapi-schema check-softfloat check-qtest-x86_64 +CONFIGURE_ARGS: --extra-objcflags=-Wno-deprecated-declarations + # The following jobs run VM-based tests via KVM on a Linux-based Cirrus-CI job .cirrus_kvm_job: diff --git a/.gitlab-ci.d/cirrus/macos-12.vars b/.gitlab-ci.d/cirrus/macos-12.vars new file mode 100644 index 000..997dbc762c8 --- /dev/null +++ b/.gitlab-ci.d/cirrus/macos-12.vars @@ -0,0 +1,16 @@ +# THIS FILE WAS AUTO-GENERATED +# +# $ lcitool variables macos-12 qemu +# +# https://gitlab.com/libvirt/libvirt-ci + +CCACHE='/usr/local/bin/ccache' +CPAN_PKGS='Test::Harness' +CROSS_PKGS='' +MAKE='/usr/local/bin/gmake' +NINJA='/usr/local/bin/ninja' +PACKAGING_COMMAND='brew' +PIP3='/usr/local/bin/pip3' +PKGS='bash bc bzip2 capstone ccache cpanminus ctags curl dbus diffutils dtc gcovr gettext git glib gnu-sed gnutls gtk+3 jemalloc jpeg-turbo libepoxy libffi libgcrypt libiscsi libnfs libpng libslirp libssh libtasn1 libusb libxml2 llvm lzo make meson ncurses nettle ninja perl pixman pkg-config python3 rpm2cpio sdl2 sdl2_image snappy sparse spice-protocol tesseract texinfo usbredir vde vte3 zlib zstd' +PYPI_PKGS='PyYAML numpy pillow sphinx sphinx-rtd-theme virtualenv' +PYTHON='/usr/local/bin/python3'
Re: [PATCH v2 4/9] hw/dma: Remove CONFIG_USER_ONLY check
On 04.01.22 09:54, Philippe Mathieu-Daudé wrote: > From: Philippe Mathieu-Daudé > > DMA API should not be included in user-mode emulation. > If so, build should fail. Remove the CONFIG_USER_ONLY check. > > Signed-off-by: Philippe Mathieu-Daudé > Signed-off-by: Philippe Mathieu-Daudé > --- > include/sysemu/dma.h | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/include/sysemu/dma.h b/include/sysemu/dma.h > index b3faef41b2f..0db2478a506 100644 > --- a/include/sysemu/dma.h > +++ b/include/sysemu/dma.h > @@ -31,8 +31,6 @@ struct QEMUSGList { > AddressSpace *as; > }; > > -#ifndef CONFIG_USER_ONLY > - > /* > * When an IOMMU is present, bus addresses become distinct from > * CPU/memory physical addresses and may be a different size. Because > @@ -288,7 +286,6 @@ void qemu_sglist_init(QEMUSGList *qsg, DeviceState *dev, > int alloc_hint, >AddressSpace *as); > void qemu_sglist_add(QEMUSGList *qsg, dma_addr_t base, dma_addr_t len); > void qemu_sglist_destroy(QEMUSGList *qsg); > -#endif > > typedef BlockAIOCB *DMAIOFunc(int64_t offset, QEMUIOVector *iov, >BlockCompletionFunc *cb, void *cb_opaque, Reviewed-by: David Hildenbrand -- Thanks, David / dhildenb
Re: [RFC PATCH v2 2/6] audio/coreaudio: Remove a deprecation warning on macOS 12
On 1/10/22 09:17, Akihiko Odaki wrote: > On 2022/01/10 2:06, Philippe Mathieu-Daudé wrote: >> When building on macOS 12 we get: >> >> audio/coreaudio.c:50:5: error: 'kAudioObjectPropertyElementMaster' >> is deprecated: first deprecated in macOS 12.0 >> [-Werror,-Wdeprecated-declarations] >> kAudioObjectPropertyElementMaster >> ^ >> kAudioObjectPropertyElementMain >> >> /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/CoreAudio.framework/Headers/AudioHardwareBase.h:208:5: >> note: 'kAudioObjectPropertyElementMaster' has been explicitly marked >> deprecated here >> kAudioObjectPropertyElementMaster >> API_DEPRECATED_WITH_REPLACEMENT("kAudioObjectPropertyElementMain", >> macos(10.0, 12.0), ios(2.0, 15.0), watchos(1.0, 8.0), tvos(9.0, 15.0)) >> = kAudioObjectPropertyElementMain >> ^ >> >> Use kAudioObjectPropertyElementMain (define it to >> kAudioObjectPropertyElementMaster on macOS < 12). >> >> Signed-off-by: Philippe Mathieu-Daudé >> --- >> audio/coreaudio.c | 16 ++-- >> 1 file changed, 10 insertions(+), 6 deletions(-) >> >> diff --git a/audio/coreaudio.c b/audio/coreaudio.c >> index d8a21d3e507..c836bc9dd37 100644 >> --- a/audio/coreaudio.c >> +++ b/audio/coreaudio.c >> @@ -44,10 +44,14 @@ typedef struct coreaudioVoiceOut { >> bool enabled; >> } coreaudioVoiceOut; >> +#if !defined(MAC_OS_VERSION_12_0) >> +#define kAudioObjectPropertyElementMain >> kAudioObjectPropertyElementMaster >> +#endif >> + > > Semantically MAC_OS_VERSION_12_0 defines the numeric value of version > 12.0 and its existence does not mean that > kAudioObjectPropertyElementMain is defined. I suggest the following: > #if !__is_identifier(kAudioObjectPropertyElementMain) > #define kAudioObjectPropertyElementMain kAudioObjectPropertyElementMaster > #endif OK, thank you! > Regards, > Akihiko Odaki
Re: [PATCH v2 2/9] hw/pci: Restrict pci-bus stub to sysemu
On 04.01.22 09:54, Philippe Mathieu-Daudé wrote: > From: Philippe Mathieu-Daudé > > Neither tools nor user-mode emulation require the PCI bus stub. > > Signed-off-by: Philippe Mathieu-Daudé > Signed-off-by: Philippe Mathieu-Daudé > --- > stubs/meson.build | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/stubs/meson.build b/stubs/meson.build > index 363f6fa785d..d359cbe1ad7 100644 > --- a/stubs/meson.build > +++ b/stubs/meson.build > @@ -26,7 +26,6 @@ > stub_ss.add(files('module-opts.c')) > stub_ss.add(files('monitor.c')) > stub_ss.add(files('monitor-core.c')) > -stub_ss.add(files('pci-bus.c')) > stub_ss.add(files('qemu-timer-notify-cb.c')) > stub_ss.add(files('qmp_memory_device.c')) > stub_ss.add(files('qmp-command-available.c')) > @@ -51,6 +50,7 @@ > endif > if have_system >stub_ss.add(files('fw_cfg.c')) > + stub_ss.add(files('pci-bus.c')) >stub_ss.add(files('semihost.c')) >stub_ss.add(files('usb-dev-stub.c')) >stub_ss.add(files('xen-hw-stub.c')) Reviewed-by: David Hildenbrand -- Thanks, David / dhildenb
Re: [RFC PATCH v2 2/6] audio/coreaudio: Remove a deprecation warning on macOS 12
On 2022/01/10 2:06, Philippe Mathieu-Daudé wrote: When building on macOS 12 we get: audio/coreaudio.c:50:5: error: 'kAudioObjectPropertyElementMaster' is deprecated: first deprecated in macOS 12.0 [-Werror,-Wdeprecated-declarations] kAudioObjectPropertyElementMaster ^ kAudioObjectPropertyElementMain /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/CoreAudio.framework/Headers/AudioHardwareBase.h:208:5: note: 'kAudioObjectPropertyElementMaster' has been explicitly marked deprecated here kAudioObjectPropertyElementMaster API_DEPRECATED_WITH_REPLACEMENT("kAudioObjectPropertyElementMain", macos(10.0, 12.0), ios(2.0, 15.0), watchos(1.0, 8.0), tvos(9.0, 15.0)) = kAudioObjectPropertyElementMain ^ Use kAudioObjectPropertyElementMain (define it to kAudioObjectPropertyElementMaster on macOS < 12). Signed-off-by: Philippe Mathieu-Daudé --- audio/coreaudio.c | 16 ++-- 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/audio/coreaudio.c b/audio/coreaudio.c index d8a21d3e507..c836bc9dd37 100644 --- a/audio/coreaudio.c +++ b/audio/coreaudio.c @@ -44,10 +44,14 @@ typedef struct coreaudioVoiceOut { bool enabled; } coreaudioVoiceOut; +#if !defined(MAC_OS_VERSION_12_0) +#define kAudioObjectPropertyElementMain kAudioObjectPropertyElementMaster +#endif + Semantically MAC_OS_VERSION_12_0 defines the numeric value of version 12.0 and its existence does not mean that kAudioObjectPropertyElementMain is defined. I suggest the following: #if !__is_identifier(kAudioObjectPropertyElementMain) #define kAudioObjectPropertyElementMain kAudioObjectPropertyElementMaster #endif Regards, Akihiko Odaki static const AudioObjectPropertyAddress voice_addr = { kAudioHardwarePropertyDefaultOutputDevice, kAudioObjectPropertyScopeGlobal, -kAudioObjectPropertyElementMaster +kAudioObjectPropertyElementMain }; static OSStatus coreaudio_get_voice(AudioDeviceID *id) @@ -69,7 +73,7 @@ static OSStatus coreaudio_get_framesizerange(AudioDeviceID id, AudioObjectPropertyAddress addr = { kAudioDevicePropertyBufferFrameSizeRange, kAudioDevicePropertyScopeOutput, -kAudioObjectPropertyElementMaster +kAudioObjectPropertyElementMain }; return AudioObjectGetPropertyData(id, @@ -86,7 +90,7 @@ static OSStatus coreaudio_get_framesize(AudioDeviceID id, UInt32 *framesize) AudioObjectPropertyAddress addr = { kAudioDevicePropertyBufferFrameSize, kAudioDevicePropertyScopeOutput, -kAudioObjectPropertyElementMaster +kAudioObjectPropertyElementMain }; return AudioObjectGetPropertyData(id, @@ -103,7 +107,7 @@ static OSStatus coreaudio_set_framesize(AudioDeviceID id, UInt32 *framesize) AudioObjectPropertyAddress addr = { kAudioDevicePropertyBufferFrameSize, kAudioDevicePropertyScopeOutput, -kAudioObjectPropertyElementMaster +kAudioObjectPropertyElementMain }; return AudioObjectSetPropertyData(id, @@ -121,7 +125,7 @@ static OSStatus coreaudio_set_streamformat(AudioDeviceID id, AudioObjectPropertyAddress addr = { kAudioDevicePropertyStreamFormat, kAudioDevicePropertyScopeOutput, -kAudioObjectPropertyElementMaster +kAudioObjectPropertyElementMain }; return AudioObjectSetPropertyData(id, @@ -138,7 +142,7 @@ static OSStatus coreaudio_get_isrunning(AudioDeviceID id, UInt32 *result) AudioObjectPropertyAddress addr = { kAudioDevicePropertyDeviceIsRunning, kAudioDevicePropertyScopeOutput, -kAudioObjectPropertyElementMaster +kAudioObjectPropertyElementMain }; return AudioObjectGetPropertyData(id,