Re: [PATCH 0/2] scsi: to fix issue on passing host_status to the guest kernel

2022-01-10 Thread Dongli Zhang
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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'

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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()

2022-01-10 Thread John Snow
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)

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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)

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread John Snow
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

2022-01-10 Thread Akihiko Odaki




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

2022-01-10 Thread Christian Schoenebeck
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

2022-01-10 Thread Akihiko Odaki




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

2022-01-10 Thread Christian Schoenebeck
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

2022-01-10 Thread Philippe Mathieu-Daudé
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

2022-01-10 Thread Peter Maydell
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

2022-01-10 Thread Akihiko Odaki

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()

2022-01-10 Thread Hanna Reitz

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

2022-01-10 Thread Christian Schoenebeck
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

2022-01-10 Thread Akihiko Odaki

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

2022-01-10 Thread Stefan Hajnoczi
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

2022-01-10 Thread Stefan Hajnoczi
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

2022-01-10 Thread Stefan Hajnoczi
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

2022-01-10 Thread Stefan Hajnoczi
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

2022-01-10 Thread Peter Maydell
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

2022-01-10 Thread Peter Maydell
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

2022-01-10 Thread Stefano Garzarella

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

2022-01-10 Thread Peter Maydell
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

2022-01-10 Thread Philippe Mathieu-Daudé
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

2022-01-10 Thread Christian Schoenebeck
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

2022-01-10 Thread Philippe Mathieu-Daudé
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

2022-01-10 Thread Peng Liang via
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

2022-01-10 Thread Peter Lieven
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

2022-01-10 Thread Peter Lieven
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

2022-01-10 Thread Peter Lieven
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

2022-01-10 Thread Daniel P . Berrangé
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

2022-01-10 Thread Philippe Mathieu-Daudé
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

2022-01-10 Thread Philippe Mathieu-Daudé
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

2022-01-10 Thread David Hildenbrand
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

2022-01-10 Thread David Hildenbrand
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'

2022-01-10 Thread David Hildenbrand
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

2022-01-10 Thread David Hildenbrand
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

2022-01-10 Thread David Hildenbrand
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

2022-01-10 Thread Akihiko Odaki




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

2022-01-10 Thread David Hildenbrand
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

2022-01-10 Thread Philippe Mathieu-Daudé
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

2022-01-10 Thread David Hildenbrand
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

2022-01-10 Thread Akihiko Odaki




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,