Re: [RFC PATCH 10/78] hw/ide/atapi.c: add fallthrough pseudo-keyword
On Fri, Oct 13, 2023 at 3:50 AM Emmanouil Pitsidianakis wrote: > > In preparation of raising -Wimplicit-fallthrough to 5, replace all > fall-through comments with the fallthrough attribute pseudo-keyword. > > Signed-off-by: Emmanouil Pitsidianakis > --- > hw/ide/atapi.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c > index dcc39df9a4..85c74a5ffe 100644 > --- a/hw/ide/atapi.c > +++ b/hw/ide/atapi.c > @@ -1189,53 +1189,54 @@ static void cmd_read_disc_information(IDEState *s, > uint8_t* buf) > static void cmd_read_dvd_structure(IDEState *s, uint8_t* buf) > { > int max_len; > int media = buf[1]; > int format = buf[7]; > int ret; > > max_len = lduw_be_p(buf + 8); > > if (format < 0xff) { > if (media_is_cd(s)) { > ide_atapi_cmd_error(s, ILLEGAL_REQUEST, > ASC_INCOMPATIBLE_FORMAT); > return; > } else if (!media_present(s)) { > ide_atapi_cmd_error(s, ILLEGAL_REQUEST, > ASC_INV_FIELD_IN_CMD_PACKET); > return; > } > } > > memset(buf, 0, max_len > IDE_DMA_BUF_SECTORS * BDRV_SECTOR_SIZE + 4 ? > IDE_DMA_BUF_SECTORS * BDRV_SECTOR_SIZE + 4 : max_len); > > switch (format) { > case 0x00 ... 0x7f: > case 0xff: > if (media == 0) { > ret = ide_dvd_read_structure(s, format, buf, buf); > > if (ret < 0) { > ide_atapi_cmd_error(s, ILLEGAL_REQUEST, -ret); > } else { > ide_atapi_cmd_reply(s, ret, max_len); > } > > break; > } > /* TODO: BD support, fall through for now */ > +fallthrough; ACK. For a moment I was wondering if this was something new to gcc, but I guess it's just a macro you made O:-) Acked-by: John Snow > > /* Generic disk structures */ > case 0x80: /* TODO: AACS volume identifier */ > case 0x81: /* TODO: AACS media serial number */ > case 0x82: /* TODO: AACS media identifier */ > case 0x83: /* TODO: AACS media key block */ > case 0x90: /* TODO: List of recognized format layers */ > case 0xc0: /* TODO: Write protection status */ > default: > ide_atapi_cmd_error(s, ILLEGAL_REQUEST, > ASC_INV_FIELD_IN_CMD_PACKET); > break; > } > } > -- > 2.39.2 >
[PULL 04/25] python/machine: use socketpair() for console connections
Create a socketpair for the console output. This should help eliminate race conditions around console text early in the boot process that might otherwise have been dropped on the floor before being able to connect to QEMU under "server,nowait". Signed-off-by: John Snow Reviewed-by: Ani Sinha Reviewed-by: Daniel P. Berrangé Message-id: 20230928044943.849073-5-js...@redhat.com Signed-off-by: John Snow --- python/qemu/machine/machine.py | 30 +++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py index e26109e6f0..4156b8cf7d 100644 --- a/python/qemu/machine/machine.py +++ b/python/qemu/machine/machine.py @@ -159,6 +159,8 @@ def __init__(self, self._name = name or f"{id(self):x}" self._sock_pair: Optional[Tuple[socket.socket, socket.socket]] = None +self._cons_sock_pair: Optional[ +Tuple[socket.socket, socket.socket]] = None self._temp_dir: Optional[str] = None self._base_temp_dir = base_temp_dir self._sock_dir = sock_dir @@ -316,8 +318,9 @@ def _base_args(self) -> List[str]: for _ in range(self._console_index): args.extend(['-serial', 'null']) if self._console_set: -chardev = ('socket,id=console,path=%s,server=on,wait=off' % - self._console_address) +assert self._cons_sock_pair is not None +fd = self._cons_sock_pair[0].fileno() +chardev = f"socket,id=console,fd={fd}" args.extend(['-chardev', chardev]) if self._console_device_type is None: args.extend(['-serial', 'chardev:console']) @@ -352,6 +355,10 @@ def _pre_launch(self) -> None: nickname=self._name ) +if self._console_set: +self._cons_sock_pair = socket.socketpair() +os.set_inheritable(self._cons_sock_pair[0].fileno(), True) + # NOTE: Make sure any opened resources are *definitely* freed in # _post_shutdown()! # pylint: disable=consider-using-with @@ -369,6 +376,9 @@ def _pre_launch(self) -> None: def _post_launch(self) -> None: if self._sock_pair: self._sock_pair[0].close() +if self._cons_sock_pair: +self._cons_sock_pair[0].close() + if self._qmp_connection: if self._sock_pair: self._qmp.connect() @@ -524,6 +534,11 @@ def _early_cleanup(self) -> None: self._console_socket.close() self._console_socket = None +if self._cons_sock_pair: +self._cons_sock_pair[0].close() +self._cons_sock_pair[1].close() +self._cons_sock_pair = None + def _hard_shutdown(self) -> None: """ Perform early cleanup, kill the VM, and wait for it to terminate. @@ -885,10 +900,19 @@ def console_socket(self) -> socket.socket: """ if self._console_socket is None: LOG.debug("Opening console socket") +if not self._console_set: +raise QEMUMachineError( +"Attempt to access console socket with no connection") +assert self._cons_sock_pair is not None +# os.dup() is used here for sock_fd because otherwise we'd +# have two rich python socket objects that would each try to +# close the same underlying fd when either one gets garbage +# collected. self._console_socket = console_socket.ConsoleSocket( -self._console_address, +sock_fd=os.dup(self._cons_sock_pair[1].fileno()), file=self._console_log_path, drain=self._drain_console) +self._cons_sock_pair[1].close() return self._console_socket @property -- 2.41.0
[PULL 10/25] Python: Enable python3.12 support
Python 3.12 has released, so update the test infrastructure to test against this version. Update the configure script to look for it when an explicit Python interpreter isn't chosen. Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy Message-id: 20231006195243.3131140-5-js...@redhat.com Signed-off-by: John Snow --- configure | 3 ++- python/setup.cfg | 3 ++- tests/docker/dockerfiles/python.docker | 6 +- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/configure b/configure index a49fc7b7e7..96d0dd5ffc 100755 --- a/configure +++ b/configure @@ -562,7 +562,8 @@ first_python= if test -z "${PYTHON}"; then # A bare 'python' is traditionally python 2.x, but some distros # have it as python 3.x, so check in both places. -for binary in python3 python python3.11 python3.10 python3.9 python3.8; do +for binary in python3 python python3.12 python3.11 \ + python3.10 python3.9 python3.8; do if has "$binary"; then python=$(command -v "$binary") if check_py_version "$python"; then diff --git a/python/setup.cfg b/python/setup.cfg index 8c67dce457..48668609d3 100644 --- a/python/setup.cfg +++ b/python/setup.cfg @@ -18,6 +18,7 @@ classifiers = Programming Language :: Python :: 3.9 Programming Language :: Python :: 3.10 Programming Language :: Python :: 3.11 +Programming Language :: Python :: 3.12 Typing :: Typed [options] @@ -182,7 +183,7 @@ multi_line_output=3 # of python available on your system to run this test. [tox:tox] -envlist = py38, py39, py310, py311 +envlist = py38, py39, py310, py311, py312 skip_missing_interpreters = true [testenv] diff --git a/tests/docker/dockerfiles/python.docker b/tests/docker/dockerfiles/python.docker index 383ccbdc3a..a3c1321190 100644 --- a/tests/docker/dockerfiles/python.docker +++ b/tests/docker/dockerfiles/python.docker @@ -11,7 +11,11 @@ ENV PACKAGES \ python3-pip \ python3-tox \ python3-virtualenv \ -python3.10 +python3.10 \ +python3.11 \ +python3.12 \ +python3.8 \ +python3.9 RUN dnf install -y $PACKAGES RUN rpm -q $PACKAGES | sort > /packages.txt -- 2.41.0
[PULL 12/25] qmp_shell.py: _fill_completion() use .command() instead of .cmd()
From: Vladimir Sementsov-Ogievskiy We just want to ignore failure, so we don't need low level .cmd(). This helps further renaming .command() to .cmd(). Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Eric Blake Message-id: 20231006154125.1068348-3-vsement...@yandex-team.ru Signed-off-by: John Snow --- python/qemu/qmp/qmp_shell.py | 20 ++-- 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/python/qemu/qmp/qmp_shell.py b/python/qemu/qmp/qmp_shell.py index 619ab42ced..988d79c01b 100644 --- a/python/qemu/qmp/qmp_shell.py +++ b/python/qemu/qmp/qmp_shell.py @@ -91,14 +91,21 @@ import sys from typing import ( IO, +Dict, Iterator, List, NoReturn, Optional, Sequence, +cast, ) -from qemu.qmp import ConnectError, QMPError, SocketAddrT +from qemu.qmp import ( +ConnectError, +ExecuteError, +QMPError, +SocketAddrT, +) from qemu.qmp.legacy import ( QEMUMonitorProtocol, QMPBadPortError, @@ -194,11 +201,12 @@ def close(self) -> None: super().close() def _fill_completion(self) -> None: -cmds = self.cmd('query-commands') -if 'error' in cmds: -return -for cmd in cmds['return']: -self._completer.append(cmd['name']) +try: +cmds = cast(List[Dict[str, str]], self.command('query-commands')) +for cmd in cmds: +self._completer.append(cmd['name']) +except ExecuteError: +pass def _completer_setup(self) -> None: self._completer = QMPCompleter() -- 2.41.0
[PULL 16/25] python/machine.py: upgrade vm.cmd() method
From: Vladimir Sementsov-Ogievskiy The method is not popular in iotests, we prefer use vm.qmp() and then check success by hand. But that's not optimal. To simplify movement to vm.cmd() let's support same interface improvements like in vm.qmp(). Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Eric Blake Message-id: 20231006154125.1068348-7-vsement...@yandex-team.ru Signed-off-by: John Snow --- python/qemu/machine/machine.py | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py index eae193eb00..31cb9d617d 100644 --- a/python/qemu/machine/machine.py +++ b/python/qemu/machine/machine.py @@ -708,13 +708,23 @@ def qmp(self, cmd: str, return ret def cmd(self, cmd: str, -conv_keys: bool = True, +args_dict: Optional[Dict[str, object]] = None, +conv_keys: Optional[bool] = None, **args: Any) -> QMPReturnValue: """ Invoke a QMP command. On success return the response dict. On failure raise an exception. """ +if args_dict is not None: +assert not args +assert conv_keys is None +args = args_dict +conv_keys = False + +if conv_keys is None: +conv_keys = True + qmp_args = self._qmp_args(conv_keys, args) ret = self._qmp.cmd(cmd, **qmp_args) if cmd == 'quit': -- 2.41.0
[PULL 08/25] python/qmp: remove Server.wait_closed() call for Python 3.12
This patch is a backport from https://gitlab.com/qemu-project/python-qemu-qmp/-/commit/e03a3334b6a477beb09b293708632f2c06fe9f61 According to Guido in https://github.com/python/cpython/issues/104344 , this call was never meant to wait for the server to shut down - that is handled synchronously - but instead, this waits for all connections to close. Or, it would have, if it wasn't broken since it was introduced. 3.12 fixes the bug, which now causes a hang in our code. The fix is just to remove the wait. Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy Message-id: 20231006195243.3131140-3-js...@redhat.com Signed-off-by: John Snow --- python/qemu/qmp/protocol.py | 1 - 1 file changed, 1 deletion(-) diff --git a/python/qemu/qmp/protocol.py b/python/qemu/qmp/protocol.py index 753182131f..a4ffdfad51 100644 --- a/python/qemu/qmp/protocol.py +++ b/python/qemu/qmp/protocol.py @@ -495,7 +495,6 @@ async def _stop_server(self) -> None: try: self.logger.debug("Stopping server.") self._server.close() -await self._server.wait_closed() self.logger.debug("Server stopped.") finally: self._server = None -- 2.41.0
[PULL 01/25] python/machine: move socket setup out of _base_args property
This property isn't meant to do much else besides return a list of strings, so move this setup back out into _pre_launch(). Signed-off-by: John Snow Reviewed-by: Ani Sinha Reviewed-by: Daniel P. Berrangé Message-id: 20230928044943.849073-2-js...@redhat.com Signed-off-by: John Snow --- python/qemu/machine/machine.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py index 35d5a672db..345610d6e4 100644 --- a/python/qemu/machine/machine.py +++ b/python/qemu/machine/machine.py @@ -301,9 +301,7 @@ def _base_args(self) -> List[str]: if self._qmp_set: if self._sock_pair: -fd = self._sock_pair[0].fileno() -os.set_inheritable(fd, True) -moncdev = f"socket,id=mon,fd={fd}" +moncdev = f"socket,id=mon,fd={self._sock_pair[0].fileno()}" elif isinstance(self._monitor_address, tuple): moncdev = "socket,id=mon,host={},port={}".format( *self._monitor_address @@ -340,6 +338,7 @@ def _pre_launch(self) -> None: if self._qmp_set: if self._monitor_address is None: self._sock_pair = socket.socketpair() +os.set_inheritable(self._sock_pair[0].fileno(), True) sock = self._sock_pair[1] if isinstance(self._monitor_address, str): self._remove_files.append(self._monitor_address) -- 2.41.0
[PULL 13/25] scripts/cpu-x86-uarch-abi.py: use .command() instead of .cmd()
From: Vladimir Sementsov-Ogievskiy Here we don't expect a failure. In case of failure we'll crash on trying to access ['return']. Better is to use .command() that clearly raises on failure. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Eric Blake Message-id: 20231006154125.1068348-4-vsement...@yandex-team.ru Signed-off-by: John Snow --- scripts/cpu-x86-uarch-abi.py | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/scripts/cpu-x86-uarch-abi.py b/scripts/cpu-x86-uarch-abi.py index 82ff07582f..893afd1b35 100644 --- a/scripts/cpu-x86-uarch-abi.py +++ b/scripts/cpu-x86-uarch-abi.py @@ -69,7 +69,7 @@ shell = QEMUMonitorProtocol(sock) shell.connect() -models = shell.cmd("query-cpu-definitions") +models = shell.command("query-cpu-definitions") # These QMP props don't correspond to CPUID fatures # so ignore them @@ -85,7 +85,7 @@ names = [] -for model in models["return"]: +for model in models: if "alias-of" in model: continue names.append(model["name"]) @@ -93,12 +93,12 @@ models = {} for name in sorted(names): -cpu = shell.cmd("query-cpu-model-expansion", - { "type": "static", - "model": { "name": name }}) +cpu = shell.command("query-cpu-model-expansion", +{ "type": "static", + "model": { "name": name }}) got = {} -for (feature, present) in cpu["return"]["model"]["props"].items(): +for (feature, present) in cpu["model"]["props"].items(): if present and feature not in skip: got[feature] = True -- 2.41.0
[PULL 17/25] iotests: QemuStorageDaemon: add cmd() method like in QEMUMachine.
From: Vladimir Sementsov-Ogievskiy Add similar method for consistency. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Eric Blake Message-id: 20231006154125.1068348-8-vsement...@yandex-team.ru Signed-off-by: John Snow --- tests/qemu-iotests/iotests.py | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index 0c061d8986..ee7b6ddeff 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -38,7 +38,7 @@ from contextlib import contextmanager from qemu.machine import qtest -from qemu.qmp.legacy import QMPMessage, QEMUMonitorProtocol +from qemu.qmp.legacy import QMPMessage, QMPReturnValue, QEMUMonitorProtocol from qemu.utils import VerboseProcessError # Use this logger for logging messages directly from the iotests module @@ -466,6 +466,11 @@ def get_qmp(self) -> QEMUMonitorProtocol: assert self._qmp is not None return self._qmp +def cmd(self, cmd: str, args: Optional[Dict[str, object]] = None) \ +-> QMPReturnValue: +assert self._qmp is not None +return self._qmp.cmd(cmd, **(args or {})) + def stop(self, kill_signal=15): self._p.send_signal(kill_signal) self._p.wait() -- 2.41.0
[PULL 03/25] python/console_socket: accept existing FD in initializer
Useful if we want to use ConsoleSocket() for a socket created by socketpair(). Signed-off-by: John Snow Reviewed-by: Ani Sinha Reviewed-by: Daniel P. Berrangé Message-id: 20230928044943.849073-4-js...@redhat.com Signed-off-by: John Snow --- python/qemu/machine/console_socket.py | 29 +++ 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/python/qemu/machine/console_socket.py b/python/qemu/machine/console_socket.py index 4e28ba9bb2..0a4e09ffc7 100644 --- a/python/qemu/machine/console_socket.py +++ b/python/qemu/machine/console_socket.py @@ -24,19 +24,32 @@ class ConsoleSocket(socket.socket): """ ConsoleSocket represents a socket attached to a char device. -Optionally (if drain==True), drains the socket and places the bytes -into an in memory buffer for later processing. - -Optionally a file path can be passed in and we will also -dump the characters to this file for debugging purposes. +:param address: An AF_UNIX path or address. +:param sock_fd: Optionally, an existing socket file descriptor. +One of address or sock_fd must be specified. +:param file: Optionally, a filename to log to. +:param drain: Optionally, drains the socket and places the bytes + into an in memory buffer for later processing. """ -def __init__(self, address: str, file: Optional[str] = None, +def __init__(self, + address: Optional[str] = None, + sock_fd: Optional[int] = None, + file: Optional[str] = None, drain: bool = False): +if address is None and sock_fd is None: +raise ValueError("one of 'address' or 'sock_fd' must be specified") +if address is not None and sock_fd is not None: +raise ValueError("can't specify both 'address' and 'sock_fd'") + self._recv_timeout_sec = 300.0 self._sleep_time = 0.5 self._buffer: Deque[int] = deque() -socket.socket.__init__(self, socket.AF_UNIX, socket.SOCK_STREAM) -self.connect(address) +if address is not None: +socket.socket.__init__(self, socket.AF_UNIX, socket.SOCK_STREAM) +self.connect(address) +else: +assert sock_fd is not None +socket.socket.__init__(self, fileno=sock_fd) self._logfile = None if file: # pylint: disable=consider-using-with -- 2.41.0
[PULL 23/25] tests/vm/basevm.py: use cmd() instead of qmp()
From: Vladimir Sementsov-Ogievskiy We don't expect failure here and need 'result' object. cmd() is better in this case. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Eric Blake Message-id: 20231006154125.1068348-14-vsement...@yandex-team.ru Signed-off-by: John Snow --- tests/vm/basevm.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py index a97e23b0ce..8aef4cff96 100644 --- a/tests/vm/basevm.py +++ b/tests/vm/basevm.py @@ -312,8 +312,8 @@ def boot(self, img, extra_args=[]): self._guest = guest # Init console so we can start consuming the chars. self.console_init() -usernet_info = guest.qmp("human-monitor-command", - command_line="info usernet").get("return") +usernet_info = guest.cmd("human-monitor-command", + command_line="info usernet") self.ssh_port = get_info_usernet_hostfwd_port(usernet_info) if not self.ssh_port: raise Exception("Cannot find ssh port from 'info usernet':\n%s" % \ -- 2.41.0
[PULL 07/25] Python/iotests: Add type hint for nbd module
The test bails gracefully if this module isn't installed, but linters need a little help understanding that. It's enough to just declare the type in this case. (Fixes pylint complaining about use of an uninitialized variable because it isn't wise enough to understand the notrun call is noreturn.) Signed-off-by: John Snow Reviewed-by: Eric Blake Reviewed-by: Vladimir Sementsov-Ogievskiy Message-id: 20231006195243.3131140-2-js...@redhat.com Signed-off-by: John Snow --- tests/qemu-iotests/tests/nbd-multiconn | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/qemu-iotests/tests/nbd-multiconn b/tests/qemu-iotests/tests/nbd-multiconn index 478a1eaba2..7e686a786e 100755 --- a/tests/qemu-iotests/tests/nbd-multiconn +++ b/tests/qemu-iotests/tests/nbd-multiconn @@ -20,6 +20,8 @@ import os from contextlib import contextmanager +from types import ModuleType + import iotests from iotests import qemu_img_create, qemu_io @@ -28,7 +30,7 @@ disk = os.path.join(iotests.test_dir, 'disk') size = '4M' nbd_sock = os.path.join(iotests.sock_dir, 'nbd_sock') nbd_uri = 'nbd+unix:///{}?socket=' + nbd_sock - +nbd: ModuleType @contextmanager def open_nbd(export_name): -- 2.41.0
[PULL 18/25] iotests: add some missed checks of qmp result
From: Vladimir Sementsov-Ogievskiy Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Eric Blake Message-id: 20231006154125.1068348-9-vsement...@yandex-team.ru Signed-off-by: John Snow --- tests/qemu-iotests/041| 1 + tests/qemu-iotests/151| 1 + tests/qemu-iotests/152| 2 ++ tests/qemu-iotests/tests/migrate-bitmaps-test | 2 ++ 4 files changed, 6 insertions(+) diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041 index 8429958bf0..4d7a829b65 100755 --- a/tests/qemu-iotests/041 +++ b/tests/qemu-iotests/041 @@ -1087,6 +1087,7 @@ class TestRepairQuorum(iotests.QMPTestCase): result = self.vm.qmp('blockdev-snapshot-sync', node_name='img1', snapshot_file=quorum_snapshot_file, snapshot_node_name="snap1"); +self.assert_qmp(result, 'return', {}) result = self.vm.qmp('drive-mirror', job_id='job0', device='quorum0', sync='full', node_name='repair0', replaces="img1", diff --git a/tests/qemu-iotests/151 b/tests/qemu-iotests/151 index b4d1bc2553..668d0c1e9c 100755 --- a/tests/qemu-iotests/151 +++ b/tests/qemu-iotests/151 @@ -159,6 +159,7 @@ class TestActiveMirror(iotests.QMPTestCase): sync='full', copy_mode='write-blocking', speed=1) +self.assert_qmp(result, 'return', {}) self.vm.hmp_qemu_io('source', 'break write_aio A') self.vm.hmp_qemu_io('source', 'aio_write 0 1M') # 1 diff --git a/tests/qemu-iotests/152 b/tests/qemu-iotests/152 index 4e179c340f..b73a0d08a2 100755 --- a/tests/qemu-iotests/152 +++ b/tests/qemu-iotests/152 @@ -43,6 +43,7 @@ class TestUnaligned(iotests.QMPTestCase): def test_unaligned(self): result = self.vm.qmp('drive-mirror', device='drive0', sync='full', granularity=65536, target=target_img) +self.assert_qmp(result, 'return', {}) self.complete_and_wait() self.vm.shutdown() self.assertEqual(iotests.image_size(test_img), iotests.image_size(target_img), @@ -51,6 +52,7 @@ class TestUnaligned(iotests.QMPTestCase): def test_unaligned_with_update(self): result = self.vm.qmp('drive-mirror', device='drive0', sync='full', granularity=65536, target=target_img) +self.assert_qmp(result, 'return', {}) self.wait_ready() self.vm.hmp_qemu_io('drive0', 'write 0 512') self.complete_and_wait(wait_ready=False) diff --git a/tests/qemu-iotests/tests/migrate-bitmaps-test b/tests/qemu-iotests/tests/migrate-bitmaps-test index 59f3357580..8668caae1e 100755 --- a/tests/qemu-iotests/tests/migrate-bitmaps-test +++ b/tests/qemu-iotests/tests/migrate-bitmaps-test @@ -101,6 +101,7 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase): sha256 = get_bitmap_hash(self.vm_a) result = self.vm_a.qmp('migrate', uri=mig_cmd) +self.assert_qmp(result, 'return', {}) while True: event = self.vm_a.event_wait('MIGRATION') if event['data']['status'] == 'completed': @@ -176,6 +177,7 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase): self.assert_qmp(result, 'return', {}) result = self.vm_a.qmp('migrate', uri=mig_cmd) +self.assert_qmp(result, 'return', {}) while True: event = self.vm_a.event_wait('MIGRATION') if event['data']['status'] == 'completed': -- 2.41.0
[PULL 11/25] python/qemu/qmp/legacy: cmd(): drop cmd_id unused argument
From: Vladimir Sementsov-Ogievskiy The argument is unused, let's drop it for now, as we are going to refactor the interface and don't want to refactor unused things. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Eric Blake Message-id: 20231006154125.1068348-2-vsement...@yandex-team.ru Signed-off-by: John Snow --- python/qemu/qmp/legacy.py | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/python/qemu/qmp/legacy.py b/python/qemu/qmp/legacy.py index e1e9383978..fe115e301c 100644 --- a/python/qemu/qmp/legacy.py +++ b/python/qemu/qmp/legacy.py @@ -195,20 +195,16 @@ def cmd_obj(self, qmp_cmd: QMPMessage) -> QMPMessage: ) def cmd(self, name: str, -args: Optional[Dict[str, object]] = None, -cmd_id: Optional[object] = None) -> QMPMessage: +args: Optional[Dict[str, 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: -- 2.41.0
[PULL 19/25] iotests: refactor some common qmp result checks into generic pattern
From: Vladimir Sementsov-Ogievskiy To simplify further conversion. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Eric Blake Message-id: 20231006154125.1068348-10-vsement...@yandex-team.ru Signed-off-by: John Snow --- tests/qemu-iotests/040 | 3 ++- tests/qemu-iotests/147 | 3 ++- tests/qemu-iotests/155 | 4 ++-- tests/qemu-iotests/218 | 4 ++-- tests/qemu-iotests/296 | 3 ++- 5 files changed, 10 insertions(+), 7 deletions(-) diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040 index 5601a4873c..e61e7f2433 100755 --- a/tests/qemu-iotests/040 +++ b/tests/qemu-iotests/040 @@ -62,9 +62,10 @@ class ImageCommitTestCase(iotests.QMPTestCase): self.assert_no_active_block_jobs() if node_names: result = self.vm.qmp('block-commit', device='drive0', top_node=top, base_node=base) +self.assert_qmp(result, 'return', {}) else: result = self.vm.qmp('block-commit', device='drive0', top=top, base=base) -self.assert_qmp(result, 'return', {}) +self.assert_qmp(result, 'return', {}) self.wait_for_complete(need_ready) def run_default_commit_test(self): diff --git a/tests/qemu-iotests/147 b/tests/qemu-iotests/147 index 47dfa62e6b..770b73e2f4 100755 --- a/tests/qemu-iotests/147 +++ b/tests/qemu-iotests/147 @@ -159,10 +159,11 @@ class BuiltinNBD(NBDBlockdevAddBase): if export_name is None: result = self.server.qmp('nbd-server-add', device='nbd-export') +self.assert_qmp(result, 'return', {}) else: result = self.server.qmp('nbd-server-add', device='nbd-export', name=export_name) -self.assert_qmp(result, 'return', {}) +self.assert_qmp(result, 'return', {}) if export_name2 is not None: result = self.server.qmp('nbd-server-add', device='nbd-export', diff --git a/tests/qemu-iotests/155 b/tests/qemu-iotests/155 index eadda52615..d3e1b7401e 100755 --- a/tests/qemu-iotests/155 +++ b/tests/qemu-iotests/155 @@ -181,6 +181,7 @@ class MirrorBaseClass(BaseClass): result = self.vm.qmp(self.cmd, job_id='mirror-job', device='source', sync=sync, target='target', auto_finalize=False) +self.assert_qmp(result, 'return', {}) else: if self.existing: mode = 'existing' @@ -190,8 +191,7 @@ class MirrorBaseClass(BaseClass): sync=sync, target=target_img, format=iotests.imgfmt, mode=mode, node_name='target', auto_finalize=False) - -self.assert_qmp(result, 'return', {}) +self.assert_qmp(result, 'return', {}) self.vm.run_job('mirror-job', auto_finalize=False, pre_finalize=self.openBacking, auto_dismiss=True) diff --git a/tests/qemu-iotests/218 b/tests/qemu-iotests/218 index 6320c4cb56..5e74c55b6a 100755 --- a/tests/qemu-iotests/218 +++ b/tests/qemu-iotests/218 @@ -61,14 +61,14 @@ def start_mirror(vm, speed=None, buf_size=None): sync='full', speed=speed, buf_size=buf_size) +assert ret['return'] == {} else: ret = vm.qmp('blockdev-mirror', job_id='mirror', device='source', target='target', sync='full') - -assert ret['return'] == {} +assert ret['return'] == {} log('') diff --git a/tests/qemu-iotests/296 b/tests/qemu-iotests/296 index 0d21b740a7..19a674c5ae 100755 --- a/tests/qemu-iotests/296 +++ b/tests/qemu-iotests/296 @@ -133,9 +133,10 @@ class EncryptionSetupTestCase(iotests.QMPTestCase): if reOpen: result = vm.qmp(command, options=[opts]) +self.assert_qmp(result, 'return', {}) else: result = vm.qmp(command, **opts) -self.assert_qmp(result, 'return', {}) +self.assert_qmp(result, 'return', {}) ### -- 2.41.0
[PULL 20/25] iotests: drop some extra semicolons
From: Vladimir Sementsov-Ogievskiy Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Eric Blake Message-id: 20231006154125.1068348-11-vsement...@yandex-team.ru Signed-off-by: John Snow --- tests/qemu-iotests/041 | 2 +- tests/qemu-iotests/196 | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041 index 4d7a829b65..550e4dc391 100755 --- a/tests/qemu-iotests/041 +++ b/tests/qemu-iotests/041 @@ -1086,7 +1086,7 @@ class TestRepairQuorum(iotests.QMPTestCase): def test_after_a_quorum_snapshot(self): result = self.vm.qmp('blockdev-snapshot-sync', node_name='img1', snapshot_file=quorum_snapshot_file, - snapshot_node_name="snap1"); + snapshot_node_name="snap1") self.assert_qmp(result, 'return', {}) result = self.vm.qmp('drive-mirror', job_id='job0', device='quorum0', diff --git a/tests/qemu-iotests/196 b/tests/qemu-iotests/196 index 76509a5ad1..27c1629be3 100755 --- a/tests/qemu-iotests/196 +++ b/tests/qemu-iotests/196 @@ -46,7 +46,7 @@ class TestInvalidateAutoclear(iotests.QMPTestCase): def test_migration(self): result = self.vm_a.qmp('migrate', uri='exec:cat>' + migfile) -self.assert_qmp(result, 'return', {}); +self.assert_qmp(result, 'return', {}) self.assertNotEqual(self.vm_a.event_wait("STOP"), None) with open(disk, 'r+b') as f: -- 2.41.0
[PULL 09/25] configure: fix error message to say Python 3.8
Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy Message-id: 20231006195243.3131140-4-js...@redhat.com Signed-off-by: John Snow --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index 8fada85a71..a49fc7b7e7 100755 --- a/configure +++ b/configure @@ -948,7 +948,7 @@ then # If first_python is set, there was a binary somewhere even though # it was not suitable. Use it for the error message. if test -n "$first_python"; then -error_exit "Cannot use '$first_python', Python >= 3.7 is required." \ +error_exit "Cannot use '$first_python', Python >= 3.8 is required." \ "Use --python=/path/to/python to specify a supported Python." else error_exit "Python not found. Use --python=/path/to/python" -- 2.41.0
[PULL 15/25] python/qemu: rename command() to cmd()
From: Vladimir Sementsov-Ogievskiy Use a shorter name. We are going to move in iotests from qmp() to command() where possible. But command() is longer than qmp() and don't look better. Let's rename. You can simply grep for '\.command(' and for 'def command(' to check that everything is updated (command() in tests/docker/docker.py is unrelated). Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Daniel P. Berrangé Reviewed-by: Eric Blake Reviewed-by: Cédric Le Goater Reviewed-by: Juan Quintela Message-id: 20231006154125.1068348-6-vsement...@yandex-team.ru [vsementsov: also update three occurrences in tests/avocado/machine_aspeed.py and keep r-b] Signed-off-by: John Snow --- docs/devel/testing.rst| 10 +- python/qemu/machine/machine.py| 8 +- python/qemu/qmp/legacy.py | 2 +- python/qemu/qmp/qmp_shell.py | 2 +- python/qemu/utils/qemu_ga_client.py | 2 +- python/qemu/utils/qom.py | 8 +- python/qemu/utils/qom_common.py | 2 +- python/qemu/utils/qom_fuse.py | 6 +- scripts/cpu-x86-uarch-abi.py | 8 +- scripts/device-crash-test | 8 +- scripts/render_block_graph.py | 8 +- tests/avocado/avocado_qemu/__init__.py| 4 +- tests/avocado/cpu_queries.py | 5 +- tests/avocado/hotplug_cpu.py | 10 +- tests/avocado/info_usernet.py | 4 +- tests/avocado/machine_arm_integratorcp.py | 6 +- tests/avocado/machine_aspeed.py | 12 +- tests/avocado/machine_m68k_nextcube.py| 4 +- tests/avocado/machine_mips_malta.py | 6 +- tests/avocado/machine_s390_ccw_virtio.py | 28 ++--- tests/avocado/migration.py| 10 +- tests/avocado/pc_cpu_hotplug_props.py | 2 +- tests/avocado/version.py | 4 +- tests/avocado/virtio_check_params.py | 6 +- tests/avocado/virtio_version.py | 5 +- tests/avocado/x86_cpu_model_versions.py | 13 +- tests/migration/guestperf/engine.py | 144 +++--- tests/qemu-iotests/256| 34 ++--- tests/qemu-iotests/257| 34 ++--- 29 files changed, 200 insertions(+), 195 deletions(-) diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst index f3e2472189..b0680cbb22 100644 --- a/docs/devel/testing.rst +++ b/docs/devel/testing.rst @@ -1014,8 +1014,8 @@ class. Here's a simple usage example: """ def test_qmp_human_info_version(self): self.vm.launch() - res = self.vm.command('human-monitor-command', -command_line='info version') + res = self.vm.cmd('human-monitor-command', +command_line='info version') self.assertRegexpMatches(res, r'^(\d+\.\d+\.\d)') To execute your test, run: @@ -1065,15 +1065,15 @@ and hypothetical example follows: first_machine.launch() second_machine.launch() - first_res = first_machine.command( + first_res = first_machine.cmd( 'human-monitor-command', command_line='info version') - second_res = second_machine.command( + second_res = second_machine.cmd( 'human-monitor-command', command_line='info version') - third_res = self.get_vm(name='third_machine').command( + third_res = self.get_vm(name='third_machine').cmd( 'human-monitor-command', command_line='info version') diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py index 50bba33729..eae193eb00 100644 --- a/python/qemu/machine/machine.py +++ b/python/qemu/machine/machine.py @@ -707,16 +707,16 @@ def qmp(self, cmd: str, self._quit_issued = True return ret -def command(self, cmd: str, -conv_keys: bool = True, -**args: Any) -> QMPReturnValue: +def cmd(self, cmd: str, +conv_keys: bool = True, +**args: Any) -> QMPReturnValue: """ Invoke a QMP command. On success return the response dict. On failure raise an exception. """ qmp_args = self._qmp_args(conv_keys, args) -ret = self._qmp.command(cmd, **qmp_args) +ret = self._qmp.cmd(cmd, **qmp_args) if cmd == 'quit': self._quit_issued = True return ret diff --git a/python/qemu/qmp/legacy.py b/python/qemu/qmp/legacy.py index e5fa1ce9c4..22a2b5616e 100644 --- a/python/qemu/qmp/legacy.py +++ b/python/qemu/qmp/legacy.py @@ -207,7 +207,7 @@ def cmd_raw(self, name: str, qmp_cmd['arguments'] = args return self.cmd_obj(qmp_cmd) -def command(self, cmd: str, **kwds: object) -> QMPReturnValue: +def cmd(self, cmd: str, **kwds: object) -> QMPReturnValue: """ Build and send a QMP command to
[PULL 21/25] iotests: drop some extra ** in qmp() call
From: Vladimir Sementsov-Ogievskiy qmp() method supports passing dict (if it doesn't need substituting '_' to '-' in keys). So, drop some extra '**' operators. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Eric Blake Message-id: 20231006154125.1068348-12-vsement...@yandex-team.ru Signed-off-by: John Snow --- tests/qemu-iotests/040| 4 +- tests/qemu-iotests/041| 14 +++--- tests/qemu-iotests/129| 2 +- tests/qemu-iotests/147| 2 +- tests/qemu-iotests/155| 2 +- tests/qemu-iotests/264| 12 ++--- tests/qemu-iotests/295| 5 +- tests/qemu-iotests/296| 15 +++--- tests/qemu-iotests/tests/migrate-bitmaps-test | 4 +- .../tests/mirror-ready-cancel-error | 50 +-- 10 files changed, 54 insertions(+), 56 deletions(-) diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040 index e61e7f2433..4b8bf09a5d 100755 --- a/tests/qemu-iotests/040 +++ b/tests/qemu-iotests/040 @@ -774,7 +774,7 @@ class TestCommitWithFilters(iotests.QMPTestCase): result = self.vm.qmp('object-add', qom_type='throttle-group', id='tg') self.assert_qmp(result, 'return', {}) -result = self.vm.qmp('blockdev-add', **{ +result = self.vm.qmp('blockdev-add', { 'node-name': 'top-filter', 'driver': 'throttle', 'throttle-group': 'tg', @@ -935,7 +935,7 @@ class TestCommitWithOverriddenBacking(iotests.QMPTestCase): self.vm.launch() # Use base_b instead of base_a as the backing of top -result = self.vm.qmp('blockdev-add', **{ +result = self.vm.qmp('blockdev-add', { 'node-name': 'top', 'driver': iotests.imgfmt, 'file': { diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041 index 550e4dc391..3aef42aec8 100755 --- a/tests/qemu-iotests/041 +++ b/tests/qemu-iotests/041 @@ -236,7 +236,7 @@ class TestSingleBlockdev(TestSingleDrive): args = {'driver': iotests.imgfmt, 'node-name': self.qmp_target, 'file': { 'filename': target_img, 'driver': 'file' } } -result = self.vm.qmp("blockdev-add", **args) +result = self.vm.qmp("blockdev-add", args) self.assert_qmp(result, 'return', {}) def test_mirror_to_self(self): @@ -963,7 +963,7 @@ class TestRepairQuorum(iotests.QMPTestCase): #assemble the quorum block device from the individual files args = { "driver": "quorum", "node-name": "quorum0", "vote-threshold": 2, "children": [ "img0", "img1", "img2" ] } -result = self.vm.qmp("blockdev-add", **args) +result = self.vm.qmp("blockdev-add", args) self.assert_qmp(result, 'return', {}) @@ -1278,7 +1278,7 @@ class TestReplaces(iotests.QMPTestCase): """ Check that we can replace filter nodes. """ -result = self.vm.qmp('blockdev-add', **{ +result = self.vm.qmp('blockdev-add', { 'driver': 'copy-on-read', 'node-name': 'filter0', 'file': { @@ -1319,7 +1319,7 @@ class TestFilters(iotests.QMPTestCase): self.vm = iotests.VM().add_device('virtio-scsi,id=vio-scsi') self.vm.launch() -result = self.vm.qmp('blockdev-add', **{ +result = self.vm.qmp('blockdev-add', { 'node-name': 'target', 'driver': iotests.imgfmt, 'file': { @@ -1355,7 +1355,7 @@ class TestFilters(iotests.QMPTestCase): os.remove(backing_img) def test_cor(self): -result = self.vm.qmp('blockdev-add', **{ +result = self.vm.qmp('blockdev-add', { 'node-name': 'filter', 'driver': 'copy-on-read', 'file': self.filterless_chain @@ -1384,7 +1384,7 @@ class TestFilters(iotests.QMPTestCase): assert target_map[1]['depth'] == 0 def test_implicit_mirror_filter(self): -result = self.vm.qmp('blockdev-add', **self.filterless_chain) +result = self.vm.qmp('blockdev-add', self.filterless_chain) self.assert_qmp(result, 'return', {}) # We need this so we can query from above the mirror node @@ -1418,7 +1418,7 @@ class TestFilters(iotests.QMPTestCase): def test_explicit_mirror_filter(self): # Same test as above, but this time we give the mirror filter # a node-name so it will not be invisible -result = self.vm.qmp('blockdev-add', **self.filterless_chain) +result = self.vm.qmp('blockdev-add', self.filterless_chain) self.assert_qmp(result, 'return', {})
[PULL 05/25] python/machine: use socketpair() for qtest connection
Like the QMP and console sockets, begin using socketpairs for the qtest connection, too. After this patch, we'll be able to remove the vestigial sock_dir argument, but that cleanup is best done in its own patch. Signed-off-by: John Snow Reviewed-by: Daniel P. Berrangé Message-id: 20230928044943.849073-6-js...@redhat.com Signed-off-by: John Snow --- python/qemu/machine/qtest.py | 49 +--- 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/python/qemu/machine/qtest.py b/python/qemu/machine/qtest.py index 1c46138bd0..8180d3ab01 100644 --- a/python/qemu/machine/qtest.py +++ b/python/qemu/machine/qtest.py @@ -24,6 +24,7 @@ Optional, Sequence, TextIO, +Tuple, ) from qemu.qmp import SocketAddrT @@ -38,23 +39,41 @@ class QEMUQtestProtocol: :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: server mode, listens on the socket (bool) +:param sock: An existing socket can be provided as an alternative to + an address. One of address or sock must be provided. +:param server: server mode, listens on the socket. Only meaningful + in conjunction with an address and not an existing + socket. + :raise socket.error: on socket connection errors .. note:: No connection is established by __init__(), this is done by the connect() or accept() methods. """ -def __init__(self, address: SocketAddrT, +def __init__(self, + address: Optional[SocketAddrT] = None, + sock: Optional[socket.socket] = None, server: bool = False): +if address is None and sock is None: +raise ValueError("Either 'address' or 'sock' must be specified") +if address is not None and sock is not None: +raise ValueError( +"Either 'address' or 'sock' must be specified, but not both") +if sock is not None and server: +raise ValueError("server=True is meaningless when passing socket") + self._address = address -self._sock = self._get_sock() +self._sock = sock or self._get_sock() self._sockfile: Optional[TextIO] = None + if server: +assert self._address is not None self._sock.bind(self._address) self._sock.listen(1) def _get_sock(self) -> socket.socket: +assert self._address is not None if isinstance(self._address, tuple): family = socket.AF_INET else: @@ -67,7 +86,8 @@ def connect(self) -> None: @raise socket.error on socket connection errors """ -self._sock.connect(self._address) +if self._address is not None: +self._sock.connect(self._address) self._sockfile = self._sock.makefile(mode='r') def accept(self) -> None: @@ -127,29 +147,40 @@ def __init__(self, base_temp_dir=base_temp_dir, sock_dir=sock_dir, qmp_timer=qmp_timer) self._qtest: Optional[QEMUQtestProtocol] = None -self._qtest_path = os.path.join(sock_dir, name + "-qtest.sock") +self._qtest_sock_pair: Optional[ +Tuple[socket.socket, socket.socket]] = None @property def _base_args(self) -> List[str]: args = super()._base_args +assert self._qtest_sock_pair is not None +fd = self._qtest_sock_pair[0].fileno() args.extend([ -'-qtest', f"unix:path={self._qtest_path}", +'-chardev', f"socket,id=qtest,fd={fd}", +'-qtest', 'chardev:qtest', '-accel', 'qtest' ]) return args def _pre_launch(self) -> None: +self._qtest_sock_pair = socket.socketpair() +os.set_inheritable(self._qtest_sock_pair[0].fileno(), True) super()._pre_launch() -self._qtest = QEMUQtestProtocol(self._qtest_path, server=True) +self._qtest = QEMUQtestProtocol(sock=self._qtest_sock_pair[1]) def _post_launch(self) -> None: assert self._qtest is not None super()._post_launch() -self._qtest.accept() +if self._qtest_sock_pair: +self._qtest_sock_pair[0].close() +self._qtest.connect() def _post_shutdown(self) -> None: +if self._qtest_sock_pair: +self._qtest_sock_pair[0].close() +self._qtest_sock_pair[1].close() +self._qtest_sock_pair = None super()._post_shutdown() -self._remove_if_exists(self._qtest_path) def qtest(self, cmd: str) -> str: """ -- 2.41.0
[PULL 14/25] python: rename QEMUMonitorProtocol.cmd() to cmd_raw()
From: Vladimir Sementsov-Ogievskiy Having cmd() and command() methods in one class doesn't look good. Rename cmd() to cmd_raw(), to show its meaning better. We also want to rename command() to cmd() in future, so this commit is a necessary step. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Eric Blake Message-id: 20231006154125.1068348-5-vsement...@yandex-team.ru Signed-off-by: John Snow --- python/qemu/machine/machine.py | 2 +- python/qemu/qmp/legacy.py | 4 ++-- tests/qemu-iotests/iotests.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py index d539e91268..50bba33729 100644 --- a/python/qemu/machine/machine.py +++ b/python/qemu/machine/machine.py @@ -702,7 +702,7 @@ def qmp(self, cmd: str, conv_keys = True qmp_args = self._qmp_args(conv_keys, args) -ret = self._qmp.cmd(cmd, args=qmp_args) +ret = self._qmp.cmd_raw(cmd, args=qmp_args) if cmd == 'quit' and 'error' not in ret and 'return' in ret: self._quit_issued = True return ret diff --git a/python/qemu/qmp/legacy.py b/python/qemu/qmp/legacy.py index fe115e301c..e5fa1ce9c4 100644 --- a/python/qemu/qmp/legacy.py +++ b/python/qemu/qmp/legacy.py @@ -194,8 +194,8 @@ def cmd_obj(self, qmp_cmd: QMPMessage) -> QMPMessage: ) ) -def cmd(self, name: str, -args: Optional[Dict[str, object]] = None) -> QMPMessage: +def cmd_raw(self, name: str, +args: Optional[Dict[str, object]] = None) -> QMPMessage: """ Build a QMP command and send it to the QMP Monitor. diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index 145c682713..0c061d8986 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -460,7 +460,7 @@ def __init__(self, *args: str, instance_id: str = 'a', qmp: bool = False): def qmp(self, cmd: str, args: Optional[Dict[str, object]] = None) \ -> QMPMessage: assert self._qmp is not None -return self._qmp.cmd(cmd, args) +return self._qmp.cmd_raw(cmd, args) def get_qmp(self) -> QEMUMonitorProtocol: assert self._qmp is not None -- 2.41.0
[PULL 22/25] iotests.py: pause_job(): drop return value
From: Vladimir Sementsov-Ogievskiy The returned value is unused. It's simple to check by command git grep -B 3 '\.pause_job(' Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Eric Blake Message-id: 20231006154125.1068348-13-vsement...@yandex-team.ru Signed-off-by: John Snow --- tests/qemu-iotests/iotests.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index ee7b6ddeff..f43814e802 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -1338,8 +1338,7 @@ def pause_job(self, job_id='job0', wait=True): result = self.vm.qmp('block-job-pause', device=job_id) self.assert_qmp(result, 'return', {}) if wait: -return self.pause_wait(job_id) -return result +self.pause_wait(job_id) def case_skip(self, reason): '''Skip this test case''' -- 2.41.0
[PULL 24/25] scripts: add python_qmp_updater.py
From: Vladimir Sementsov-Ogievskiy A script, to update the pattern result = self.vm.qmp(...) self.assert_qmp(result, 'return', {}) (and some similar ones) into self.vm.cmd(...) Used in the next commit "python: use vm.cmd() instead of vm.qmp() where appropriate" Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Eric Blake Message-id: 20231006154125.1068348-15-vsement...@yandex-team.ru Signed-off-by: John Snow --- scripts/python_qmp_updater.py | 136 ++ 1 file changed, 136 insertions(+) create mode 100755 scripts/python_qmp_updater.py diff --git a/scripts/python_qmp_updater.py b/scripts/python_qmp_updater.py new file mode 100755 index 00..494a169812 --- /dev/null +++ b/scripts/python_qmp_updater.py @@ -0,0 +1,136 @@ +#!/usr/bin/env python3 +# +# Intended usage: +# +# git grep -l '\.qmp(' | xargs ./scripts/python_qmp_updater.py +# + +import re +import sys +from typing import Optional + +start_reg = re.compile(r'^(?P *)(?P\w+) = (?P.*).qmp\(', + flags=re.MULTILINE) + +success_reg_templ = re.sub('\n *', '', r""" +(\n*{padding}(?P\#.*$))? +\n*{padding} +( +self.assert_qmp\({res},\ 'return',\ {{}}\) +| +assert\ {res}\['return'\]\ ==\ {{}} +| +assert\ {res}\ ==\ {{'return':\ {{ +| +self.assertEqual\({res}\['return'\],\ {{}}\) +)""") + +some_check_templ = re.sub('\n *', '', r""" +(\n*{padding}(?P\#.*$))? +\s*self.assert_qmp\({res},""") + + +def tmatch(template: str, text: str, + padding: str, res: str) -> Optional[re.Match[str]]: +return re.match(template.format(padding=padding, res=res), text, +flags=re.MULTILINE) + + +def find_closing_brace(text: str, start: int) -> int: +""" +Having '(' at text[start] search for pairing ')' and return its index. +""" +assert text[start] == '(' + +height = 1 + +for i in range(start + 1, len(text)): +if text[i] == '(': +height += 1 +elif text[i] == ')': +height -= 1 +if height == 0: +return i + +raise ValueError + + +def update(text: str) -> str: +result = '' + +while True: +m = start_reg.search(text) +if m is None: +result += text +break + +result += text[:m.start()] + +args_ind = m.end() +args_end = find_closing_brace(text, args_ind - 1) + +all_args = text[args_ind:args_end].split(',', 1) + +name = all_args[0] +args = None if len(all_args) == 1 else all_args[1] + +unchanged_call = text[m.start():args_end+1] +text = text[args_end+1:] + +padding, res, vm = m.group('padding', 'res', 'vm') + +m = tmatch(success_reg_templ, text, padding, res) + +if m is None: +result += unchanged_call + +if ('query-' not in name and +'x-debug-block-dirty-bitmap-sha256' not in name and +not tmatch(some_check_templ, text, padding, res)): +print(unchanged_call + text[:200] + '...\n\n') + +continue + +if m.group('comment'): +result += f'{padding}{m.group("comment")}\n' + +result += f'{padding}{vm}.cmd({name}' + +if args: +result += ',' + +if '\n' in args: +m_args = re.search('(?P *).*$', args) +assert m_args is not None + +cur_padding = len(m_args.group('pad')) +expected = len(f'{padding}{res} = {vm}.qmp(') +drop = len(f'{res} = ') +if cur_padding == expected - 1: +# tolerate this bad style +drop -= 1 +elif cur_padding < expected - 1: +# assume nothing to do +drop = 0 + +if drop: +args = re.sub('\n' + ' ' * drop, '\n', args) + +result += args + +result += ')' + +text = text[m.end():] + +return result + + +for fname in sys.argv[1:]: +print(fname) +with open(fname) as f: +t = f.read() + +t = update(t) + +with open(fname, 'w') as f: +f.write(t) -- 2.41.0
[PULL 02/25] python/machine: close sock_pair in cleanup path
If everything has gone smoothly, we'll already have closed the socket we gave to the child during post_launch. The other half of the pair that we gave to the QMP connection should, likewise, be definitively closed by now. However, in the cleanup path, it's possible we've created the socketpair but flubbed the launch and need to clean up resources. These resources *would* be handled by the garbage collector, but that can happen at unpredictable times. Nicer to just clean them up synchronously on the exit path, here. Signed-off-by: John Snow Reviewed-by: Ani Sinha Reviewed-by: Daniel P. Berrangé Message-id: 20230928044943.849073-3-js...@redhat.com Signed-off-by: John Snow --- python/qemu/machine/machine.py | 5 + 1 file changed, 5 insertions(+) diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py index 345610d6e4..e26109e6f0 100644 --- a/python/qemu/machine/machine.py +++ b/python/qemu/machine/machine.py @@ -396,6 +396,11 @@ def _post_shutdown(self) -> None: finally: assert self._qmp_connection is None +if self._sock_pair: +self._sock_pair[0].close() +self._sock_pair[1].close() +self._sock_pair = None + self._close_qemu_log_file() self._load_io_log() -- 2.41.0
[PULL 06/25] python/machine: remove unused sock_dir argument
By using a socketpair for all of the sockets managed by the VM class and its extensions, we don't need the sock_dir argument anymore, so remove it. We only added this argument so that we could specify a second, shorter temporary directory for cases where the temp/log dirs were "too long" as a socket name on macOS. We don't need it for this class now. In one case, avocado testing takes over responsibility for creating an appropriate sockdir. Signed-off-by: John Snow Reviewed-by: Daniel P. Berrangé Message-id: 20230928044943.849073-7-js...@redhat.com Signed-off-by: John Snow --- python/qemu/machine/machine.py | 18 -- python/qemu/machine/qtest.py | 5 + tests/avocado/acpi-bits.py | 5 + tests/avocado/avocado_qemu/__init__.py | 2 +- tests/avocado/machine_aspeed.py| 5 - tests/qemu-iotests/iotests.py | 2 +- tests/qemu-iotests/tests/copy-before-write | 3 +-- 7 files changed, 9 insertions(+), 31 deletions(-) diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py index 4156b8cf7d..d539e91268 100644 --- a/python/qemu/machine/machine.py +++ b/python/qemu/machine/machine.py @@ -127,7 +127,6 @@ def __init__(self, name: Optional[str] = None, base_temp_dir: str = "/var/tmp", monitor_address: Optional[SocketAddrT] = None, - sock_dir: Optional[str] = None, drain_console: bool = False, console_log: Optional[str] = None, log_dir: Optional[str] = None, @@ -141,7 +140,6 @@ def __init__(self, @param name: prefix for socket and log file names (default: qemu-PID) @param base_temp_dir: default location where temp files are created @param monitor_address: address for QMP monitor -@param sock_dir: where to create socket (defaults to base_temp_dir) @param drain_console: (optional) True to drain console socket to buffer @param console_log: (optional) path to console log file @param log_dir: where to create and keep log files @@ -163,7 +161,6 @@ def __init__(self, Tuple[socket.socket, socket.socket]] = None self._temp_dir: Optional[str] = None self._base_temp_dir = base_temp_dir -self._sock_dir = sock_dir self._log_dir = log_dir self._monitor_address = monitor_address @@ -189,9 +186,6 @@ def __init__(self, self._console_index = 0 self._console_set = False self._console_device_type: Optional[str] = None -self._console_address = os.path.join( -self.sock_dir, f"{self._name}.con" -) self._console_socket: Optional[socket.socket] = None self._console_file: Optional[socket.SocketIO] = None self._remove_files: List[str] = [] @@ -335,9 +329,6 @@ def args(self) -> List[str]: return self._args def _pre_launch(self) -> None: -if self._console_set: -self._remove_files.append(self._console_address) - if self._qmp_set: if self._monitor_address is None: self._sock_pair = socket.socketpair() @@ -937,15 +928,6 @@ def temp_dir(self) -> str: dir=self._base_temp_dir) return self._temp_dir -@property -def sock_dir(self) -> str: -""" -Returns the directory used for sockfiles by this machine. -""" -if self._sock_dir: -return self._sock_dir -return self.temp_dir - @property def log_dir(self) -> str: """ diff --git a/python/qemu/machine/qtest.py b/python/qemu/machine/qtest.py index 8180d3ab01..4f5ede85b2 100644 --- a/python/qemu/machine/qtest.py +++ b/python/qemu/machine/qtest.py @@ -135,17 +135,14 @@ def __init__(self, wrapper: Sequence[str] = (), name: Optional[str] = None, base_temp_dir: str = "/var/tmp", - sock_dir: Optional[str] = None, qmp_timer: Optional[float] = None): # pylint: disable=too-many-arguments if name is None: name = "qemu-%d" % os.getpid() -if sock_dir is None: -sock_dir = base_temp_dir super().__init__(binary, args, wrapper=wrapper, name=name, base_temp_dir=base_temp_dir, - sock_dir=sock_dir, qmp_timer=qmp_timer) + qmp_timer=qmp_timer) self._qtest: Optional[QEMUQtestProtocol] = None self._qtest_sock_pair: Optional[ Tuple[socket.socket, socket.socket]] = None diff --git a/tests/avocado/acpi-bits.py b/tests/avocado/acpi-bits.py index bb3f818689..eca13dc518 100644 --- a/tests/avocado/acpi-bits.py +++ b/tests/avocado/acpi-bits.py @@ -92,17 +92,14 @@ def __init__(self, base_temp_dir: str = "/var/tmp",
[PULL 00/25] Python patches
The following changes since commit a51e5124a655b3dad80b36b18547cb1eca2c5eb2: Merge tag 'pull-omnibus-111023-1' of https://gitlab.com/stsquad/qemu into staging (2023-10-11 09:43:10 -0400) are available in the Git repository at: https://gitlab.com/jsnow/qemu.git tags/python-pull-request for you to fetch changes up to b6aed193e5ecca32bb07e062f58f0daca06e7009: python: use vm.cmd() instead of vm.qmp() where appropriate (2023-10-12 14:21:44 -0400) Python Pullreq Python PR: - Use socketpair for all machine.py connections - Support Python 3.12 - Switch iotests over to using raise-on-error QMP command interface (Thank you very much, Vladimir!) John Snow (10): python/machine: move socket setup out of _base_args property python/machine: close sock_pair in cleanup path python/console_socket: accept existing FD in initializer python/machine: use socketpair() for console connections python/machine: use socketpair() for qtest connection python/machine: remove unused sock_dir argument Python/iotests: Add type hint for nbd module python/qmp: remove Server.wait_closed() call for Python 3.12 configure: fix error message to say Python 3.8 Python: Enable python3.12 support Vladimir Sementsov-Ogievskiy (15): python/qemu/qmp/legacy: cmd(): drop cmd_id unused argument qmp_shell.py: _fill_completion() use .command() instead of .cmd() scripts/cpu-x86-uarch-abi.py: use .command() instead of .cmd() python: rename QEMUMonitorProtocol.cmd() to cmd_raw() python/qemu: rename command() to cmd() python/machine.py: upgrade vm.cmd() method iotests: QemuStorageDaemon: add cmd() method like in QEMUMachine. iotests: add some missed checks of qmp result iotests: refactor some common qmp result checks into generic pattern iotests: drop some extra semicolons iotests: drop some extra ** in qmp() call iotests.py: pause_job(): drop return value tests/vm/basevm.py: use cmd() instead of qmp() scripts: add python_qmp_updater.py python: use vm.cmd() instead of vm.qmp() where appropriate docs/devel/testing.rst| 10 +- configure | 5 +- python/qemu/machine/console_socket.py | 29 +- python/qemu/machine/machine.py| 78 +-- python/qemu/machine/qtest.py | 54 +- python/qemu/qmp/legacy.py | 10 +- python/qemu/qmp/protocol.py | 1 - python/qemu/qmp/qmp_shell.py | 20 +- python/qemu/utils/qemu_ga_client.py | 2 +- python/qemu/utils/qom.py | 8 +- python/qemu/utils/qom_common.py | 2 +- python/qemu/utils/qom_fuse.py | 6 +- python/setup.cfg | 3 +- scripts/cpu-x86-uarch-abi.py | 8 +- scripts/device-crash-test | 8 +- scripts/python_qmp_updater.py | 136 + scripts/render_block_graph.py | 8 +- tests/avocado/acpi-bits.py| 5 +- tests/avocado/avocado_qemu/__init__.py| 6 +- tests/avocado/cpu_queries.py | 5 +- tests/avocado/hotplug_cpu.py | 10 +- tests/avocado/info_usernet.py | 4 +- tests/avocado/machine_arm_integratorcp.py | 6 +- tests/avocado/machine_aspeed.py | 17 +- tests/avocado/machine_m68k_nextcube.py| 4 +- tests/avocado/machine_mips_malta.py | 6 +- tests/avocado/machine_s390_ccw_virtio.py | 28 +- tests/avocado/migration.py| 10 +- tests/avocado/pc_cpu_hotplug_props.py | 2 +- tests/avocado/version.py | 4 +- tests/avocado/virtio_check_params.py | 6 +- tests/avocado/virtio_version.py | 5 +- tests/avocado/vnc.py | 16 +- tests/avocado/x86_cpu_model_versions.py | 13 +- tests/docker/dockerfiles/python.docker| 6 +- tests/migration/guestperf/engine.py | 144 +++--- tests/qemu-iotests/030| 168 +++ tests/qemu-iotests/040| 171 +++ tests/qemu-iotests/041| 470 -- tests/qemu-iotests/045| 15 +- tests/qemu-iotests/055| 62 +-- tests/qemu-iotests/056| 77 ++- tests/qemu-iotests/093| 42 +- tests/qemu-iotests/118| 223 - tests/qemu-iotests/124| 102 ++-- tests/qemu-iotests/129| 14 +- tests/qemu-iotests/132| 5 +- tests/qemu-iotests/139| 45 +- tests/qemu-iotests/147| 30 +- tests/qemu-iotests/151
[PATCH 1/2] block: Fix locking in media change monitor commands
blk_insert_bs() requires that the caller holds the AioContext lock for the node to be inserted. Since commit c066e808e11, neglecting to do so causes a crash when the child has to be moved to a different AioContext to attach it to the BlockBackend. This fixes qmp_blockdev_insert_anon_medium(), which is called for the QMP commands 'blockdev-insert-medium' and 'blockdev-change-medium', to correctly take the lock. Cc: qemu-sta...@nongnu.org Fixes: https://issues.redhat.com/browse/RHEL-3922 Fixes: c066e808e11a5c181b625537b6c78e0de27a4801 Signed-off-by: Kevin Wolf --- block/qapi-sysemu.c | 5 + 1 file changed, 5 insertions(+) diff --git a/block/qapi-sysemu.c b/block/qapi-sysemu.c index 3f614cbc04..1618cd225a 100644 --- a/block/qapi-sysemu.c +++ b/block/qapi-sysemu.c @@ -237,6 +237,7 @@ static void qmp_blockdev_insert_anon_medium(BlockBackend *blk, BlockDriverState *bs, Error **errp) { Error *local_err = NULL; +AioContext *ctx; bool has_device; int ret; @@ -258,7 +259,11 @@ static void qmp_blockdev_insert_anon_medium(BlockBackend *blk, return; } +ctx = bdrv_get_aio_context(bs); +aio_context_acquire(ctx); ret = blk_insert_bs(blk, bs, errp); +aio_context_release(ctx); + if (ret < 0) { return; } -- 2.41.0
[PATCH 2/2] iotests: Test media change with iothreads
iotests case 118 already tests all relevant operations for media change with multiple devices, however never with iothreads. This changes the test so that the virtio-scsi tests run with an iothread. Signed-off-by: Kevin Wolf --- tests/qemu-iotests/118 | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/qemu-iotests/118 b/tests/qemu-iotests/118 index cae52ffa5e..bc7533bb54 100755 --- a/tests/qemu-iotests/118 +++ b/tests/qemu-iotests/118 @@ -295,7 +295,8 @@ class TestInitiallyFilled(GeneralChangeTestsBaseClass): 'file.driver=file', 'file.filename=%s' % old_img ]) if self.interface == 'scsi': -self.vm.add_device('virtio-scsi-pci') +self.vm.add_object('iothread,id=iothread0') +self.vm.add_device('virtio-scsi-pci,iothread=iothread0') self.vm.add_device('%s,drive=drive0,id=%s' % (interface_to_device_name(self.interface), self.device_name)) @@ -332,7 +333,8 @@ class TestInitiallyEmpty(GeneralChangeTestsBaseClass): if self.use_drive: self.vm.add_drive(None, 'media=%s' % self.media, 'none') if self.interface == 'scsi': -self.vm.add_device('virtio-scsi-pci') +self.vm.add_object('iothread,id=iothread0') +self.vm.add_device('virtio-scsi-pci,iothread=iothread0') self.vm.add_device('%s,%sid=%s' % (interface_to_device_name(self.interface), 'drive=drive0,' if self.use_drive else '', -- 2.41.0
[PATCH 0/2] block: Fix locking in media change monitor commands
Kevin Wolf (2): block: Fix locking in media change monitor commands iotests: Test media change with iothreads block/qapi-sysemu.c| 5 + tests/qemu-iotests/118 | 6 -- 2 files changed, 9 insertions(+), 2 deletions(-) -- 2.41.0
Re: [PATCH v4 04/10] migration: migrate 'blk' command option is deprecated.
Juan Quintela writes: > Set the 'block' migration capability to 'true' instead. > > Signed-off-by: Juan Quintela > Acked-by: Stefan Hajnoczi > Reviewed-by: Thomas Huth > > --- > > Improve documentation and style (markus) > --- > docs/about/deprecated.rst | 7 +++ > qapi/migration.json | 6 -- > migration/migration.c | 5 + > 3 files changed, 16 insertions(+), 2 deletions(-) > > diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst > index 1b6b2870cf..e6928b96cf 100644 > --- a/docs/about/deprecated.rst > +++ b/docs/about/deprecated.rst > @@ -459,3 +459,10 @@ The new way to modify migration is using migration > parameters. > ``inc`` functionality can be achieved by setting the > ``block-incremental`` migration parameter to ``true``. > > +``blk`` migrate command option (since 8.2) > +'' > + > +The new way to modify migration is using migration parameters. > +``blk`` functionality can be achieved by setting the > +``block`` migration capability to ``true``. > + > diff --git a/qapi/migration.json b/qapi/migration.json > index 56bbd55b87..64ebced761 100644 > --- a/qapi/migration.json > +++ b/qapi/migration.json > @@ -1495,7 +1495,8 @@ > # Features: > # > # @deprecated: Member @inc is deprecated. Use migration parameter > -# @block-incremental instead. > +# @block-incremental instead. Member @blk is deprecated. Set Two spaces between sentences for consistency. > +# migration capability 'block' to 'true' instead. > # > # Returns: nothing on success > # > @@ -1518,7 +1519,8 @@ > # <- { "return": {} } > ## > { 'command': 'migrate', > - 'data': {'uri': 'str', '*blk': 'bool', > + 'data': {'uri': 'str', > + '*blk': { 'type': 'bool', 'features': [ 'deprecated' ] }, > '*inc': { 'type': 'bool', 'features': [ 'deprecated' ] }, > '*detach': 'bool', '*resume': 'bool' } } > > diff --git a/migration/migration.c b/migration/migration.c > index ac4897fe0d..9e4ae6b772 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -1607,6 +1607,11 @@ static bool migrate_prepare(MigrationState *s, bool > blk, bool blk_inc, > " instead."); > } > > +if (blk) { > +warn_report("@blk/-i migrate option is deprecated, set the " > +"'block' capability to 'true' instead."); > +} > + > if (resume) { > if (s->state != MIGRATION_STATUS_POSTCOPY_PAUSED) { > error_setg(errp, "Cannot resume if there is no " My comments for PATCH 01 apply.
Re: [PATCH v4 03/10] migration: migrate 'inc' command option is deprecated.
Juan Quintela writes: > Set the 'block_incremental' migration parameter to 'true' instead. > > Reviewed-by: Thomas Huth > Acked-by: Stefan Hajnoczi > Signed-off-by: Juan Quintela > > --- > > Improve documentation and style (thanks Markus) > --- > docs/about/deprecated.rst | 7 +++ > qapi/migration.json | 8 +++- > migration/migration.c | 6 ++ > 3 files changed, 20 insertions(+), 1 deletion(-) > > diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst > index 1c4d7f36f0..1b6b2870cf 100644 > --- a/docs/about/deprecated.rst > +++ b/docs/about/deprecated.rst > @@ -452,3 +452,10 @@ Migration > ``skipped`` field in Migration stats has been deprecated. It hasn't > been used for more than 10 years. > > +``inc`` migrate command option (since 8.2) > +'' > + > +The new way to modify migration is using migration parameters. > +``inc`` functionality can be achieved by setting the > +``block-incremental`` migration parameter to ``true``. > + > diff --git a/qapi/migration.json b/qapi/migration.json > index 6865fea3c5..56bbd55b87 100644 > --- a/qapi/migration.json > +++ b/qapi/migration.json > @@ -1492,6 +1492,11 @@ > # > # @resume: resume one paused migration, default "off". (since 3.0) > # > +# Features: > +# > +# @deprecated: Member @inc is deprecated. Use migration parameter > +# @block-incremental instead. This is fine now. It becomes bad advice in PATCH 05, which deprecates @block-incremental. Two solutions: 1. Change this patch to point to an alternative that will *not* be deprecated. 2. Change PATCH 05. Same end result. > +# > # Returns: nothing on success > # > # Since: 0.14 > @@ -1513,7 +1518,8 @@ > # <- { "return": {} } > ## > { 'command': 'migrate', > - 'data': {'uri': 'str', '*blk': 'bool', '*inc': 'bool', > + 'data': {'uri': 'str', '*blk': 'bool', > + '*inc': { 'type': 'bool', 'features': [ 'deprecated' ] }, > '*detach': 'bool', '*resume': 'bool' } } > > ## > diff --git a/migration/migration.c b/migration/migration.c > index 1c6c81ad49..ac4897fe0d 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -1601,6 +1601,12 @@ static bool migrate_prepare(MigrationState *s, bool > blk, bool blk_inc, > { > Error *local_err = NULL; > > +if (blk_inc) { > +warn_report("@inc/-i migrate option is deprecated, set the" This is either about QMP migrate's parameter "inc", or HMP migrate's flags -i. In the former case, we want something like "parameter 'inc' is deprecated". In the latter case, we want something like "-i is deprecated". Trying to do both in a single message results in a sub-par message. If you want to do better, you have to check in hmp_migrate(), too. Then hmp_migrate can refer to "-i", and migrate_prepare() to "parameter 'inc'". Up to you. If you decide a single message is good enough, use something like "parameter 'inc' / flag -i is deprecated". > +"'block-incremental' migration parameter to 'true'" > +" instead."); Again, fine now, becomes bad advice in PATCH 05. > +} > + > if (resume) { > if (s->state != MIGRATION_STATUS_POSTCOPY_PAUSED) { > error_setg(errp, "Cannot resume if there is no "
Re: [RFC PATCH 00/78] Strict disable implicit fallthrough
On Fri, Oct 13, 2023 at 03:51:22PM +0300, Manos Pitsidianakis wrote: > On Fri, 13 Oct 2023 11:14, "Daniel P. Berrangé" wrote: > > On Fri, Oct 13, 2023 at 10:47:04AM +0300, Emmanouil Pitsidianakis wrote: > > > > > > Main questions this RFC poses > > > = > > > > > > - Is this change desirable and net-positive. > > > > Yes, IMHO it is worth standardizing on use of the attribute. The allowed > > use of comments was a nice thing by the compiler for coping with > > pre-existing > > code, but using the attribute is best long term for a consistent style. > > > > > - Should the `fallthrough;` pseudo-keyword be defined like in the Linux > > > kernel, or use glib's G_GNUC_FALLTHROUGH, or keep the already existing > > > QEMU_FALLTHROUGH macro. > > > > As a general rule, if glib provides functionality we aim o use that > > and not reinvent the wheel. IOW, we should just use G_GNUC_FALLTHROUGH. > > I agree. My reasoning was: > > - The reinvented wheel is only an attribute and not a big bunch of NIH code It isn't just about the amount of code, it is the familiarity of the code. QEMU's codebase is glib based, by using glib functionality we leverage developers' general familiarity with / knowledge of glib, as opposed to custom QEMU approaches which need learning. > - The macro def in glib depends on the glib version you use If we need to depend on something that is newer than our min glib version, then we add back compat definitino to 'include/glib-compat.h' for the older version. > - G_GNUC_FALLTHROUGH looks kind of abrasive to my eye, while `fallthrough` > blends in with other switch keywords like break. My impression seeing this series, was exactly the opposite. I did not like 'fallthrough' blending in with the rest of code, and also did not like that it is a macro in lowercase, thus hiding that it is a macro > - C23 standardises fallthrough. We might not ever support C23 but it's good > to be consistent with standards and other, larger projects (linux kernel). We're at least a decade away from being able to use anything from C23. With 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: [RFC PATCH 00/78] Strict disable implicit fallthrough
On Fri, 13 Oct 2023 11:14, "Daniel P. Berrangé" wrote: On Fri, Oct 13, 2023 at 10:47:04AM +0300, Emmanouil Pitsidianakis wrote: Main questions this RFC poses = - Is this change desirable and net-positive. Yes, IMHO it is worth standardizing on use of the attribute. The allowed use of comments was a nice thing by the compiler for coping with pre-existing code, but using the attribute is best long term for a consistent style. - Should the `fallthrough;` pseudo-keyword be defined like in the Linux kernel, or use glib's G_GNUC_FALLTHROUGH, or keep the already existing QEMU_FALLTHROUGH macro. As a general rule, if glib provides functionality we aim o use that and not reinvent the wheel. IOW, we should just use G_GNUC_FALLTHROUGH. I agree. My reasoning was: - The reinvented wheel is only an attribute and not a big bunch of NIH code - The macro def in glib depends on the glib version you use - G_GNUC_FALLTHROUGH looks kind of abrasive to my eye, while `fallthrough` blends in with other switch keywords like break. - C23 standardises fallthrough. We might not ever support C23 but it's good to be consistent with standards and other, larger projects (linux kernel). I think these (except for myself finding G_GNUC_FALLTHROUGH ugly) make a strong case for not using the glib macro, personally. I'd be interested to know if there is a counterpoint to it: because I don't want this change to cause problems in the future. Manos
Re: [PATCH v4 01/10] migration: Improve json and formatting
Juan Quintela writes: > Signed-off-by: Juan Quintela > --- > qapi/migration.json | 7 +++ > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/qapi/migration.json b/qapi/migration.json > index d7dfaa5db9..6865fea3c5 100644 > --- a/qapi/migration.json > +++ b/qapi/migration.json > @@ -73,7 +73,7 @@ > { 'struct': 'MigrationStats', >'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' , > 'duplicate': 'int', > - 'skipped': { 'type': 'int', 'features': ['deprecated'] }, > + 'skipped': { 'type': 'int', 'features': [ 'deprecated' ] }, > 'normal': 'int', > 'normal-bytes': 'int', 'dirty-pages-rate': 'int', > 'mbps': 'number', 'dirty-sync-count': 'int', > @@ -440,10 +440,9 @@ > # compress and xbzrle are both on, compress only takes effect in > # the ram bulk stage, after that, it will be disabled and only > # xbzrle takes effect, this can help to minimize migration > -# traffic. The feature is disabled by default. (since 2.4 ) > +# traffic. The feature is disabled by default. (since 2.4) > # > -# @events: generate events for each migration state change (since 2.4 > -# ) > +# @events: generate events for each migration state change (since 2.4) > # > # @auto-converge: If enabled, QEMU will automatically throttle down > # the guest to speed up convergence of RAM migration. (since 1.6) Reviewed-by: Markus Armbruster
Re: [RFC PATCH 01/78] include/qemu/compiler.h: replace QEMU_FALLTHROUGH with fallthrough
Hello Markus, On Fri, 13 Oct 2023 15:28, Markus Armbruster wrote: The commit message needs to explain why. Certainly. This is wrong. docs/devel/style.rst: Include directives -- Order include directives as follows: .. code-block:: c #include "qemu/osdep.h" /* Always first... */ #include <...> /* then system headers... */ #include "..." /* and finally QEMU headers. */ Separate patch, please. I know. spa headers use the `fallthrough` attribute and qemu/compiler.h defines it as a macro, so it breaks compilation. If the spa headers go after, we'd need to undef fallthrough before including them and re-define or re-include qemu/compiler.h after. What do you think would be best? diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h index 1109482a00..959982805d 100644 --- a/include/qemu/compiler.h +++ b/include/qemu/compiler.h @@ -1,215 +1,231 @@ [...] #define QEMU_ALWAYS_INLINE #endif -/** - * In most cases, normal "fallthrough" comments are good enough for - * switch-case statements, but sometimes the compiler has problems - * with those. In that case you can use QEMU_FALLTHROUGH instead. +/* + * Add the pseudo keyword 'fallthrough' so case statement blocks Pseudo-keyword? It's a macro. C calls reserved words that you cannot redefine 'keywords'. Like 'break', 'continue', 'return'. Hence it's a pseudo-keyword. It's also a macro. They are not mutually exclusive. I did not write this, it was taken verbatim from the linux kernel source, see: /include/linux/compiler_attributes.h + * must end with any of these keywords: + * break; + * fallthrough; + * continue; + * goto ; + * return [expression]; These are statements, not keywords. I'm pretty sure it refers to {break, fallthrough, continue, goto, return} by themselves. + * + * gcc: https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html#Statement-Attributes Not sure we need to point to the docs here. We have hundreds of __attribute__ uses in the tree. Again, copied from /include/linux/compiler_attributes.h */ -#if __has_attribute(fallthrough) -# define QEMU_FALLTHROUGH __attribute__((fallthrough)) + +/* + * glib_macros.h contains its own definition of fallthrough, so if we define + * the pseudokeyword here it will expand when the glib header checks for the + * attribute. glib headers must be #included after this header. + */ +#ifdef fallthrough +#undef fallthrough +#endif Why do we need to roll our own macro then? Glib uses a different name. The problem is when it checks for the compiler attribute, which expands into our macro. -- Manos
Re: [RFC PATCH 01/78] include/qemu/compiler.h: replace QEMU_FALLTHROUGH with fallthrough
The commit message needs to explain why. Emmanouil Pitsidianakis writes: > Signed-off-by: Emmanouil Pitsidianakis > --- > audio/pwaudio.c | 8 > hw/arm/smmuv3.c | 2 +- > include/qemu/compiler.h | 30 +++--- > include/qemu/osdep.h | 4 ++-- > target/loongarch/cpu.c | 4 ++-- > target/loongarch/translate.c | 2 +- > tcg/optimize.c | 8 > 7 files changed, 37 insertions(+), 21 deletions(-) > > diff --git a/audio/pwaudio.c b/audio/pwaudio.c > index 3ce5f6507b..bf26fadb06 100644 > --- a/audio/pwaudio.c > +++ b/audio/pwaudio.c > @@ -1,29 +1,29 @@ > /* > * QEMU PipeWire audio driver > * > * Copyright (c) 2023 Red Hat Inc. > * > * Author: Dorinda Bassey > * > * SPDX-License-Identifier: GPL-2.0-or-later > */ > > +#include > +#include > +#include > +#include > #include "qemu/osdep.h" > #include "qemu/module.h" > #include "audio.h" > #include > #include "qemu/error-report.h" > #include "qapi/error.h" > -#include > -#include > -#include > -#include > > #include > #include "trace.h" > > #define AUDIO_CAP "pipewire" > #define RINGBUFFER_SIZE(1u << 22) > #define RINGBUFFER_MASK(RINGBUFFER_SIZE - 1) > > #include "audio_int.h" This is wrong. docs/devel/style.rst: Include directives -- Order include directives as follows: .. code-block:: c #include "qemu/osdep.h" /* Always first... */ #include <...> /* then system headers... */ #include "..." /* and finally QEMU headers. */ Separate patch, please. [...] > diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h > index 1109482a00..959982805d 100644 > --- a/include/qemu/compiler.h > +++ b/include/qemu/compiler.h > @@ -1,215 +1,231 @@ [...] > #define QEMU_ALWAYS_INLINE > #endif > > -/** > - * In most cases, normal "fallthrough" comments are good enough for > - * switch-case statements, but sometimes the compiler has problems > - * with those. In that case you can use QEMU_FALLTHROUGH instead. > +/* > + * Add the pseudo keyword 'fallthrough' so case statement blocks Pseudo-keyword? It's a macro. > + * must end with any of these keywords: > + * break; > + * fallthrough; > + * continue; > + * goto ; > + * return [expression]; These are statements, not keywords. > + * > + * gcc: > https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html#Statement-Attributes Not sure we need to point to the docs here. We have hundreds of __attribute__ uses in the tree. > */ > -#if __has_attribute(fallthrough) > -# define QEMU_FALLTHROUGH __attribute__((fallthrough)) > + > +/* > + * glib_macros.h contains its own definition of fallthrough, so if we define > + * the pseudokeyword here it will expand when the glib header checks for the > + * attribute. glib headers must be #included after this header. > + */ > +#ifdef fallthrough > +#undef fallthrough > +#endif Why do we need to roll our own macro then? > + > +#if __has_attribute(__fallthrough__) > +# define fallthrough__attribute__((__fallthrough__)) > #else > -# define QEMU_FALLTHROUGH do {} while (0) /* fallthrough */ > +# define fallthroughdo {} while (0) /* fallthrough */ > #endif > > #ifdef CONFIG_CFI > /* [...] > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h > index 475a1c62ff..8f790f0deb 100644 > --- a/include/qemu/osdep.h > +++ b/include/qemu/osdep.h > @@ -1,171 +1,171 @@ > /* > * OS includes and handling of OS dependencies > * > * This header exists to pull in some common system headers that > * most code in QEMU will want, and to fix up some possible issues with > * it (missing defines, Windows weirdness, and so on). > * > * To avoid getting into possible circular include dependencies, this > * file should not include any other QEMU headers, with the exceptions > * of config-host.h, config-target.h, qemu/compiler.h, > * sysemu/os-posix.h, sysemu/os-win32.h, glib-compat.h and > * qemu/typedefs.h, all of which are doing a similar job to this file > * and are under similar constraints. > * > * This header also contains prototypes for functions defined in > * os-*.c and util/oslib-*.c; those would probably be better split > * out into separate header files. > * > * In an ideal world this header would contain only: > * (1) things which everybody needs > * (2) things without which code would work on most platforms but > * fail to compile or misbehave on a minority of host OSes > * > * This work is licensed under the terms of the GNU GPL, version 2 or later. > * See the COPYING file in the top-level directory. > */ > #ifndef QEMU_OSDEP_H > #define QEMU_OSDEP_H > > #if !defined _FORTIFY_SOURCE && defined __OPTIMIZE__ && __OPTIMIZE__ && > defined __linux__ > # define _FORTIFY_SOURCE 2 > #endif > > #include "config-host.h" > #
Re: [RFC PATCH 00/78] Strict disable implicit fallthrough
On Fri, 13 Oct 2023, Emmanouil Pitsidianakis wrote: Hello, This RFC is inspired by the kernel's move to -Wimplicit-fallthrough=3 back in 2019.[0] We take one step (or two) further by increasing it to 5 which rejects fall through comments and requires an attribute statement. [0]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a035d552a93b The line differences are not many, but they spread all over different subsystems, architectures and devices. An attempt has been made to split them in cohesive patches to aid post-RFC review. Part of the RFC is to determine whether these patch divisions needs improvement. Main questions this RFC poses = - Is this change desirable and net-positive. IMO current fall through comments are OK. This new way does not improve it much because it adds one more peculiarity new people have to get used to. Adding a fall through comment when one gets a warning is something one easily can figure out but finding out there's a macro for that would need consulting docs. - Should the `fallthrough;` pseudo-keyword be defined like in the Linux kernel, or use glib's G_GNUC_FALLTHROUGH, or keep the already existing QEMU_FALLTHROUGH macro. If there'w already QEMU_FALTHROUGH why not use that? Looks more obvious than a macro that looks like a strange function or keyword. Then a check could be added to checkpatch.pl to tell people to used QEMU_FALLTHROUGH istead of a fall through comment (matching the same regexp gcc accepts as others will already be checked by gcc) to enforce its usage. - Should fallthrough comments be removed if they do not include extra information. If this change is considered useful and not just code churn then replace comments, don't leave both as then the comment do not add any info. Some external resources === See the RFC discussion in the kernel: https://lore.kernel.org/lkml/1d2830aadbe9d8151728a7df5b88528fc72a0095.1564549413.git@perches.com/ The `fallthrough;` pseudo-keyword in the kernel source code: https://elixir.bootlin.com/linux/latest/C/ident/fallthrough In summary, I quote the doc comment and definition: /* * Add the pseudo keyword 'fallthrough' so case statement blocks * must end with any of these keywords: * break; * fallthrough; * continue; * goto ; * return [expression]; * * gcc: https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html#Statement-Attributes */ #if __has_attribute(__fallthrough__) # define fallthrough__attribute__((__fallthrough__)) #else # define fallthroughdo {} while (0) /* fallthrough */ Is the empty loop needed here? If the compiler does not mind a semicolon after a comment then there could be a semicolon as an empty statement there too. But maybe there are some cases where this would be needed, just curious why it's there in this case? Regards, BALATON Zoltan #endif Background - Motivation === The C switch statement allows you to conditionally goto different labels depending on a value. A break; statement conveniently goto's the end of the switch. If a "case" does not end in a break, we say that the control flow falls through the next case label, if any, implicitly. This can lead to bugs and QEMU uses the GCC warning -Wimplicit-fallthrough to prevent this. Currently, QEMU is built with -Wimplicit-fallthrough=2. This makes GCC's static analyzer check for a case-insensitive matches of the .*falls?[ \t-]*thr(ough|u).* regular expression. This means the following list of comments taken from QEMU all disable the implicit fallthrough warning: - /* FALLTHRU */ - /* fall through */ - /* Fall through. */ - /* Fall through... */ - /* fall through if hEventTimeout is signaled */ - /* FALL THROUGH */ To keep a constistent code style, this commit adds a macro `fallthrough` that looks like a C keyword but expands to an attribute statement in supported compilers (GCC at the moment). Note: there was already such a macro, QEMU_FALLTHROUGH, and it was used only around 7 times in the code base. The first commit replaces it. Emmanouil Pitsidianakis (78): include/qemu/compiler.h: replace QEMU_FALLTHROUGH with fallthrough block: add fallthrough pseudo-keyword fpu/softfloat: add fallthrough pseudo-keyword qapi/opts-visitor: add fallthrough pseudo-keyword qobject/json: add fallthrough pseudo-keyword tcg: add fallthrough pseudo-keyword hw/virtio/virtio-balloon.c: add fallthrough pseudo-keyword hw/block: add fallthrough pseudo-keyword hw/acpi/aml-build.c: add fallthrough pseudo-keyword hw/ide/atapi.c: add fallthrough pseudo-keyword hw/timer: add fallthrough pseudo-keyword hw/usb: add fallthrough pseudo-keyword hw/adc: add fallthrough pseudo-keyword util/error-report.c: add fallthrough pseudo-keyword accel/tcg: add fallthrough pseudo-keyword audio: add fallthrough pseudo-keyword ui
Re: [PATCH v4] migration: hold the BQL during setup
Am 12.10.23 um 22:40 schrieb Fabiano Rosas: > Fiona Ebner writes: > >> This is intended to be a semantic revert of commit 9b09503752 >> ("migration: run setup callbacks out of big lock"). There have been so >> many changes since that commit (e.g. a new setup callback >> dirty_bitmap_save_setup() that also needs to be adapted now), it's >> easier to do the revert manually. >> >> For snapshots, the bdrv_writev_vmstate() function is used during setup >> (in QIOChannelBlock backing the QEMUFile), but not holding the BQL >> while calling it could lead to an assertion failure. To understand >> how, first note the following: > > Would it make sense to add a GLOBAL_STATE_CODE() annotation to > qio_channel_block_writev? > Since bdrv_writev_vmstate() is IO_OR_GS_CODE(), would using that be better? And I guess if we add an annotation for qio_channel_block_writev(), we should go ahead and also do it for other functions in the file? E.g. qio_channel_block_new() would have to be GLOBAL_STATE_CODE(), because it uses bdrv_ref(). Best Regards, Fiona
[PATCH v5] migration: hold the BQL during setup
This is intended to be a semantic revert of commit 9b09503752 ("migration: run setup callbacks out of big lock"). There have been so many changes since that commit (e.g. a new setup callback dirty_bitmap_save_setup() that also needs to be adapted now), it's easier to do the revert manually. For snapshots, the bdrv_writev_vmstate() function is used during setup (in QIOChannelBlock backing the QEMUFile), but not holding the BQL while calling it could lead to an assertion failure. To understand how, first note the following: 1. Generated coroutine wrappers for block layer functions spawn the coroutine and use AIO_WAIT_WHILE()/aio_poll() to wait for it. 2. If the host OS switches threads at an inconvenient time, it can happen that a bottom half scheduled for the main thread's AioContext is executed as part of a vCPU thread's aio_poll(). An example leading to the assertion failure is as follows: main thread: 1. A snapshot-save QMP command gets issued. 2. snapshot_save_job_bh() is scheduled. vCPU thread: 3. aio_poll() for the main thread's AioContext is called (e.g. when the guest writes to a pflash device, as part of blk_pwrite which is a generated coroutine wrapper). 4. snapshot_save_job_bh() is executed as part of aio_poll(). 3. qemu_savevm_state() is called. 4. qemu_mutex_unlock_iothread() is called. Now qemu_get_current_aio_context() returns 0x0. 5. bdrv_writev_vmstate() is executed during the usual savevm setup via qemu_fflush(). But this function is a generated coroutine wrapper, so it uses AIO_WAIT_WHILE. There, the assertion assert(qemu_get_current_aio_context() == qemu_get_aio_context()); will fail. To fix it, ensure that the BQL is held during setup. While it would only be needed for snapshots, adapting migration too avoids additional logic for conditional locking/unlocking in the setup callbacks. Writing the header could (in theory) also trigger qemu_fflush() and thus bdrv_writev_vmstate(), so the locked section also covers the qemu_savevm_state_header() call, even for migration for consistency. The section around multifd_send_sync_main() needs to be unlocked to avoid a deadlock. In particular, the multifd_save_setup() function calls socket_send_channel_create() using multifd_new_send_channel_async() as a callback and then waits for the callback to signal via the channels_ready semaphore. The connection happens via qio_task_run_in_thread(), but the callback is only executed via qio_task_thread_result() which is scheduled for the main event loop. Without unlocking the section, the main thread would never get to process the task result and the callback meaning there would be no signal via the channels_ready semaphore. The comment in ram_init_bitmaps() was introduced by 4987783400 ("migration: fix incorrect memory_global_dirty_log_start outside BQL") and is removed, because it referred to the qemu_mutex_lock_iothread() call. Signed-off-by: Fiona Ebner Reviewed-by: Fabiano Rosas Reviewed-by: Juan Quintela --- Changes in v5: * Mention which function calls socket_send_channel_create() in commit message to fix the meaning of the paragraph. * Fix typo in commit message (consistentcy -> consistency). Changes in v4: * Rebase on current master (save_prepare handler got added). Changes in v3: * Add unlocked section around multifd_send_sync_main(). Changes in v2: * Also hold the BQL for migration, rather than conditionally acquiring/releasing the lock inside the setup callbacks. include/migration/register.h | 2 +- migration/block-dirty-bitmap.c | 3 --- migration/block.c | 5 - migration/migration.c | 6 ++ migration/ram.c| 6 +++--- migration/savevm.c | 2 -- 6 files changed, 10 insertions(+), 14 deletions(-) diff --git a/include/migration/register.h b/include/migration/register.h index 2b12c6adec..fed1d04a3c 100644 --- a/include/migration/register.h +++ b/include/migration/register.h @@ -25,6 +25,7 @@ typedef struct SaveVMHandlers { * used to perform early checks. */ int (*save_prepare)(void *opaque, Error **errp); +int (*save_setup)(QEMUFile *f, void *opaque); void (*save_cleanup)(void *opaque); int (*save_live_complete_postcopy)(QEMUFile *f, void *opaque); int (*save_live_complete_precopy)(QEMUFile *f, void *opaque); @@ -50,7 +51,6 @@ typedef struct SaveVMHandlers { int (*save_live_iterate)(QEMUFile *f, void *opaque); /* This runs outside the iothread lock! */ -int (*save_setup)(QEMUFile *f, void *opaque); /* Note for save_live_pending: * must_precopy: * - must be migrated in precopy or in stopped state diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c index 032fc5f405..03cb2e72ee 100644 --- a/migration/block-dirty-bitmap.c +++ b/migration/block-dirty-bitmap.c @@ -1214,9 +1214,7 @@ static int dirty_bitmap_save_setup(QEMUFile *f, void *opaque) DBMSaveState *s = &((DBMState *)opaque)->save; Save
[PATCH v4 03/10] migration: migrate 'inc' command option is deprecated.
Set the 'block_incremental' migration parameter to 'true' instead. Reviewed-by: Thomas Huth Acked-by: Stefan Hajnoczi Signed-off-by: Juan Quintela --- Improve documentation and style (thanks Markus) --- docs/about/deprecated.rst | 7 +++ qapi/migration.json | 8 +++- migration/migration.c | 6 ++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 1c4d7f36f0..1b6b2870cf 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -452,3 +452,10 @@ Migration ``skipped`` field in Migration stats has been deprecated. It hasn't been used for more than 10 years. +``inc`` migrate command option (since 8.2) +'' + +The new way to modify migration is using migration parameters. +``inc`` functionality can be achieved by setting the +``block-incremental`` migration parameter to ``true``. + diff --git a/qapi/migration.json b/qapi/migration.json index 6865fea3c5..56bbd55b87 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -1492,6 +1492,11 @@ # # @resume: resume one paused migration, default "off". (since 3.0) # +# Features: +# +# @deprecated: Member @inc is deprecated. Use migration parameter +# @block-incremental instead. +# # Returns: nothing on success # # Since: 0.14 @@ -1513,7 +1518,8 @@ # <- { "return": {} } ## { 'command': 'migrate', - 'data': {'uri': 'str', '*blk': 'bool', '*inc': 'bool', + 'data': {'uri': 'str', '*blk': 'bool', + '*inc': { 'type': 'bool', 'features': [ 'deprecated' ] }, '*detach': 'bool', '*resume': 'bool' } } ## diff --git a/migration/migration.c b/migration/migration.c index 1c6c81ad49..ac4897fe0d 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -1601,6 +1601,12 @@ static bool migrate_prepare(MigrationState *s, bool blk, bool blk_inc, { Error *local_err = NULL; +if (blk_inc) { +warn_report("@inc/-i migrate option is deprecated, set the" +"'block-incremental' migration parameter to 'true'" +" instead."); +} + if (resume) { if (s->state != MIGRATION_STATUS_POSTCOPY_PAUSED) { error_setg(errp, "Cannot resume if there is no " -- 2.41.0
[PATCH v4 06/10] migration: Deprecate old compression method
Signed-off-by: Juan Quintela Acked-by: Stefan Hajnoczi Acked-by: Peter Xu --- docs/about/deprecated.rst | 8 qapi/migration.json | 79 +-- migration/options.c | 13 +++ 3 files changed, 72 insertions(+), 28 deletions(-) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 6490925cdc..a6da0df8e1 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -476,3 +476,11 @@ Please see "QMP invocation for live storage migration with ``blockdev-mirror`` + NBD" in docs/interop/live-block-operations.rst for a detailed explanation. +old compression method (since 8.2) +'' + +Compression method fails too much. Too many races. We are going to +remove it if nobody fixes it. For starters, migration-test +compression tests are disabled becase they fail randomly. If you need +compression, use multifd compression methods. + diff --git a/qapi/migration.json b/qapi/migration.json index 5e545d10c0..e599062f7d 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -272,6 +272,10 @@ # Features: # # @deprecated: Member @disk is deprecated because block migration is. +# Member @compression is deprecated because it is unreliable and +# untested. It is recommended to use multifd migration, which +# offers an alternative compression implementation that is +# reliable and tested. # # Since: 0.14 ## @@ -289,7 +293,7 @@ '*blocked-reasons': ['str'], '*postcopy-blocktime': 'uint32', '*postcopy-vcpu-blocktime': ['uint32'], - '*compression': 'CompressionStats', + '*compression': { 'type': 'CompressionStats', 'features': [ 'deprecated' ] }, '*socket-address': ['SocketAddress'], '*dirty-limit-throttle-time-per-round': 'uint64', '*dirty-limit-ring-full-time': 'uint64'} } @@ -530,7 +534,10 @@ # Features: # # @deprecated: Member @block is deprecated. Use blockdev-mirror with -# NBD instead. +# NBD instead. Member @compression is deprecated because it is +# unreliable and untested. It is recommended to use multifd +# migration, which offers an alternative compression +# implementation that is reliable and tested. # # @unstable: Members @x-colo and @x-ignore-shared are experimental. # @@ -538,7 +545,8 @@ ## { 'enum': 'MigrationCapability', 'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks', - 'compress', 'events', 'postcopy-ram', + { 'name': 'compress', 'features': [ 'deprecated' ] }, + 'events', 'postcopy-ram', { 'name': 'x-colo', 'features': [ 'unstable' ] }, 'release-ram', { 'name': 'block', 'features': [ 'deprecated' ] }, @@ -834,7 +842,9 @@ # Features: # # @deprecated: Member @block-incremental is deprecated. Use -# blockdev-mirror with NBD instead. +# blockdev-mirror with NBD instead. Members @compress-level, +# @compress-threads, @decompress-threads and @compress-wait-thread +# are deprecated because @compression is deprecated. # # @unstable: Members @x-checkpoint-delay and @x-vcpu-dirty-limit-period # are experimental. @@ -844,8 +854,11 @@ { 'enum': 'MigrationParameter', 'data': ['announce-initial', 'announce-max', 'announce-rounds', 'announce-step', - 'compress-level', 'compress-threads', 'decompress-threads', - 'compress-wait-thread', 'throttle-trigger-threshold', + { 'name': 'compress-level', 'features': [ 'deprecated' ] }, + { 'name': 'compress-threads', 'features': [ 'deprecated' ] }, + { 'name': 'decompress-threads', 'features': [ 'deprecated' ] }, + { 'name': 'compress-wait-thread', 'features': [ 'deprecated' ] }, + 'throttle-trigger-threshold', 'cpu-throttle-initial', 'cpu-throttle-increment', 'cpu-throttle-tailslow', 'tls-creds', 'tls-hostname', 'tls-authz', 'max-bandwidth', @@ -875,16 +888,16 @@ # @announce-step: Increase in delay (in milliseconds) between # subsequent packets in the announcement (Since 4.0) # -# @compress-level: compression level +# @compress-level: compression level. # -# @compress-threads: compression thread count +# @compress-threads: compression thread count. # # @compress-wait-thread: Controls behavior when all compression # threads are currently busy. If true (default), wait for a free # compression thread to become available; otherwise, send the page -# uncompressed. (Since 3.1) +# uncompressed. (Since 3.1) # -# @decompress-threads: decompression thread count +# @decompress-threads: decompression thread count. # # @throttle-trigger-threshold: The ratio of bytes_dirty_period and # bytes_xfer_period to trigger throttling. It is expressed as @@ -1003,7 +1016,9 @@ # Features: # # @deprecated: Member @block-incremental is deprecated. Use -# blockdev-mi
[PATCH v4 08/10] [RFC] migration: Remove helpers needed for -i/-b migrate options
[DON'T MERGE] We were abusing capabilities and parameters to implement -i/-b. Previous patch convert that options into one error. Remove all the helpers needed to implement them. Signed-off-by: Juan Quintela --- migration/migration.h | 4 migration/options.h | 6 -- migration/migration.c | 2 -- migration/options.c | 41 - 4 files changed, 53 deletions(-) diff --git a/migration/migration.h b/migration/migration.h index cd5534337c..ae53d9b26a 100644 --- a/migration/migration.h +++ b/migration/migration.h @@ -382,10 +382,6 @@ struct MigrationState { /* mutex to protect errp */ QemuMutex error_mutex; -/* Do we have to clean up -b/-i from old migrate parameters */ -/* This feature is deprecated and will be removed */ -bool must_remove_block_options; - /* * Global switch on whether we need to store the global state * during migration. diff --git a/migration/options.h b/migration/options.h index 045e2a41a2..4e8c0b9223 100644 --- a/migration/options.h +++ b/migration/options.h @@ -61,7 +61,6 @@ bool migrate_tls(void); /* capabilities helpers */ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp); -bool migrate_cap_set(int cap, bool value, Error **errp); /* parameters */ @@ -91,14 +90,9 @@ const char *migrate_tls_creds(void); const char *migrate_tls_hostname(void); uint64_t migrate_xbzrle_cache_size(void); -/* parameters setters */ - -void migrate_set_block_incremental(bool value); - /* parameters helpers */ bool migrate_params_check(MigrationParameters *params, Error **errp); void migrate_params_init(MigrationParameters *params); -void block_cleanup_parameters(void); #endif diff --git a/migration/migration.c b/migration/migration.c index 50fc112e98..ceeb2e6cb2 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -1205,7 +1205,6 @@ static void migrate_fd_cleanup(MigrationState *s) error_report_err(error_copy(s->error)); } notifier_list_notify(&migration_state_notifiers, s); -block_cleanup_parameters(); yank_unregister_instance(MIGRATION_YANK_INSTANCE); } @@ -1714,7 +1713,6 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk, "a valid migration protocol"); migrate_set_state(&s->state, MIGRATION_STATUS_SETUP, MIGRATION_STATUS_FAILED); -block_cleanup_parameters(); } if (local_err) { diff --git a/migration/options.c b/migration/options.c index e46d847de2..ecb34a3bbd 100644 --- a/migration/options.c +++ b/migration/options.c @@ -622,26 +622,6 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp) return true; } -bool migrate_cap_set(int cap, bool value, Error **errp) -{ -MigrationState *s = migrate_get_current(); -bool new_caps[MIGRATION_CAPABILITY__MAX]; - -if (migration_is_running(s->state)) { -error_setg(errp, QERR_MIGRATION_ACTIVE); -return false; -} - -memcpy(new_caps, s->capabilities, sizeof(new_caps)); -new_caps[cap] = value; - -if (!migrate_caps_check(s->capabilities, new_caps, errp)) { -return false; -} -s->capabilities[cap] = value; -return true; -} - MigrationCapabilityStatusList *qmp_query_migrate_capabilities(Error **errp) { MigrationCapabilityStatusList *head = NULL, **tail = &head; @@ -861,29 +841,8 @@ uint64_t migrate_xbzrle_cache_size(void) return s->parameters.xbzrle_cache_size; } -/* parameter setters */ - -void migrate_set_block_incremental(bool value) -{ -MigrationState *s = migrate_get_current(); - -s->parameters.block_incremental = value; -} - /* parameters helpers */ -void block_cleanup_parameters(void) -{ -MigrationState *s = migrate_get_current(); - -if (s->must_remove_block_options) { -/* setting to false can never fail */ -migrate_cap_set(MIGRATION_CAPABILITY_BLOCK, false, &error_abort); -migrate_set_block_incremental(false); -s->must_remove_block_options = false; -} -} - AnnounceParameters *migrate_announce_params(void) { static AnnounceParameters ap; -- 2.41.0
[PATCH v4 10/10] [RFC] migration: Remove block migration support
[DON'T MERGE] Signed-off-by: Juan Quintela --- meson.build|2 - qapi/migration.json| 31 +- include/migration/misc.h |8 - migration/block.h | 52 -- migration/options.h|1 - migration/block.c | 1030 migration/colo.c |1 - migration/migration-hmp-cmds.c | 34 +- migration/migration.c | 28 +- migration/options.c| 26 - migration/ram.c| 16 - migration/savevm.c |5 - hmp-commands.hx| 12 +- migration/meson.build |3 - 14 files changed, 14 insertions(+), 1235 deletions(-) delete mode 100644 migration/block.h delete mode 100644 migration/block.c diff --git a/meson.build b/meson.build index bd65a111aa..2332fa3dcb 100644 --- a/meson.build +++ b/meson.build @@ -2199,7 +2199,6 @@ config_host_data.set('CONFIG_DEBUG_GRAPH_LOCK', get_option('debug_graph_lock')) config_host_data.set('CONFIG_DEBUG_MUTEX', get_option('debug_mutex')) config_host_data.set('CONFIG_DEBUG_STACK_USAGE', get_option('debug_stack_usage')) config_host_data.set('CONFIG_DEBUG_TCG', get_option('debug_tcg')) -config_host_data.set('CONFIG_LIVE_BLOCK_MIGRATION', get_option('live_block_migration').allowed()) config_host_data.set('CONFIG_QOM_CAST_DEBUG', get_option('qom_cast_debug')) config_host_data.set('CONFIG_REPLICATION', get_option('replication').allowed()) @@ -4181,7 +4180,6 @@ if have_block summary_info += {'Use block whitelist in tools': get_option('block_drv_whitelist_in_tools')} summary_info += {'VirtFS (9P) support':have_virtfs} summary_info += {'VirtFS (9P) Proxy Helper support (deprecated)': have_virtfs_proxy_helper} - summary_info += {'Live block migration': config_host_data.get('CONFIG_LIVE_BLOCK_MIGRATION')} summary_info += {'replication support': config_host_data.get('CONFIG_REPLICATION')} summary_info += {'bochs support': get_option('bochs').allowed()} summary_info += {'cloop support': get_option('cloop').allowed()} diff --git a/qapi/migration.json b/qapi/migration.json index 545da4e257..46c0e0a90e 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -202,9 +202,6 @@ # @ram: @MigrationStats containing detailed migration status, only # returned if status is 'active' or 'completed'(since 1.2) # -# @disk: @MigrationStats containing detailed disk migration status, -# only returned if status is 'active' and it is a block migration -# # @xbzrle-cache: @XBZRLECacheStats containing detailed XBZRLE # migration statistics, only returned if XBZRLE feature is on and # status is 'active' or 'completed' (since 1.2) @@ -271,17 +268,15 @@ # # Features: # -# @deprecated: Member @disk is deprecated because block migration is. -# Member @compression is deprecated because it is unreliable and -# untested. It is recommended to use multifd migration, which -# offers an alternative compression implementation that is -# reliable and tested. +# @deprecated: Member @compression is deprecated because it is +# unreliable and untested. It is recommended to use multifd +# migration, which offers an alternative compression +# implementation that is reliable and tested. # # Since: 0.14 ## { 'struct': 'MigrationInfo', 'data': {'*status': 'MigrationStatus', '*ram': 'MigrationStats', - '*disk': { 'type': 'MigrationStats', 'features': [ 'deprecated' ] }, '*vfio': 'VfioStats', '*xbzrle-cache': 'XBZRLECacheStats', '*total-time': 'int', @@ -469,11 +464,6 @@ # @release-ram: if enabled, qemu will free the migrated ram pages on # the source during postcopy-ram migration. (since 2.9) # -# @block: If enabled, QEMU will also migrate the contents of all block -# devices. Default is disabled. A possible alternative uses -# mirror jobs to a builtin NBD server on the destination, which -# offers more flexibility. (Since 2.10) -# # @return-path: If enabled, migration will use the return path even # for precopy. (since 2.10) # @@ -533,8 +523,7 @@ # # Features: # -# @deprecated: Member @block is deprecated. Use blockdev-mirror with -# NBD instead. Member @compression is deprecated because it is +# @deprecated: Member @compression is deprecated because it is # unreliable and untested. It is recommended to use multifd # migration, which offers an alternative compression # implementation that is reliable and tested. @@ -549,7 +538,6 @@ 'events', 'postcopy-ram', { 'name': 'x-colo', 'features': [ 'unstable' ] }, 'release-ram', - { 'name': 'block', 'features': [ 'deprecated' ] }, 'return-path', 'pause-before-switchover', 'multifd', 'dirty-bitmaps', 'postcopy-blocktime', 'late-block-activate', { 'name': 'x-ignore-shared', 'features': [ 'unstable' ] }, @@ -1496,8 +1
[PATCH v4 05/10] migration: Deprecate block migration
It is obsolete. It is better to use driver-mirror with NBD instead. CC: Kevin Wolf CC: Eric Blake CC: Stefan Hajnoczi CC: Hanna Czenczek Signed-off-by: Juan Quintela Acked-by: Stefan Hajnoczi --- docs/about/deprecated.rst | 10 ++ qapi/migration.json | 29 - migration/block.c | 3 +++ migration/options.c | 9 - 4 files changed, 45 insertions(+), 6 deletions(-) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index e6928b96cf..6490925cdc 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -466,3 +466,13 @@ The new way to modify migration is using migration parameters. ``blk`` functionality can be achieved by setting the ``block`` migration capability to ``true``. +block migration (since 8.2) +''' + +Block migration is too inflexible. It needs to migrate all block +devices or none. + +Please see "QMP invocation for live storage migration with +``blockdev-mirror`` + NBD" in docs/interop/live-block-operations.rst +for a detailed explanation. + diff --git a/qapi/migration.json b/qapi/migration.json index 64ebced761..5e545d10c0 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -269,11 +269,15 @@ # average memory load of the virtual CPU indirectly. Note that # zero means guest doesn't dirty memory. (Since 8.1) # +# Features: +# +# @deprecated: Member @disk is deprecated because block migration is. +# # Since: 0.14 ## { 'struct': 'MigrationInfo', 'data': {'*status': 'MigrationStatus', '*ram': 'MigrationStats', - '*disk': 'MigrationStats', + '*disk': { 'type': 'MigrationStats', 'features': [ 'deprecated' ] }, '*vfio': 'VfioStats', '*xbzrle-cache': 'XBZRLECacheStats', '*total-time': 'int', @@ -525,6 +529,9 @@ # # Features: # +# @deprecated: Member @block is deprecated. Use blockdev-mirror with +# NBD instead. +# # @unstable: Members @x-colo and @x-ignore-shared are experimental. # # Since: 1.2 @@ -534,7 +541,8 @@ 'compress', 'events', 'postcopy-ram', { 'name': 'x-colo', 'features': [ 'unstable' ] }, 'release-ram', - 'block', 'return-path', 'pause-before-switchover', 'multifd', + { 'name': 'block', 'features': [ 'deprecated' ] }, + 'return-path', 'pause-before-switchover', 'multifd', 'dirty-bitmaps', 'postcopy-blocktime', 'late-block-activate', { 'name': 'x-ignore-shared', 'features': [ 'unstable' ] }, 'validate-uuid', 'background-snapshot', @@ -825,6 +833,9 @@ # # Features: # +# @deprecated: Member @block-incremental is deprecated. Use +# blockdev-mirror with NBD instead. +# # @unstable: Members @x-checkpoint-delay and @x-vcpu-dirty-limit-period # are experimental. # @@ -840,7 +851,7 @@ 'tls-creds', 'tls-hostname', 'tls-authz', 'max-bandwidth', 'downtime-limit', { 'name': 'x-checkpoint-delay', 'features': [ 'unstable' ] }, - 'block-incremental', + { 'name': 'block-incremental', 'features': [ 'deprecated' ] }, 'multifd-channels', 'xbzrle-cache-size', 'max-postcopy-bandwidth', 'max-cpu-throttle', 'multifd-compression', @@ -991,6 +1002,9 @@ # # Features: # +# @deprecated: Member @block-incremental is deprecated. Use +# blockdev-mirror with NBD instead. +# # @unstable: Members @x-checkpoint-delay and @x-vcpu-dirty-limit-period # are experimental. # @@ -1019,7 +1033,8 @@ '*downtime-limit': 'uint64', '*x-checkpoint-delay': { 'type': 'uint32', 'features': [ 'unstable' ] }, -'*block-incremental': 'bool', +'*block-incremental': { 'type': 'bool', +'features': [ 'deprecated' ] }, '*multifd-channels': 'uint8', '*xbzrle-cache-size': 'size', '*max-postcopy-bandwidth': 'size', @@ -1194,6 +1209,9 @@ # # Features: # +# @deprecated: Member @block-incremental is deprecated. Use +# blockdev-mirror with NBD instead. +# # @unstable: Members @x-checkpoint-delay and @x-vcpu-dirty-limit-period # are experimental. # @@ -1219,7 +1237,8 @@ '*downtime-limit': 'uint64', '*x-checkpoint-delay': { 'type': 'uint32', 'features': [ 'unstable' ] }, -'*block-incremental': 'bool', +'*block-incremental': { 'type': 'bool', +'features': [ 'deprecated' ] }, '*multifd-channels': 'uint8', '*xbzrle-cache-size': 'size', '*max-postcopy-bandwidth': 'size', diff --git a/migration/block.c b/migration/block.c index 5f930870a5..7f563c70f6 100644 --- a/migration/block.c +++ b/migration/block.c @@ -729,6 +729,9 @@ static int block_save_setup(QEMUFile *f, void *opaque) trace
[PATCH v4 09/10] [RFC] migration: Remove support for block_incremental
[DON'T MERGE] Signed-off-by: Juan Quintela --- docs/about/deprecated.rst | 7 - qapi/migration.json| 55 +++--- migration/options.h| 1 - migration/block.c | 2 +- migration/migration-hmp-cmds.c | 18 +-- migration/migration.c | 13 ++-- migration/options.c| 18 --- hmp-commands.hx| 13 +++- 8 files changed, 20 insertions(+), 107 deletions(-) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index a6da0df8e1..6bc41a430a 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -452,13 +452,6 @@ Migration ``skipped`` field in Migration stats has been deprecated. It hasn't been used for more than 10 years. -``inc`` migrate command option (since 8.2) -'' - -The new way to modify migration is using migration parameters. -``inc`` functionality can be achieved by setting the -``block-incremental`` migration parameter to ``true``. - ``blk`` migrate command option (since 8.2) '' diff --git a/qapi/migration.json b/qapi/migration.json index e599062f7d..545da4e257 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -779,13 +779,6 @@ # @x-checkpoint-delay: The delay time (in ms) between two COLO # checkpoints in periodic mode. (Since 2.8) # -# @block-incremental: Affects how much storage is migrated when the -# block migration capability is enabled. When false, the entire -# storage backing chain is migrated into a flattened image at the -# destination; when true, only the active qcow2 layer is migrated -# and the destination must already have access to the same backing -# chain as was used on the source. (since 2.10) -# # @multifd-channels: Number of channels used to migrate data in # parallel. This is the same number that the number of sockets # used for migration. The default value is 2 (since 4.0) @@ -841,10 +834,9 @@ # # Features: # -# @deprecated: Member @block-incremental is deprecated. Use -# blockdev-mirror with NBD instead. Members @compress-level, -# @compress-threads, @decompress-threads and @compress-wait-thread -# are deprecated because @compression is deprecated. +# @deprecated: Members @compress-level, @compress-threads, +# @decompress-threads and @compress-wait-thread are deprecated +# because @compression is deprecated. # # @unstable: Members @x-checkpoint-delay and @x-vcpu-dirty-limit-period # are experimental. @@ -864,7 +856,6 @@ 'tls-creds', 'tls-hostname', 'tls-authz', 'max-bandwidth', 'downtime-limit', { 'name': 'x-checkpoint-delay', 'features': [ 'unstable' ] }, - { 'name': 'block-incremental', 'features': [ 'deprecated' ] }, 'multifd-channels', 'xbzrle-cache-size', 'max-postcopy-bandwidth', 'max-cpu-throttle', 'multifd-compression', @@ -953,13 +944,6 @@ # @x-checkpoint-delay: the delay time between two COLO checkpoints. # (Since 2.8) # -# @block-incremental: Affects how much storage is migrated when the -# block migration capability is enabled. When false, the entire -# storage backing chain is migrated into a flattened image at the -# destination; when true, only the active qcow2 layer is migrated -# and the destination must already have access to the same backing -# chain as was used on the source. (since 2.10) -# # @multifd-channels: Number of channels used to migrate data in # parallel. This is the same number that the number of sockets # used for migration. The default value is 2 (since 4.0) @@ -1015,10 +999,9 @@ # # Features: # -# @deprecated: Member @block-incremental is deprecated. Use -# blockdev-mirror with NBD instead. Members @compress-level, -# @compress-threads, @decompress-threads and @compress-wait-thread -# are deprecated because @compression is deprecated. +# @deprecated: Members @compress-level, @compress-threads, +# @decompress-threads and @compress-wait-thread are deprecated +# because @compression is deprecated. # # @unstable: Members @x-checkpoint-delay and @x-vcpu-dirty-limit-period # are experimental. @@ -1052,8 +1035,6 @@ '*downtime-limit': 'uint64', '*x-checkpoint-delay': { 'type': 'uint32', 'features': [ 'unstable' ] }, -'*block-incremental': { 'type': 'bool', -'features': [ 'deprecated' ] }, '*multifd-channels': 'uint8', '*xbzrle-cache-size': 'size', '*max-postcopy-bandwidth': 'size', @@ -1166,13 +1147,6 @@ # @x-checkpoint-delay: the delay time between two COLO checkpoints. # (Since 2.8) # -# @block-incremental: Affects how much storage is migrated when the -# block migration capability is enabled. When false, the
[PATCH v4 01/10] migration: Improve json and formatting
Signed-off-by: Juan Quintela --- qapi/migration.json | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/qapi/migration.json b/qapi/migration.json index d7dfaa5db9..6865fea3c5 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -73,7 +73,7 @@ { 'struct': 'MigrationStats', 'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' , 'duplicate': 'int', - 'skipped': { 'type': 'int', 'features': ['deprecated'] }, + 'skipped': { 'type': 'int', 'features': [ 'deprecated' ] }, 'normal': 'int', 'normal-bytes': 'int', 'dirty-pages-rate': 'int', 'mbps': 'number', 'dirty-sync-count': 'int', @@ -440,10 +440,9 @@ # compress and xbzrle are both on, compress only takes effect in # the ram bulk stage, after that, it will be disabled and only # xbzrle takes effect, this can help to minimize migration -# traffic. The feature is disabled by default. (since 2.4 ) +# traffic. The feature is disabled by default. (since 2.4) # -# @events: generate events for each migration state change (since 2.4 -# ) +# @events: generate events for each migration state change (since 2.4) # # @auto-converge: If enabled, QEMU will automatically throttle down # the guest to speed up convergence of RAM migration. (since 1.6) -- 2.41.0
[PATCH v4 00/10] Migration deprecated parts
On this v4: - addressed all markus comments. - rebased on latest. - improve formatting of migration.json - print block migration status when needed. - patches 7-10 are not mean to merge, they just show why we want to deprecate block migration and remove its support. - Patch 7 just drop support for -i/-b and qmp equivalents. - Patch 8 shows all the helpers and convolutions we need to have to support that -i and -d. - patch 9 drops block-incremental migration support. - patch 9 drops block migration support. Please review. Thanks, Juan. On this v3: - Rebase on top of upstream. - Changed v8.1 to 8.2 (I left the reviewed by anyways) - missing the block deprecation code, please. Please, review. Later, Juan. On this v2: - dropped -incoming deprecation Paolo came with a better solution using keyvalues. - skipped field is already ready for next pull request, so dropped. - dropped the RFC bits, nermal PATCH. - Assessed all the review comments. - Added indentation of migration.json. - Used the documentation pointer to substitute block migration. Please review. [v1] Hi this series describe the migration parts that have to be deprecated. - It is an rfc because I doubt that I did the deprecation process right. Hello Markus O:-) - skipped field: It is older than me, I have never know what it stands for. As far as I know it has always been zero. - inc/blk migrate command options. They are only used by block migration (that I deprecate on the following patch). And they are really bad. grep must_remove_block_options. - block migration. block jobs, whatever they are called this week are way more flexible. Current code works, but we broke it here and there, and really nobody has stand up to maintain it. It is quite contained and can be left there. Is anyone really using it? - old compression method. It don't work. See last try from Lukas to make a test that works reliabely. I failed with the same task years ago. It is really slow, and if compression is good for you, multifd + zlib is going to perform/compress way more. I don't know what to do with this code, really. * Remove it for this release? It don't work, and haven't work reliabely in quite a few time. * Deprecate it and remove in another couple of releases, i.e. normal deprecation. * Ideas? - -incoming if you need to set parameters (multifd cames to mind, and preempt has the same problem), you really needs to use defer. So what should we do here? This part is not urget, because management apps have a working option that are already using "defer", and the code simplifacation if we remove it is not so big. So we can leave it until 9.0 or whatever we think fit. What do you think? Later, Juan. Juan Quintela (10): migration: Improve json and formatting migration: Print block status when needed migration: migrate 'inc' command option is deprecated. migration: migrate 'blk' command option is deprecated. migration: Deprecate block migration migration: Deprecate old compression method [RFC] migration: Make -i/-b an error for hmp and qmp [RFC] migration: Remove helpers needed for -i/-b migrate options [RFC] migration: Remove support for block_incremental [RFC] migration: Remove block migration support docs/about/deprecated.rst | 25 + meson.build|2 - qapi/migration.json| 133 ++--- include/migration/misc.h |8 - migration/block.h | 52 -- migration/migration.h |4 - migration/options.h|8 - migration/block.c | 1027 migration/colo.c |1 - migration/migration-hmp-cmds.c | 40 +- migration/migration.c | 47 +- migration/options.c| 89 +-- migration/ram.c| 16 - migration/savevm.c |5 - hmp-commands.hx| 17 +- migration/meson.build |3 - 16 files changed, 111 insertions(+), 1366 deletions(-) delete mode 100644 migration/block.h delete mode 100644 migration/block.c -- 2.41.0
[PATCH v4 02/10] migration: Print block status when needed
The new line was only printed when command options were used. When we used migration parameters and capabilities, it wasn't. Signed-off-by: Juan Quintela --- migration/migration-hmp-cmds.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c index c115ef2d23..e0e16afa34 100644 --- a/migration/migration-hmp-cmds.c +++ b/migration/migration-hmp-cmds.c @@ -30,6 +30,7 @@ #include "sysemu/runstate.h" #include "ui/qemu-spice.h" #include "sysemu/sysemu.h" +#include "options.h" #include "migration.h" static void migration_global_dump(Monitor *mon) @@ -682,7 +683,6 @@ void hmp_x_colo_lost_heartbeat(Monitor *mon, const QDict *qdict) typedef struct HMPMigrationStatus { QEMUTimer *timer; Monitor *mon; -bool is_block_migration; } HMPMigrationStatus; static void hmp_migrate_status_cb(void *opaque) @@ -708,7 +708,7 @@ static void hmp_migrate_status_cb(void *opaque) timer_mod(status->timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + 1000); } else { -if (status->is_block_migration) { +if (migrate_block()) { monitor_printf(status->mon, "\n"); } if (info->error_desc) { @@ -748,7 +748,6 @@ void hmp_migrate(Monitor *mon, const QDict *qdict) status = g_malloc0(sizeof(*status)); status->mon = mon; -status->is_block_migration = blk || inc; status->timer = timer_new_ms(QEMU_CLOCK_REALTIME, hmp_migrate_status_cb, status); timer_mod(status->timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME)); -- 2.41.0
[PATCH v4 07/10] [RFC] migration: Make -i/-b an error for hmp and qmp
[DON'T MERGE] Block migration is obsolete, users should use blockdev-mirror instead. Make it one error to set them. Signed-off-by: Juan Quintela --- migration/migration-hmp-cmds.c | 15 ++- migration/migration.c | 35 +++--- 2 files changed, 21 insertions(+), 29 deletions(-) diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c index e0e16afa34..582368926a 100644 --- a/migration/migration-hmp-cmds.c +++ b/migration/migration-hmp-cmds.c @@ -731,7 +731,20 @@ void hmp_migrate(Monitor *mon, const QDict *qdict) const char *uri = qdict_get_str(qdict, "uri"); Error *err = NULL; -qmp_migrate(uri, !!blk, blk, !!inc, inc, +if (inc) { +monitor_printf(mon, "-i migrate option is deprecated, set the" + "'block-incremental' migration parameter to 'true'" + " instead.\n"); +return; +} + +if (blk) { +monitor_printf(mon, "-b migrate option is deprecated, set the " + "'block' capability to 'true' instead.\n"); +return; +} + +qmp_migrate(uri, false, false, false, false, false, false, true, resume, &err); if (hmp_handle_error(mon, err)) { return; diff --git a/migration/migration.c b/migration/migration.c index 9e4ae6b772..50fc112e98 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -1599,17 +1599,17 @@ bool migration_is_blocked(Error **errp) static bool migrate_prepare(MigrationState *s, bool blk, bool blk_inc, bool resume, Error **errp) { -Error *local_err = NULL; - if (blk_inc) { -warn_report("@inc/-i migrate option is deprecated, set the" -"'block-incremental' migration parameter to 'true'" -" instead."); +error_setg(errp, "@inc migrate option is deprecated, set the" + "'block-incremental' migration parameter to 'true'" + " instead."); +return false; } if (blk) { -warn_report("@blk/-i migrate option is deprecated, set the " -"'block' capability to 'true' instead."); +error_setg(errp, "@blk/-i migrate option is deprecated, set the " + "'block' capability to 'true' instead."); +return false; } if (resume) { @@ -1661,27 +1661,6 @@ static bool migrate_prepare(MigrationState *s, bool blk, bool blk_inc, return false; } -if (blk || blk_inc) { -if (migrate_colo()) { -error_setg(errp, "No disk migration is required in COLO mode"); -return false; -} -if (migrate_block() || migrate_block_incremental()) { -error_setg(errp, "Command options are incompatible with " - "current migration capabilities"); -return false; -} -if (!migrate_cap_set(MIGRATION_CAPABILITY_BLOCK, true, &local_err)) { -error_propagate(errp, local_err); -return false; -} -s->must_remove_block_options = true; -} - -if (blk_inc) { -migrate_set_block_incremental(true); -} - if (migrate_init(s, errp)) { return false; } -- 2.41.0
[PATCH v4 04/10] migration: migrate 'blk' command option is deprecated.
Set the 'block' migration capability to 'true' instead. Signed-off-by: Juan Quintela Acked-by: Stefan Hajnoczi Reviewed-by: Thomas Huth --- Improve documentation and style (markus) --- docs/about/deprecated.rst | 7 +++ qapi/migration.json | 6 -- migration/migration.c | 5 + 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 1b6b2870cf..e6928b96cf 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -459,3 +459,10 @@ The new way to modify migration is using migration parameters. ``inc`` functionality can be achieved by setting the ``block-incremental`` migration parameter to ``true``. +``blk`` migrate command option (since 8.2) +'' + +The new way to modify migration is using migration parameters. +``blk`` functionality can be achieved by setting the +``block`` migration capability to ``true``. + diff --git a/qapi/migration.json b/qapi/migration.json index 56bbd55b87..64ebced761 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -1495,7 +1495,8 @@ # Features: # # @deprecated: Member @inc is deprecated. Use migration parameter -# @block-incremental instead. +# @block-incremental instead. Member @blk is deprecated. Set +# migration capability 'block' to 'true' instead. # # Returns: nothing on success # @@ -1518,7 +1519,8 @@ # <- { "return": {} } ## { 'command': 'migrate', - 'data': {'uri': 'str', '*blk': 'bool', + 'data': {'uri': 'str', + '*blk': { 'type': 'bool', 'features': [ 'deprecated' ] }, '*inc': { 'type': 'bool', 'features': [ 'deprecated' ] }, '*detach': 'bool', '*resume': 'bool' } } diff --git a/migration/migration.c b/migration/migration.c index ac4897fe0d..9e4ae6b772 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -1607,6 +1607,11 @@ static bool migrate_prepare(MigrationState *s, bool blk, bool blk_inc, " instead."); } +if (blk) { +warn_report("@blk/-i migrate option is deprecated, set the " +"'block' capability to 'true' instead."); +} + if (resume) { if (s->state != MIGRATION_STATUS_POSTCOPY_PAUSED) { error_setg(errp, "Cannot resume if there is no " -- 2.41.0
Re: [PATCH 1/1] block: improve alignment detection and fix 271 test
On 9/7/23 23:53, Denis V. Lunev wrote: Unfortunately 271 IO test is broken if started in non-cached mode. Commits commit a6b257a08e3d72219f03e461a52152672fec0612 Author: Nir Soffer Date: Tue Aug 13 21:21:03 2019 +0300 file-posix: Handle undetectable alignment and commit 9c60a5d1978e6dcf85c0e01b50e6f7f54ca09104 Author: Kevin Wolf Date: Thu Jul 16 16:26:00 2020 +0200 block: Require aligned image size to avoid assertion failure have interesting side effect if used togather. If the image size is not multiple of 4k and that image falls under original constraints of Nil's patch, the image can not be opened due to the check in the bdrv_check_perm(). The patch tries to satisfy the requirements of bdrv_check_perm() inside raw_probe_alignment(). This is at my opinion better that just disallowing to run that test in non-cached mode. The operation is legal by itself. Signed-off-by: Denis V. Lunev CC: Nir Soffer CC: Kevin Wolf CC: Hanna Reitz CC: Alberto Garcia --- block/file-posix.c | 17 +++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/block/file-posix.c b/block/file-posix.c index b16e9c21a1..988cfdc76c 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -447,8 +447,21 @@ static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp) for (i = 0; i < ARRAY_SIZE(alignments); i++) { align = alignments[i]; if (raw_is_io_aligned(fd, buf, align)) { -/* Fallback to safe value. */ -bs->bl.request_alignment = (align != 1) ? align : max_align; +if (align != 1) { +bs->bl.request_alignment = align; +break; +} +/* + * Fallback to safe value. max_align is perfect, but the size of the device must be multiple of + * the virtual length of the device. In the other case we will get a error in + * bdrv_node_refresh_perm(). + */ +for (align = max_align; align > 1; align /= 2) { +if ((bs->total_sectors * BDRV_SECTOR_SIZE) % align == 0) { +break; +} +} +bs->bl.request_alignment = align; break; } } ping
[PATCH v3 7/9] qapi/block-core: turn BlockJobInfo into a union
In preparation to additionally return job-type-specific information. Signed-off-by: Fiona Ebner Reviewed-by: Vladimir Sementsov-Ogievskiy --- No changes in v3. qapi/block-core.json | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/qapi/block-core.json b/qapi/block-core.json index a19718a69f..950542b735 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -1395,13 +1395,15 @@ # # Since: 1.1 ## -{ 'struct': 'BlockJobInfo', - 'data': {'type': 'JobType', 'device': 'str', 'len': 'int', +{ 'union': 'BlockJobInfo', + 'base': {'type': 'JobType', 'device': 'str', 'len': 'int', 'offset': 'int', 'busy': 'bool', 'paused': 'bool', 'speed': 'int', 'io-status': 'BlockDeviceIoStatus', 'ready': 'bool', 'status': 'JobStatus', 'auto-finalize': 'bool', 'auto-dismiss': 'bool', - '*error': 'str' } } + '*error': 'str' }, + 'discriminator': 'type', + 'data': {} } ## # @query-block-jobs: -- 2.39.2
[PATCH v3 5/9] mirror: implement mirror_change method
which allows switching the @copy-mode from 'background' to 'write-blocking'. This is useful for management applications, so they can start out in background mode to avoid limiting guest write speed and switch to active mode when certain criteria are fulfilled. In presence of an iothread, the copy_mode member is now shared between the iothread and the main thread, so turn accesses to it atomic. Signed-off-by: Fiona Ebner --- Changes in v3: * turn accesses to copy_mode atomic and... * ...slightly adapt error handling in mirror_change as a consequence block/mirror.c | 33 ++--- qapi/block-core.json | 13 - 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index 8992c09172..889cce5414 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -1075,7 +1075,7 @@ static int coroutine_fn mirror_run(Job *job, Error **errp) */ job_transition_to_ready(&s->common.job); } -if (s->copy_mode != MIRROR_COPY_MODE_BACKGROUND) { +if (qatomic_read(&s->copy_mode) != MIRROR_COPY_MODE_BACKGROUND) { s->actively_synced = true; } @@ -1246,6 +1246,32 @@ static bool commit_active_cancel(Job *job, bool force) return force || !job_is_ready(job); } +static void mirror_change(BlockJob *job, BlockJobChangeOptions *opts, + Error **errp) +{ +MirrorBlockJob *s = container_of(job, MirrorBlockJob, common); +BlockJobChangeOptionsMirror *change_opts = &opts->u.mirror; +MirrorCopyMode current; + +if (qatomic_read(&s->copy_mode) == change_opts->copy_mode) { +return; +} + +if (change_opts->copy_mode != MIRROR_COPY_MODE_WRITE_BLOCKING) { +error_setg(errp, "Change to copy mode '%s' is not implemented", + MirrorCopyMode_str(change_opts->copy_mode)); +return; +} + +current = qatomic_cmpxchg(&s->copy_mode, MIRROR_COPY_MODE_BACKGROUND, + change_opts->copy_mode); +if (current != MIRROR_COPY_MODE_BACKGROUND) { +error_setg(errp, "Expected current copy mode '%s', got '%s'", + MirrorCopyMode_str(MIRROR_COPY_MODE_BACKGROUND), + MirrorCopyMode_str(current)); +} +} + static const BlockJobDriver mirror_job_driver = { .job_driver = { .instance_size = sizeof(MirrorBlockJob), @@ -1260,6 +1286,7 @@ static const BlockJobDriver mirror_job_driver = { .cancel = mirror_cancel, }, .drained_poll = mirror_drained_poll, +.change = mirror_change, }; static const BlockJobDriver commit_active_job_driver = { @@ -1467,7 +1494,7 @@ static bool should_copy_to_target(MirrorBDSOpaque *s) { return s->job && s->job->ret >= 0 && !job_is_cancelled(&s->job->common.job) && -s->job->copy_mode == MIRROR_COPY_MODE_WRITE_BLOCKING; +qatomic_read(&s->job->copy_mode) == MIRROR_COPY_MODE_WRITE_BLOCKING; } static int coroutine_fn GRAPH_RDLOCK @@ -1812,7 +1839,7 @@ static BlockJob *mirror_start_job( s->is_none_mode = is_none_mode; s->backing_mode = backing_mode; s->zero_target = zero_target; -s->copy_mode = copy_mode; +qatomic_set(&s->copy_mode, copy_mode); s->base = base; s->base_overlay = bdrv_find_overlay(bs, base); s->granularity = granularity; diff --git a/qapi/block-core.json b/qapi/block-core.json index c6f31a9399..01427c259a 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -3044,6 +3044,17 @@ { 'command': 'block-job-finalize', 'data': { 'id': 'str' }, 'allow-preconfig': true } +## +# @BlockJobChangeOptionsMirror: +# +# @copy-mode: Switch to this copy mode. Currenlty, only the switch +# from 'background' to 'write-blocking' is implemented. +# +# Since: 8.2 +## +{ 'struct': 'BlockJobChangeOptionsMirror', + 'data': { 'copy-mode' : 'MirrorCopyMode' } } + ## # @BlockJobChangeOptions: # @@ -3058,7 +3069,7 @@ { 'union': 'BlockJobChangeOptions', 'base': { 'id': 'str', 'type': 'JobType' }, 'discriminator': 'type', - 'data': {} } + 'data': { 'mirror': 'BlockJobChangeOptionsMirror' } } ## # @block-job-change: -- 2.39.2
[PATCH v3 8/9] blockjob: query driver-specific info via a new 'query' driver method
Signed-off-by: Fiona Ebner --- Changes in v3: * Unlock job mutex while calling the driver's handler for query. blockjob.c | 6 ++ include/block/blockjob_int.h | 5 + 2 files changed, 11 insertions(+) diff --git a/blockjob.c b/blockjob.c index f8cf6e58e2..9c1cf2ba78 100644 --- a/blockjob.c +++ b/blockjob.c @@ -376,6 +376,7 @@ BlockJobInfo *block_job_query_locked(BlockJob *job, Error **errp) { BlockJobInfo *info; uint64_t progress_current, progress_total; +const BlockJobDriver *drv = block_job_driver(job); GLOBAL_STATE_CODE(); @@ -405,6 +406,11 @@ BlockJobInfo *block_job_query_locked(BlockJob *job, Error **errp) g_strdup(error_get_pretty(job->job.err)) : g_strdup(strerror(-job->job.ret)); } +if (drv->query) { +job_unlock(); +drv->query(job, info); +job_lock(); +} return info; } diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h index f604985315..4ab88b3c97 100644 --- a/include/block/blockjob_int.h +++ b/include/block/blockjob_int.h @@ -72,6 +72,11 @@ struct BlockJobDriver { * Change the @job's options according to @opts. */ void (*change)(BlockJob *job, BlockJobChangeOptions *opts, Error **errp); + +/* + * Query information specific to this kind of block job. + */ +void (*query)(BlockJob *job, BlockJobInfo *info); }; /* -- 2.39.2
[PATCH v3 9/9] mirror: return mirror-specific information upon query
To start out, only actively-synced is returned. For example, this is useful for jobs that started out in background mode and switched to active mode. Once actively-synced is true, it's clear that the mode switch has been completed. Note that completion of the switch might happen much earlier, e.g. if the switch happens before the job is ready, once all background operations have finished. It's assumed that whether the disks are actively-synced or not is more interesting than whether the mode switch completed. That information can still be added if required in the future. In presence of an iothread, the actively_synced member is now shared between the iothread and the main thread, so turn accesses to it atomic. Requires to adapt the output for iotest 109. Signed-off-by: Fiona Ebner --- Changes in v3: * squash with patch adapting iotest output * turn accesses to actively_synced atomic block/mirror.c | 21 - qapi/block-core.json | 16 +++- tests/qemu-iotests/109.out | 24 3 files changed, 43 insertions(+), 18 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index 889cce5414..b305c03918 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -122,7 +122,7 @@ typedef enum MirrorMethod { static BlockErrorAction mirror_error_action(MirrorBlockJob *s, bool read, int error) { -s->actively_synced = false; +qatomic_set(&s->actively_synced, false); if (read) { return block_job_error_action(&s->common, s->on_source_error, true, error); @@ -962,7 +962,7 @@ static int coroutine_fn mirror_run(Job *job, Error **errp) if (s->bdev_length == 0) { /* Transition to the READY state and wait for complete. */ job_transition_to_ready(&s->common.job); -s->actively_synced = true; +qatomic_set(&s->actively_synced, true); while (!job_cancel_requested(&s->common.job) && !s->should_complete) { job_yield(&s->common.job); } @@ -1076,7 +1076,7 @@ static int coroutine_fn mirror_run(Job *job, Error **errp) job_transition_to_ready(&s->common.job); } if (qatomic_read(&s->copy_mode) != MIRROR_COPY_MODE_BACKGROUND) { -s->actively_synced = true; +qatomic_set(&s->actively_synced, true); } should_complete = s->should_complete || @@ -1272,6 +1272,15 @@ static void mirror_change(BlockJob *job, BlockJobChangeOptions *opts, } } +static void mirror_query(BlockJob *job, BlockJobInfo *info) +{ +MirrorBlockJob *s = container_of(job, MirrorBlockJob, common); + +info->u.mirror = (BlockJobInfoMirror) { +.actively_synced = qatomic_read(&s->actively_synced), +}; +} + static const BlockJobDriver mirror_job_driver = { .job_driver = { .instance_size = sizeof(MirrorBlockJob), @@ -1287,6 +1296,7 @@ static const BlockJobDriver mirror_job_driver = { }, .drained_poll = mirror_drained_poll, .change = mirror_change, +.query = mirror_query, }; static const BlockJobDriver commit_active_job_driver = { @@ -1405,7 +1415,7 @@ do_sync_target_write(MirrorBlockJob *job, MirrorMethod method, bitmap_end = QEMU_ALIGN_UP(offset + bytes, job->granularity); bdrv_set_dirty_bitmap(job->dirty_bitmap, bitmap_offset, bitmap_end - bitmap_offset); -job->actively_synced = false; +qatomic_set(&job->actively_synced, false); action = mirror_error_action(job, false, -ret); if (action == BLOCK_ERROR_ACTION_REPORT) { @@ -1464,7 +1474,8 @@ static void coroutine_fn GRAPH_RDLOCK active_write_settle(MirrorOp *op) uint64_t end_chunk = DIV_ROUND_UP(op->offset + op->bytes, op->s->granularity); -if (!--op->s->in_active_write_counter && op->s->actively_synced) { +if (!--op->s->in_active_write_counter && +qatomic_read(&op->s->actively_synced)) { BdrvChild *source = op->s->mirror_top_bs->backing; if (QLIST_FIRST(&source->bs->parents) == source && diff --git a/qapi/block-core.json b/qapi/block-core.json index 950542b735..35d67410cc 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -1352,6 +1352,20 @@ { 'enum': 'MirrorCopyMode', 'data': ['background', 'write-blocking'] } +## +# @BlockJobInfoMirror: +# +# Information specific to mirror block jobs. +# +# @actively-synced: Whether the source is actively synced to the +# target, i.e. same data and new writes are done synchronously to +# both. +# +# Since 8.2 +## +{ 'struct': 'BlockJobInfoMirror', + 'data': { 'actively-synced': 'bool' } } + ## # @BlockJobInfo: # @@ -1403,7 +1417,7 @@ 'auto-finalize': 'bool', 'auto-dismiss': 'bool', '*error': 'str' }, 'discr
[PATCH v3 3/9] block/mirror: move dirty bitmap to filter
In preparation to allow switching to active mode without draining. Initialization of the bitmap in mirror_dirty_init() still happens with the original/backing BlockDriverState, which should be fine, because the mirror top has the same length. Suggested-by: Vladimir Sementsov-Ogievskiy Signed-off-by: Fiona Ebner Reviewed-by: Vladimir Sementsov-Ogievskiy --- No changes in v3. block/mirror.c | 16 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index b764ad5108..0ed54754e2 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -1500,6 +1500,10 @@ bdrv_mirror_top_do_write(BlockDriverState *bs, MirrorMethod method, abort(); } +if (!copy_to_target && s->job && s->job->dirty_bitmap) { +bdrv_set_dirty_bitmap(s->job->dirty_bitmap, offset, bytes); +} + if (ret < 0) { goto out; } @@ -1823,13 +1827,17 @@ static BlockJob *mirror_start_job( s->should_complete = true; } -s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, NULL, errp); +s->dirty_bitmap = bdrv_create_dirty_bitmap(s->mirror_top_bs, granularity, + NULL, errp); if (!s->dirty_bitmap) { goto fail; } -if (s->copy_mode == MIRROR_COPY_MODE_WRITE_BLOCKING) { -bdrv_disable_dirty_bitmap(s->dirty_bitmap); -} + +/* + * The dirty bitmap is set by bdrv_mirror_top_do_write() when not in active + * mode. + */ +bdrv_disable_dirty_bitmap(s->dirty_bitmap); ret = block_job_add_bdrv(&s->common, "source", bs, 0, BLK_PERM_WRITE_UNCHANGED | BLK_PERM_WRITE | -- 2.39.2
[PATCH v3 4/9] block/mirror: determine copy_to_target only once
In preparation to allow changing the copy_mode via QMP. When running in an iothread, it could be that copy_mode is changed from the main thread in between reading copy_mode in bdrv_mirror_top_pwritev() and reading copy_mode in bdrv_mirror_top_do_write(), so they might end up disagreeing about whether copy_to_target is true or false. Avoid that scenario by determining copy_to_target only once and passing it to bdrv_mirror_top_do_write() as an argument. Signed-off-by: Fiona Ebner Reviewed-by: Vladimir Sementsov-Ogievskiy --- No changes in v3. block/mirror.c | 41 ++--- 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index 0ed54754e2..8992c09172 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -1463,21 +1463,21 @@ bdrv_mirror_top_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes, return bdrv_co_preadv(bs->backing, offset, bytes, qiov, flags); } +static bool should_copy_to_target(MirrorBDSOpaque *s) +{ +return s->job && s->job->ret >= 0 && +!job_is_cancelled(&s->job->common.job) && +s->job->copy_mode == MIRROR_COPY_MODE_WRITE_BLOCKING; +} + static int coroutine_fn GRAPH_RDLOCK bdrv_mirror_top_do_write(BlockDriverState *bs, MirrorMethod method, - uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, - int flags) + bool copy_to_target, uint64_t offset, uint64_t bytes, + QEMUIOVector *qiov, int flags) { MirrorOp *op = NULL; MirrorBDSOpaque *s = bs->opaque; int ret = 0; -bool copy_to_target = false; - -if (s->job) { -copy_to_target = s->job->ret >= 0 && - !job_is_cancelled(&s->job->common.job) && - s->job->copy_mode == MIRROR_COPY_MODE_WRITE_BLOCKING; -} if (copy_to_target) { op = active_write_prepare(s->job, offset, bytes); @@ -1523,17 +1523,10 @@ static int coroutine_fn GRAPH_RDLOCK bdrv_mirror_top_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes, QEMUIOVector *qiov, BdrvRequestFlags flags) { -MirrorBDSOpaque *s = bs->opaque; QEMUIOVector bounce_qiov; void *bounce_buf; int ret = 0; -bool copy_to_target = false; - -if (s->job) { -copy_to_target = s->job->ret >= 0 && - !job_is_cancelled(&s->job->common.job) && - s->job->copy_mode == MIRROR_COPY_MODE_WRITE_BLOCKING; -} +bool copy_to_target = should_copy_to_target(bs->opaque); if (copy_to_target) { /* The guest might concurrently modify the data to write; but @@ -1550,8 +1543,8 @@ bdrv_mirror_top_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes, flags &= ~BDRV_REQ_REGISTERED_BUF; } -ret = bdrv_mirror_top_do_write(bs, MIRROR_METHOD_COPY, offset, bytes, qiov, - flags); +ret = bdrv_mirror_top_do_write(bs, MIRROR_METHOD_COPY, copy_to_target, + offset, bytes, qiov, flags); if (copy_to_target) { qemu_iovec_destroy(&bounce_qiov); @@ -1574,15 +1567,17 @@ static int coroutine_fn GRAPH_RDLOCK bdrv_mirror_top_pwrite_zeroes(BlockDriverState *bs, int64_t offset, int64_t bytes, BdrvRequestFlags flags) { -return bdrv_mirror_top_do_write(bs, MIRROR_METHOD_ZERO, offset, bytes, NULL, -flags); +bool copy_to_target = should_copy_to_target(bs->opaque); +return bdrv_mirror_top_do_write(bs, MIRROR_METHOD_ZERO, copy_to_target, +offset, bytes, NULL, flags); } static int coroutine_fn GRAPH_RDLOCK bdrv_mirror_top_pdiscard(BlockDriverState *bs, int64_t offset, int64_t bytes) { -return bdrv_mirror_top_do_write(bs, MIRROR_METHOD_DISCARD, offset, bytes, -NULL, 0); +bool copy_to_target = should_copy_to_target(bs->opaque); +return bdrv_mirror_top_do_write(bs, MIRROR_METHOD_DISCARD, copy_to_target, +offset, bytes, NULL, 0); } static void bdrv_mirror_top_refresh_filename(BlockDriverState *bs) -- 2.39.2
[PATCH v3 6/9] qapi/block-core: use JobType for BlockJobInfo's type
In preparation to turn BlockJobInfo into a union with @type as the discriminator. That requires it to be an enum. No functional change is intended. Signed-off-by: Fiona Ebner Reviewed-by: Vladimir Sementsov-Ogievskiy --- No changes in v3. block/monitor/block-hmp-cmds.c | 4 ++-- blockjob.c | 2 +- qapi/block-core.json | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c index ca2599de44..f9f87e5c47 100644 --- a/block/monitor/block-hmp-cmds.c +++ b/block/monitor/block-hmp-cmds.c @@ -843,7 +843,7 @@ void hmp_info_block_jobs(Monitor *mon, const QDict *qdict) } while (list) { -if (strcmp(list->value->type, "stream") == 0) { +if (list->value->type == JOB_TYPE_STREAM) { monitor_printf(mon, "Streaming device %s: Completed %" PRId64 " of %" PRId64 " bytes, speed limit %" PRId64 " bytes/s\n", @@ -855,7 +855,7 @@ void hmp_info_block_jobs(Monitor *mon, const QDict *qdict) monitor_printf(mon, "Type %s, device %s: Completed %" PRId64 " of %" PRId64 " bytes, speed limit %" PRId64 " bytes/s\n", - list->value->type, + JobType_str(list->value->type), list->value->device, list->value->offset, list->value->len, diff --git a/blockjob.c b/blockjob.c index d53bc775d2..f8cf6e58e2 100644 --- a/blockjob.c +++ b/blockjob.c @@ -388,7 +388,7 @@ BlockJobInfo *block_job_query_locked(BlockJob *job, Error **errp) &progress_total); info = g_new0(BlockJobInfo, 1); -info->type = g_strdup(job_type_str(&job->job)); +info->type = job_type(&job->job); info->device= g_strdup(job->job.id); info->busy = job->job.busy; info->paused= job->job.pause_count > 0; diff --git a/qapi/block-core.json b/qapi/block-core.json index 01427c259a..a19718a69f 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -1396,7 +1396,7 @@ # Since: 1.1 ## { 'struct': 'BlockJobInfo', - 'data': {'type': 'str', 'device': 'str', 'len': 'int', + 'data': {'type': 'JobType', 'device': 'str', 'len': 'int', 'offset': 'int', 'busy': 'bool', 'paused': 'bool', 'speed': 'int', 'io-status': 'BlockDeviceIoStatus', 'ready': 'bool', 'status': 'JobStatus', -- 2.39.2
[PATCH v3 1/9] blockjob: introduce block-job-change QMP command
which will allow changing job-type-specific options after job creation. In the JobVerbTable, the same allow bits as for set-speed are used, because set-speed can be considered an existing change command. Signed-off-by: Fiona Ebner Reviewed-by: Vladimir Sementsov-Ogievskiy --- No changes in v3. blockdev.c | 14 ++ blockjob.c | 20 include/block/blockjob.h | 11 +++ include/block/blockjob_int.h | 5 + job.c| 1 + qapi/block-core.json | 26 ++ qapi/job.json| 4 +++- 7 files changed, 80 insertions(+), 1 deletion(-) diff --git a/blockdev.c b/blockdev.c index 325b7a3bef..d0e274ff8b 100644 --- a/blockdev.c +++ b/blockdev.c @@ -3344,6 +3344,20 @@ void qmp_block_job_dismiss(const char *id, Error **errp) job_dismiss_locked(&job, errp); } +void qmp_block_job_change(BlockJobChangeOptions *opts, Error **errp) +{ +BlockJob *job; + +JOB_LOCK_GUARD(); +job = find_block_job_locked(opts->id, errp); + +if (!job) { +return; +} + +block_job_change_locked(job, opts, errp); +} + void qmp_change_backing_file(const char *device, const char *image_node_name, const char *backing_file, diff --git a/blockjob.c b/blockjob.c index 58c5d64539..d53bc775d2 100644 --- a/blockjob.c +++ b/blockjob.c @@ -328,6 +328,26 @@ static bool block_job_set_speed(BlockJob *job, int64_t speed, Error **errp) return block_job_set_speed_locked(job, speed, errp); } +void block_job_change_locked(BlockJob *job, BlockJobChangeOptions *opts, + Error **errp) +{ +const BlockJobDriver *drv = block_job_driver(job); + +GLOBAL_STATE_CODE(); + +if (job_apply_verb_locked(&job->job, JOB_VERB_CHANGE, errp)) { +return; +} + +if (drv->change) { +job_unlock(); +drv->change(job, opts, errp); +job_lock(); +} else { +error_setg(errp, "Job type does not support change"); +} +} + void block_job_ratelimit_processed_bytes(BlockJob *job, uint64_t n) { IO_CODE(); diff --git a/include/block/blockjob.h b/include/block/blockjob.h index 058b0c824c..95854f1477 100644 --- a/include/block/blockjob.h +++ b/include/block/blockjob.h @@ -172,6 +172,17 @@ bool block_job_has_bdrv(BlockJob *job, BlockDriverState *bs); */ bool block_job_set_speed_locked(BlockJob *job, int64_t speed, Error **errp); +/** + * block_job_change_locked: + * @job: The job to change. + * @opts: The new options. + * @errp: Error object. + * + * Change the job according to opts. + */ +void block_job_change_locked(BlockJob *job, BlockJobChangeOptions *opts, + Error **errp); + /** * block_job_query_locked: * @job: The job to get information about. diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h index 104824040c..f604985315 100644 --- a/include/block/blockjob_int.h +++ b/include/block/blockjob_int.h @@ -67,6 +67,11 @@ struct BlockJobDriver { void (*attached_aio_context)(BlockJob *job, AioContext *new_context); void (*set_speed)(BlockJob *job, int64_t speed); + +/* + * Change the @job's options according to @opts. + */ +void (*change)(BlockJob *job, BlockJobChangeOptions *opts, Error **errp); }; /* diff --git a/job.c b/job.c index 72d57f0934..99a2e54b54 100644 --- a/job.c +++ b/job.c @@ -80,6 +80,7 @@ bool JobVerbTable[JOB_VERB__MAX][JOB_STATUS__MAX] = { [JOB_VERB_COMPLETE] = {0, 0, 0, 0, 1, 1, 0, 0, 0, 0, 0}, [JOB_VERB_FINALIZE] = {0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0}, [JOB_VERB_DISMISS] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0}, +[JOB_VERB_CHANGE] = {0, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0}, }; /* Transactional group of jobs */ diff --git a/qapi/block-core.json b/qapi/block-core.json index 89751d81f2..c6f31a9399 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -3044,6 +3044,32 @@ { 'command': 'block-job-finalize', 'data': { 'id': 'str' }, 'allow-preconfig': true } +## +# @BlockJobChangeOptions: +# +# Block job options that can be changed after job creation. +# +# @id: The job identifier +# +# @type: The job type +# +# Since 8.2 +## +{ 'union': 'BlockJobChangeOptions', + 'base': { 'id': 'str', 'type': 'JobType' }, + 'discriminator': 'type', + 'data': {} } + +## +# @block-job-change: +# +# Change the block job's options. +# +# Since: 8.2 +## +{ 'command': 'block-job-change', + 'data': 'BlockJobChangeOptions', 'boxed': true } + ## # @BlockdevDiscardOptions: # diff --git a/qapi/job.json b/qapi/job.json index 7f0ba090de..b3957207a4 100644 --- a/qapi/job.json +++ b/qapi/job.json @@ -105,11 +105,13 @@ # # @finalize: see @job-finalize # +# @change: see @block-job-change (since 8.2) +# # Since: 2.12 ## { 'enum': 'JobVerb', 'data': ['cancel', 'pause', 'resume', 'set-speed', 'com
[PATCH v3 2/9] block/mirror: set actively_synced even after the job is ready
In preparation to allow switching from background to active mode. This ensures that setting actively_synced will not be missed when the switch happens after the job is ready. Signed-off-by: Fiona Ebner Reviewed-by: Vladimir Sementsov-Ogievskiy --- No changes in v3. block/mirror.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index 3cc0757a03..b764ad5108 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -1074,9 +1074,9 @@ static int coroutine_fn mirror_run(Job *job, Error **errp) * the target in a consistent state. */ job_transition_to_ready(&s->common.job); -if (s->copy_mode != MIRROR_COPY_MODE_BACKGROUND) { -s->actively_synced = true; -} +} +if (s->copy_mode != MIRROR_COPY_MODE_BACKGROUND) { +s->actively_synced = true; } should_complete = s->should_complete || -- 2.39.2
[PATCH v3 0/9] mirror: allow switching from background to active mode
Changes in v3: * unlock the job mutex when calling the new block job driver 'query' handler * squash patch adapting iotest output into patch that changes the output * turn accesses to copy_mode and actively_synced atomic * slightly rework error handling in mirror_change Changes in v2: * move bitmap to filter which allows to avoid draining when changing the copy mode * add patch to determine copy_to_target only once * drop patches returning redundant information upon query * update QEMU version in QAPI * update indentation in QAPI * update indentation in QAPI (like in a937b6aa73 ("qapi: Reformat doc comments to conform to current conventions")) * add patch to adapt iotest output Discussion of v2: https://lists.nongnu.org/archive/html/qemu-devel/2023-10/msg02290.html Discussion of v1: https://lists.nongnu.org/archive/html/qemu-devel/2023-02/msg07216.html With active mode, the guest write speed is limited by the synchronous writes to the mirror target. For this reason, management applications might want to start out in background mode and only switch to active mode later, when certain conditions are met. This series adds a block-job-change QMP command to achieve that, as well as job-type-specific information when querying block jobs, which can be used to decide when the switch should happen. For now, only the direction background -> active is supported. The information added upon querying is whether the target is actively synced, the total data sent, and the remaining dirty bytes. Initially, I tried to go for a more general 'job-change' command, but to avoid mutual inclusion of block-core.json and job.json, more preparation would be required. Fiona Ebner (9): blockjob: introduce block-job-change QMP command block/mirror: set actively_synced even after the job is ready block/mirror: move dirty bitmap to filter block/mirror: determine copy_to_target only once mirror: implement mirror_change method qapi/block-core: use JobType for BlockJobInfo's type qapi/block-core: turn BlockJobInfo into a union blockjob: query driver-specific info via a new 'query' driver method mirror: return mirror-specific information upon query block/mirror.c | 111 ++--- block/monitor/block-hmp-cmds.c | 4 +- blockdev.c | 14 + blockjob.c | 28 - include/block/blockjob.h | 11 include/block/blockjob_int.h | 10 +++ job.c | 1 + qapi/block-core.json | 59 +- qapi/job.json | 4 +- tests/qemu-iotests/109.out | 24 +++ 10 files changed, 212 insertions(+), 54 deletions(-) -- 2.39.2
Re: [PATCH v4] migration: hold the BQL during setup
Fiona Ebner wrote: > This is intended to be a semantic revert of commit 9b09503752 > ("migration: run setup callbacks out of big lock"). There have been so > many changes since that commit (e.g. a new setup callback > dirty_bitmap_save_setup() that also needs to be adapted now), it's > easier to do the revert manually. > > For snapshots, the bdrv_writev_vmstate() function is used during setup > (in QIOChannelBlock backing the QEMUFile), but not holding the BQL > while calling it could lead to an assertion failure. To understand > how, first note the following: > > 1. Generated coroutine wrappers for block layer functions spawn the > coroutine and use AIO_WAIT_WHILE()/aio_poll() to wait for it. > 2. If the host OS switches threads at an inconvenient time, it can > happen that a bottom half scheduled for the main thread's AioContext > is executed as part of a vCPU thread's aio_poll(). > > An example leading to the assertion failure is as follows: > > main thread: > 1. A snapshot-save QMP command gets issued. > 2. snapshot_save_job_bh() is scheduled. > > vCPU thread: > 3. aio_poll() for the main thread's AioContext is called (e.g. when > the guest writes to a pflash device, as part of blk_pwrite which is a > generated coroutine wrapper). > 4. snapshot_save_job_bh() is executed as part of aio_poll(). > 3. qemu_savevm_state() is called. > 4. qemu_mutex_unlock_iothread() is called. Now > qemu_get_current_aio_context() returns 0x0. > 5. bdrv_writev_vmstate() is executed during the usual savevm setup > via qemu_fflush(). But this function is a generated coroutine wrapper, > so it uses AIO_WAIT_WHILE. There, the assertion > assert(qemu_get_current_aio_context() == qemu_get_aio_context()); > will fail. > > To fix it, ensure that the BQL is held during setup. While it would > only be needed for snapshots, adapting migration too avoids additional > logic for conditional locking/unlocking in the setup callbacks. > Writing the header could (in theory) also trigger qemu_fflush() and > thus bdrv_writev_vmstate(), so the locked section also covers the > qemu_savevm_state_header() call, even for migration for consistentcy. > > The section around multifd_send_sync_main() needs to be unlocked to > avoid a deadlock. In particular, the function calls > socket_send_channel_create() using multifd_new_send_channel_async() as > a callback and then waits for the callback to signal via the > channels_ready semaphore. The connection happens via > qio_task_run_in_thread(), but the callback is only executed via > qio_task_thread_result() which is scheduled for the main event loop. > Without unlocking the section, the main thread would never get to > process the task result and the callback meaning there would be no > signal via the channels_ready semaphore. > > The comment in ram_init_bitmaps() was introduced by 4987783400 > ("migration: fix incorrect memory_global_dirty_log_start outside BQL") > and is removed, because it referred to the qemu_mutex_lock_iothread() > call. > > Signed-off-by: Fiona Ebner Reviewed-by: Juan Quintela I don't have a better solution for this problem, if nobody complains in the next couple of days, I will pull this change. Thanks for the effort.
[RFC PATCH v3 76/78] qemu-img.c: add fallthrough pseudo-keyword
In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- qemu-img.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qemu-img.c b/qemu-img.c index 6068ab0d27..df2457a6fe 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1906,7 +1906,7 @@ static int coroutine_fn convert_co_write(ImgConvertState *s, int64_t sector_num, } break; } -/* fall-through */ +fallthrough; case BLK_ZERO: if (s->has_zero_init) { -- 2.39.2
[RFC PATCH v3 65/78] hw/nvme: add fallthrough pseudo-keyword
In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- hw/nvme/ctrl.c | 24 hw/nvme/dif.c | 4 ++-- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index f026245d1e..acb2012fb9 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -1918,7 +1918,7 @@ static uint16_t nvme_zrm_finish(NvmeNamespace *ns, NvmeZone *zone) case NVME_ZONE_STATE_IMPLICITLY_OPEN: case NVME_ZONE_STATE_EXPLICITLY_OPEN: nvme_aor_dec_open(ns); -/* fallthrough */ +fallthrough; case NVME_ZONE_STATE_CLOSED: nvme_aor_dec_active(ns); @@ -1929,7 +1929,7 @@ static uint16_t nvme_zrm_finish(NvmeNamespace *ns, NvmeZone *zone) } } -/* fallthrough */ +fallthrough; case NVME_ZONE_STATE_EMPTY: nvme_assign_zone_state(ns, zone, NVME_ZONE_STATE_FULL); return NVME_SUCCESS; @@ -1946,7 +1946,7 @@ static uint16_t nvme_zrm_close(NvmeNamespace *ns, NvmeZone *zone) case NVME_ZONE_STATE_IMPLICITLY_OPEN: nvme_aor_dec_open(ns); nvme_assign_zone_state(ns, zone, NVME_ZONE_STATE_CLOSED); -/* fall through */ +fallthrough; case NVME_ZONE_STATE_CLOSED: return NVME_SUCCESS; @@ -1961,7 +1961,7 @@ static uint16_t nvme_zrm_reset(NvmeNamespace *ns, NvmeZone *zone) case NVME_ZONE_STATE_EXPLICITLY_OPEN: case NVME_ZONE_STATE_IMPLICITLY_OPEN: nvme_aor_dec_open(ns); -/* fallthrough */ +fallthrough; case NVME_ZONE_STATE_CLOSED: nvme_aor_dec_active(ns); @@ -1971,12 +1971,12 @@ static uint16_t nvme_zrm_reset(NvmeNamespace *ns, NvmeZone *zone) } } -/* fallthrough */ +fallthrough; case NVME_ZONE_STATE_FULL: zone->w_ptr = zone->d.zslba; zone->d.wp = zone->w_ptr; nvme_assign_zone_state(ns, zone, NVME_ZONE_STATE_EMPTY); -/* fallthrough */ +fallthrough; case NVME_ZONE_STATE_EMPTY: return NVME_SUCCESS; @@ -2017,7 +2017,7 @@ static uint16_t nvme_zrm_open_flags(NvmeCtrl *n, NvmeNamespace *ns, case NVME_ZONE_STATE_EMPTY: act = 1; -/* fallthrough */ +fallthrough; case NVME_ZONE_STATE_CLOSED: if (n->params.auto_transition_zones) { @@ -2040,7 +2040,7 @@ static uint16_t nvme_zrm_open_flags(NvmeCtrl *n, NvmeNamespace *ns, return NVME_SUCCESS; } -/* fallthrough */ +fallthrough; case NVME_ZONE_STATE_IMPLICITLY_OPEN: if (flags & NVME_ZRM_AUTO) { @@ -2049,7 +2049,7 @@ static uint16_t nvme_zrm_open_flags(NvmeCtrl *n, NvmeNamespace *ns, nvme_assign_zone_state(ns, zone, NVME_ZONE_STATE_EXPLICITLY_OPEN); -/* fallthrough */ +fallthrough; case NVME_ZONE_STATE_EXPLICITLY_OPEN: if (flags & NVME_ZRM_ZRWA) { @@ -3582,7 +3582,7 @@ static uint16_t nvme_do_write(NvmeCtrl *n, NvmeRequest *req, bool append, return NVME_INVALID_PROT_INFO | NVME_DNR; } -/* fallthrough */ +fallthrough; case NVME_ID_NS_DPS_TYPE_2: if (piremap) { @@ -3737,7 +3737,7 @@ static uint16_t nvme_offline_zone(NvmeNamespace *ns, NvmeZone *zone, switch (state) { case NVME_ZONE_STATE_READ_ONLY: nvme_assign_zone_state(ns, zone, NVME_ZONE_STATE_OFFLINE); -/* fall through */ +fallthrough; case NVME_ZONE_STATE_OFFLINE: return NVME_SUCCESS; default: @@ -4914,7 +4914,7 @@ static uint16_t nvme_cmd_effects(NvmeCtrl *n, uint8_t csi, uint32_t buf_len, switch (NVME_CC_CSS(ldl_le_p(&n->bar.cc))) { case NVME_CC_CSS_NVM: src_iocs = nvme_cse_iocs_nvm; -/* fall through */ +fallthrough; case NVME_CC_CSS_ADMIN_ONLY: break; case NVME_CC_CSS_CSI: diff --git a/hw/nvme/dif.c b/hw/nvme/dif.c index 01b19c3373..00dd96bdb3 100644 --- a/hw/nvme/dif.c +++ b/hw/nvme/dif.c @@ -161,7 +161,7 @@ static uint16_t nvme_dif_prchk_crc16(NvmeNamespace *ns, NvmeDifTuple *dif, break; } -/* fallthrough */ +fallthrough; case NVME_ID_NS_DPS_TYPE_1: case NVME_ID_NS_DPS_TYPE_2: if (be16_to_cpu(dif->g16.apptag) != 0x) { @@ -229,7 +229,7 @@ static uint16_t nvme_dif_prchk_crc64(NvmeNamespace *ns, NvmeDifTuple *dif, break; } -/* fallthrough */ +fallthrough; case NVME_ID_NS_DPS_TYPE_1: case NVME_ID_NS_DPS_TYPE_2: if (be16_to_cpu(dif->g64.apptag) != 0x) { -- 2.39.2
[RFC PATCH v3 39/78] hw/scsi: add fallthrough pseudo-keyword
In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- hw/scsi/esp.c | 2 +- hw/scsi/megasas.c | 2 +- hw/scsi/scsi-bus.c | 4 ++-- hw/scsi/scsi-disk.c | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c index 9b11d8c573..d6c8298f51 100644 --- a/hw/scsi/esp.c +++ b/hw/scsi/esp.c @@ -1025,7 +1025,7 @@ void esp_reg_write(ESPState *s, uint32_t saddr, uint64_t val) switch (saddr) { case ESP_TCHI: s->tchi_written = true; -/* fall through */ +fallthrough; case ESP_TCLO: case ESP_TCMID: s->rregs[ESP_RSTAT] &= ~STAT_TC; diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c index 32c70c9e99..54e4d7c8b6 100644 --- a/hw/scsi/megasas.c +++ b/hw/scsi/megasas.c @@ -2151,7 +2151,7 @@ static void megasas_mmio_write(void *opaque, hwaddr addr, case MFI_IQPL: trace_megasas_mmio_writel("MFI_IQPL", val); /* Received low 32 bits of a 64 bit MFI frame address */ -/* Fallthrough */ +fallthrough; case MFI_IQP: if (addr == MFI_IQP) { trace_megasas_mmio_writel("MFI_IQP", val); diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c index fc4b77fdb0..a1c298a92c 100644 --- a/hw/scsi/scsi-bus.c +++ b/hw/scsi/scsi-bus.c @@ -1078,7 +1078,7 @@ static int scsi_req_xfer(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf) if (cmd->xfer == 0) { cmd->xfer = 256; } -/* fall through */ +fallthrough; case WRITE_10: case WRITE_VERIFY_10: case WRITE_12: @@ -1093,7 +1093,7 @@ static int scsi_req_xfer(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf) if (cmd->xfer == 0) { cmd->xfer = 256; } -/* fall through */ +fallthrough; case READ_10: case READ_12: case READ_16: diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index 6691f5edb8..6564ca638c 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -2302,7 +2302,7 @@ static int32_t scsi_disk_dma_command(SCSIRequest *req, uint8_t *buf) trace_scsi_disk_dma_command_WRITE( (command & 0xe) == 0xe ? "And Verify " : "", r->req.cmd.lba, len); -/* fall through */ +fallthrough; case VERIFY_10: case VERIFY_12: case VERIFY_16: -- 2.39.2
[RFC PATCH v3 40/78] hw/sd/sdhci.c: add fallthrough pseudo-keyword
In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- hw/sd/sdhci.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c index 5564765a9b..5c641d24de 100644 --- a/hw/sd/sdhci.c +++ b/hw/sd/sdhci.c @@ -92,7 +92,7 @@ static void sdhci_check_capareg(SDHCIState *s, Error **errp) trace_sdhci_capareg("ADMA3", val); msk = FIELD_DP64(msk, SDHC_CAPAB, ADMA3, 0); -/* fallthrough */ +fallthrough; case 3: val = FIELD_EX64(s->capareg, SDHC_CAPAB, ASYNC_INT); trace_sdhci_capareg("async interrupt", val); @@ -136,7 +136,7 @@ static void sdhci_check_capareg(SDHCIState *s, Error **errp) trace_sdhci_capareg("clock multiplier", val); msk = FIELD_DP64(msk, SDHC_CAPAB, CLOCK_MULT, 0); -/* fallthrough */ +fallthrough; case 2: /* default version */ val = FIELD_EX64(s->capareg, SDHC_CAPAB, ADMA2); trace_sdhci_capareg("ADMA2", val); @@ -150,7 +150,7 @@ static void sdhci_check_capareg(SDHCIState *s, Error **errp) trace_sdhci_capareg("64-bit system bus (v3)", val); msk = FIELD_DP64(msk, SDHC_CAPAB, BUS64BIT, 0); -/* fallthrough */ +fallthrough; case 1: y = FIELD_EX64(s->capareg, SDHC_CAPAB, TOUNIT); msk = FIELD_DP64(msk, SDHC_CAPAB, TOUNIT, 0); @@ -1839,7 +1839,7 @@ usdhc_write(void *opaque, hwaddr offset, uint64_t val, unsigned size) * so we artificially set it to that value. */ val |= 0x7 << 12; -/* FALLTHROUGH */ +fallthrough; default: sdhci_write(opaque, offset, val, size); break; -- 2.39.2
[RFC PATCH v3 53/78] nbd: add fallthrough pseudo-keyword
In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- nbd/client.c | 4 ++-- nbd/common.c | 2 +- qemu-nbd.c | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/nbd/client.c b/nbd/client.c index 29ffc609a4..04507249b2 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -1051,7 +1051,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, QCryptoTLSCreds *tlscreds, } info->base_allocation = result == 1; } -/* fall through */ +fallthrough; case NBD_MODE_SIMPLE: /* Try NBD_OPT_GO first - if it works, we are done (it * also gives us a good message if the server requires @@ -1074,7 +1074,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, QCryptoTLSCreds *tlscreds, if (nbd_receive_query_exports(ioc, info->name, errp) < 0) { return -EINVAL; } -/* fall through */ +fallthrough; case NBD_MODE_EXPORT_NAME: /* write the export name request */ if (nbd_send_option_request(ioc, NBD_OPT_EXPORT_NAME, -1, info->name, diff --git a/nbd/common.c b/nbd/common.c index 3247c1d618..1140ea0888 100644 --- a/nbd/common.c +++ b/nbd/common.c @@ -249,7 +249,7 @@ int nbd_errno_to_system_errno(int err) break; default: trace_nbd_unknown_error(err); -/* fallthrough */ +fallthrough; case NBD_EINVAL: ret = EINVAL; break; diff --git a/qemu-nbd.c b/qemu-nbd.c index 186e6468b1..41e50208a5 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -632,7 +632,7 @@ int main(int argc, char **argv) break; case 'n': optarg = (char *) "none"; -/* fallthrough */ +fallthrough; case QEMU_NBD_OPT_CACHE: if (seen_cache) { error_report("-n and --cache can only be specified once"); @@ -708,7 +708,7 @@ int main(int argc, char **argv) } else { sn_id_or_name = optarg; } -/* fall through */ +fallthrough; case 'r': readonly = true; flags &= ~BDRV_O_RDWR; -- 2.39.2
[RFC PATCH v3 10/78] hw/ide/atapi.c: add fallthrough pseudo-keyword
In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- hw/ide/atapi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c index dcc39df9a4..85c74a5ffe 100644 --- a/hw/ide/atapi.c +++ b/hw/ide/atapi.c @@ -1225,6 +1225,7 @@ static void cmd_read_dvd_structure(IDEState *s, uint8_t* buf) break; } /* TODO: BD support, fall through for now */ +fallthrough; /* Generic disk structures */ case 0x80: /* TODO: AACS volume identifier */ -- 2.39.2
[RFC PATCH v3 08/78] hw/block: add fallthrough pseudo-keyword
In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- hw/block/dataplane/xen-block.c | 4 ++-- hw/block/m25p80.c | 2 +- hw/block/onenand.c | 2 +- hw/block/pflash_cfi01.c| 1 + hw/block/pflash_cfi02.c| 6 -- 5 files changed, 9 insertions(+), 6 deletions(-) diff --git a/hw/block/dataplane/xen-block.c b/hw/block/dataplane/xen-block.c index 3b6f2b0aa2..1ae25a73b2 100644 --- a/hw/block/dataplane/xen-block.c +++ b/hw/block/dataplane/xen-block.c @@ -159,7 +159,7 @@ static int xen_block_parse_request(XenBlockRequest *request) if (!request->req.nr_segments) { return 0; } -/* fall through */ +fallthrough; case BLKIF_OP_WRITE: break; case BLKIF_OP_DISCARD: @@ -299,7 +299,7 @@ static void xen_block_complete_aio(void *opaque, int ret) if (!request->req.nr_segments) { break; } -/* fall through */ +fallthrough; case BLKIF_OP_READ: if (request->status == BLKIF_RSP_OKAY) { block_acct_done(blk_get_stats(dataplane->blk), &request->acct); diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c index afc3fdf4d6..523c34da71 100644 --- a/hw/block/m25p80.c +++ b/hw/block/m25p80.c @@ -1462,7 +1462,7 @@ static void decode_new_cmd(Flash *s, uint32_t value) s->state = STATE_COLLECTING_DATA; break; } -/* Fallthrough */ +fallthrough; default: s->pos = 0; diff --git a/hw/block/onenand.c b/hw/block/onenand.c index 50d3d1c985..87583c48a0 100644 --- a/hw/block/onenand.c +++ b/hw/block/onenand.c @@ -564,7 +564,7 @@ static void onenand_command(OneNANDState *s) break; case 0x95: /* Multi-block erase */ qemu_irq_pulse(s->intr); -/* Fall through. */ +fallthrough; case 0x94: /* Block erase */ sec = ((s->addr[ONEN_BUF_BLOCK] & 0xfff) | (s->addr[ONEN_BUF_BLOCK] >> 15 ? s->density_mask : 0)) diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c index 62056b1d74..cb58f08f53 100644 --- a/hw/block/pflash_cfi01.c +++ b/hw/block/pflash_cfi01.c @@ -276,6 +276,7 @@ static uint32_t pflash_read(PFlashCFI01 *pfl, hwaddr offset, */ pfl->cmd = 0x00; /* fall through to read code */ +fallthrough; case 0x00: /* This model reset value for READ_ARRAY (not CFI compliant) */ /* Flash area read */ ret = pflash_data_read(pfl, offset, width, be); diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c index 2a99b286b0..711f978d7c 100644 --- a/hw/block/pflash_cfi02.c +++ b/hw/block/pflash_cfi02.c @@ -328,6 +328,7 @@ static uint64_t pflash_read(void *opaque, hwaddr offset, unsigned int width) trace_pflash_read_unknown_state(pfl->name, pfl->cmd); pflash_reset_state_machine(pfl); /* fall through to the read code */ +fallthrough; case 0x80: /* Erase (unlock) */ /* We accept reads during second unlock sequence... */ case 0x00: @@ -359,6 +360,7 @@ static uint64_t pflash_read(void *opaque, hwaddr offset, unsigned int width) break; } /* Fall through to data read. */ +fallthrough; default: ret = pflash_data_read(pfl, offset, width); } @@ -368,7 +370,7 @@ static uint64_t pflash_read(void *opaque, hwaddr offset, unsigned int width) case 0x30: /* Sector Erase */ /* Toggle bit 2 during erase, but not program. */ toggle_dq2(pfl); -/* fall through */ +fallthrough; case 0xA0: /* Program */ /* Toggle bit 6 */ toggle_dq6(pfl); @@ -582,7 +584,7 @@ static void pflash_write(void *opaque, hwaddr offset, uint64_t value, pfl->cmd = 0x98; return; } -/* fall through */ +fallthrough; default: trace_pflash_write_invalid(pfl->name, pfl->cmd); goto reset_flash; -- 2.39.2
[RFC PATCH v3 02/78] block: add fallthrough pseudo-keyword
In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- block/block-copy.c| 1 + block/file-posix.c| 1 + block/io.c| 1 + block/iscsi.c | 1 + block/qcow2-cluster.c | 5 - block/vhdx.c | 17 + 6 files changed, 21 insertions(+), 5 deletions(-) diff --git a/block/block-copy.c b/block/block-copy.c index 1c60368d72..b4ceb6a079 100644 --- a/block/block-copy.c +++ b/block/block-copy.c @@ -508,6 +508,7 @@ block_copy_do_copy(BlockCopyState *s, int64_t offset, int64_t bytes, trace_block_copy_copy_range_fail(s, offset, ret); *method = COPY_READ_WRITE; /* Fall through to read+write with allocated buffer */ +fallthrough; case COPY_READ_WRITE_CLUSTER: case COPY_READ_WRITE: diff --git a/block/file-posix.c b/block/file-posix.c index 50e2b20d5c..31c7719da5 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -1013,6 +1013,7 @@ static int raw_handle_perm_lock(BlockDriverState *bs, bs->filename); } /* fall through to unlock bytes. */ +fallthrough; case RAW_PL_ABORT: raw_apply_lock_bytes(s, s->fd, s->perm, ~s->shared_perm, true, &local_err); diff --git a/block/io.c b/block/io.c index e7f9448d5a..cc05457d02 100644 --- a/block/io.c +++ b/block/io.c @@ -2034,6 +2034,7 @@ bdrv_co_write_req_finish(BdrvChild *child, int64_t offset, int64_t bytes, case BDRV_TRACKED_WRITE: stat64_max(&bs->wr_highest_offset, offset + bytes); /* fall through, to set dirty bits */ +fallthrough; case BDRV_TRACKED_DISCARD: bdrv_set_dirty(bs, offset, bytes); break; diff --git a/block/iscsi.c b/block/iscsi.c index 5640c8b565..2fb7037748 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -1461,6 +1461,7 @@ static void iscsi_readcapacity_sync(IscsiLun *iscsilun, Error **errp) break; } /* Fall through and try READ CAPACITY(10) instead. */ +fallthrough; case TYPE_ROM: task = iscsi_readcapacity10_sync(iscsilun->iscsi, iscsilun->lun, 0, 0); if (task != NULL && task->status == SCSI_STATUS_GOOD) { diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index f4f6cd6ad0..c50143d493 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -1333,13 +1333,16 @@ static bool cluster_needs_new_alloc(BlockDriverState *bs, uint64_t l2_entry) { switch (qcow2_get_cluster_type(bs, l2_entry)) { case QCOW2_CLUSTER_NORMAL: +fallthrough; case QCOW2_CLUSTER_ZERO_ALLOC: if (l2_entry & QCOW_OFLAG_COPIED) { return false; } -/* fallthrough */ +fallthrough; case QCOW2_CLUSTER_UNALLOCATED: +fallthrough; case QCOW2_CLUSTER_COMPRESSED: +fallthrough; case QCOW2_CLUSTER_ZERO_PLAIN: return true; default: diff --git a/block/vhdx.c b/block/vhdx.c index a67edcc03e..9000b3fcea 100644 --- a/block/vhdx.c +++ b/block/vhdx.c @@ -1201,10 +1201,14 @@ vhdx_co_readv(BlockDriverState *bs, int64_t sector_num, int nb_sectors, /* check the payload block state */ switch (s->bat[sinfo.bat_idx] & VHDX_BAT_STATE_BIT_MASK) { -case PAYLOAD_BLOCK_NOT_PRESENT: /* fall through */ +case PAYLOAD_BLOCK_NOT_PRESENT: +fallthrough; case PAYLOAD_BLOCK_UNDEFINED: +fallthrough; case PAYLOAD_BLOCK_UNMAPPED: +fallthrough; case PAYLOAD_BLOCK_UNMAPPED_v095: +fallthrough; case PAYLOAD_BLOCK_ZERO: /* return zero */ qemu_iovec_memset(&hd_qiov, 0, 0, sinfo.bytes_avail); @@ -1222,6 +1226,7 @@ vhdx_co_readv(BlockDriverState *bs, int64_t sector_num, int nb_sectors, case PAYLOAD_BLOCK_PARTIALLY_PRESENT: /* we don't yet support difference files, fall through * to error */ +fallthrough; default: ret = -EIO; goto exit; @@ -1373,10 +1378,13 @@ vhdx_co_writev(BlockDriverState *bs, int64_t sector_num, int nb_sectors, * data that is not part of this write, so we must pad * the rest of the buffer to zeroes */ use_zero_buffers = true; -/* fall through */ -case PAYLOAD_BLOCK_NOT_PRESENT: /* fall through */ +fallthrough; +case PAYLOAD_BLOCK_NOT_PRESENT: +fallthrough; case PAYLOAD_BLOCK_UNMAPPED: +fallthrough; case PAYLOAD_BLOCK_UNMAPPED_v095: +fallthrough; case PAYLOAD_BLOCK_UNDEFINED: bat_p
Re: [RFC PATCH 01/78] include/qemu/compiler.h: replace QEMU_FALLTHROUGH with fallthrough
On Fri, 13 Oct 2023 at 11:16, Daniel P. Berrangé wrote: > This patch (and all the others in the series) have a ridiculously > large context either side of the change. It makes this horrible > to review as it requires wading through pages of pre-existing code > trying to spot the change. > > Please send patches with the default git context lines setting. Many thanks Daniel, I had not noticed at all. Somehow that option slipped through... Will reroll the patch series.
[RFC PATCH 18/78] ui/win32-kbd-hook.c: add fallthrough pseudo-keyword
In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- ui/win32-kbd-hook.c | 7 --- 1 file changed, 7 deletions(-) diff --git a/ui/win32-kbd-hook.c b/ui/win32-kbd-hook.c index 1ac237db9e..3c5c3fc597 100644 --- a/ui/win32-kbd-hook.c +++ b/ui/win32-kbd-hook.c @@ -18,59 +18,52 @@ static DWORD win32_grab; static LRESULT CALLBACK keyboard_hook_cb(int code, WPARAM wparam, LPARAM lparam) { if (win32_window && code == HC_ACTION && win32_window == GetFocus()) { KBDLLHOOKSTRUCT *hooked = (KBDLLHOOKSTRUCT *)lparam; if (wparam != WM_KEYUP) { DWORD dwmsg = (hooked->flags << 24) | ((hooked->scanCode & 0xff) << 16) | 1; switch (hooked->vkCode) { case VK_CAPITAL: -/* fall through */ case VK_SCROLL: -/* fall through */ case VK_NUMLOCK: -/* fall through */ case VK_LSHIFT: -/* fall through */ case VK_RSHIFT: -/* fall through */ case VK_RCONTROL: -/* fall through */ case VK_LMENU: -/* fall through */ case VK_RMENU: break; case VK_LCONTROL: /* * When pressing AltGr, an extra VK_LCONTROL with a special * scancode with bit 9 set is sent. Let's ignore the extra * VK_LCONTROL, as that will make AltGr misbehave. */ if (hooked->scanCode & 0x200) { return 1; } break; default: if (win32_grab) { SendMessage(win32_window, wparam, hooked->vkCode, dwmsg); return 1; } break; } } else { switch (hooked->vkCode) { case VK_LCONTROL: if (hooked->scanCode & 0x200) { return 1; } break; } } } return CallNextHookEx(NULL, code, wparam, lparam); } -- 2.39.2
Re: [RFC PATCH 26/78] target/s390x: add fallthrough pseudo-keyword
On 13.10.23 09:47, Emmanouil Pitsidianakis wrote: In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- Reviewed-by: David Hildenbrand -- Cheers, David / dhildenb
[RFC PATCH 19/78] target/hppa: add fallthrough pseudo-keyword
In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- target/hppa/translate.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/target/hppa/translate.c b/target/hppa/translate.c index 9f3ba9f42f..1df81b0fa2 100644 --- a/target/hppa/translate.c +++ b/target/hppa/translate.c @@ -480,14 +480,14 @@ static DisasCond cond_make(TCGCond c, TCGv_reg a0, TCGv_reg a1) static void cond_free(DisasCond *cond) { switch (cond->c) { default: cond->a0 = NULL; cond->a1 = NULL; -/* fallthru */ +fallthrough; case TCG_COND_ALWAYS: cond->c = TCG_COND_NEVER; break; case TCG_COND_NEVER: break; } } @@ -3831,65 +3831,65 @@ static bool trans_fcmp_d(DisasContext *ctx, arg_fclass2 *a) static bool trans_ftest(DisasContext *ctx, arg_ftest *a) { TCGv_reg t; nullify_over(ctx); t = get_temp(ctx); tcg_gen_ld32u_reg(t, tcg_env, offsetof(CPUHPPAState, fr0_shadow)); if (a->y == 1) { int mask; bool inv = false; switch (a->c) { case 0: /* simple */ tcg_gen_andi_reg(t, t, 0x400); ctx->null_cond = cond_make_0(TCG_COND_NE, t); goto done; case 2: /* rej */ inv = true; -/* fallthru */ +fallthrough; case 1: /* acc */ mask = 0x43ff800; break; case 6: /* rej8 */ inv = true; -/* fallthru */ +fallthrough; case 5: /* acc8 */ mask = 0x43f8000; break; case 9: /* acc6 */ mask = 0x43e; break; case 13: /* acc4 */ mask = 0x438; break; case 17: /* acc2 */ mask = 0x420; break; default: gen_illegal(ctx); return true; } if (inv) { TCGv_reg c = load_const(ctx, mask); tcg_gen_or_reg(t, t, c); ctx->null_cond = cond_make(TCG_COND_EQ, t, c); } else { tcg_gen_andi_reg(t, t, mask); ctx->null_cond = cond_make_0(TCG_COND_EQ, t); } } else { unsigned cbit = (a->y ^ 1) - 1; tcg_gen_extract_reg(t, t, 21 - cbit, 1); ctx->null_cond = cond_make_0(TCG_COND_NE, t); } done: return nullify_end(ctx); } /* * Float class 2 */ @@ -4219,28 +4219,28 @@ static void hppa_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs) static void hppa_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs) { DisasContext *ctx = container_of(dcbase, DisasContext, base); DisasJumpType is_jmp = ctx->base.is_jmp; switch (is_jmp) { case DISAS_NORETURN: break; case DISAS_TOO_MANY: case DISAS_IAQ_N_STALE: case DISAS_IAQ_N_STALE_EXIT: copy_iaoq_entry(cpu_iaoq_f, ctx->iaoq_f, cpu_iaoq_f); copy_iaoq_entry(cpu_iaoq_b, ctx->iaoq_b, cpu_iaoq_b); nullify_save(ctx); -/* FALLTHRU */ +fallthrough; case DISAS_IAQ_N_UPDATED: if (is_jmp != DISAS_IAQ_N_STALE_EXIT) { tcg_gen_lookup_and_goto_ptr(); break; } -/* FALLTHRU */ +fallthrough; case DISAS_EXIT: tcg_gen_exit_tb(NULL, 0); break; default: g_assert_not_reached(); } } -- 2.39.2
[RFC PATCH 20/78] target/mips: add fallthrough pseudo-keyword
In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- target/mips/sysemu/physaddr.c | 2 +- target/mips/tcg/micromips_translate.c.inc | 4 +- target/mips/tcg/mips16e_translate.c.inc | 30 - target/mips/tcg/mxu_translate.c | 8 +-- target/mips/tcg/nanomips_translate.c.inc | 4 +- target/mips/tcg/op_helper.c | 2 +- target/mips/tcg/translate.c | 79 --- 7 files changed, 66 insertions(+), 63 deletions(-) diff --git a/target/mips/sysemu/physaddr.c b/target/mips/sysemu/physaddr.c index 05990aa5bb..ebcaeea1bc 100644 --- a/target/mips/sysemu/physaddr.c +++ b/target/mips/sysemu/physaddr.c @@ -24,52 +24,52 @@ static int is_seg_am_mapped(unsigned int am, bool eu, int mmu_idx) { /* * Interpret access control mode and mmu_idx. * AdE? TLB? * AM K S U E K S U E * UK0 0 1 1 0 0 - - 0 * MK1 0 1 1 0 1 - - !eu * MSK 2 0 0 1 0 1 1 - !eu * MUSK 3 0 0 0 0 1 1 1 !eu * MUSUK 4 0 0 0 0 0 1 1 0 * USK 5 0 0 1 0 0 0 - 0 * - 6 - - - - - - - - * UUSK 7 0 0 0 0 0 0 0 0 */ int32_t adetlb_mask; switch (mmu_idx) { case 3: /* ERL */ /* If EU is set, always unmapped */ if (eu) { return 0; } -/* fall through */ +fallthrough; case MIPS_HFLAG_KM: /* Never AdE, TLB mapped if AM={1,2,3} */ adetlb_mask = 0x7000; goto check_tlb; case MIPS_HFLAG_SM: /* AdE if AM={0,1}, TLB mapped if AM={2,3,4} */ adetlb_mask = 0xc038; goto check_ade; case MIPS_HFLAG_UM: /* AdE if AM={0,1,2,5}, TLB mapped if AM={3,4} */ adetlb_mask = 0xe418; /* fall through */ check_ade: /* does this AM cause AdE in current execution mode */ if ((adetlb_mask << am) < 0) { return TLBRET_BADADDR; } adetlb_mask <<= 8; /* fall through */ check_tlb: /* is this AM mapped in current execution mode */ return ((adetlb_mask << am) < 0); default: g_assert_not_reached(); }; } diff --git a/target/mips/tcg/micromips_translate.c.inc b/target/mips/tcg/micromips_translate.c.inc index 7510831701..00e96ce27a 100644 --- a/target/mips/tcg/micromips_translate.c.inc +++ b/target/mips/tcg/micromips_translate.c.inc @@ -1621,1353 +1621,1353 @@ static void gen_pool32fxf(DisasContext *ctx, int rt, int rs) static void decode_micromips32_opc(CPUMIPSState *env, DisasContext *ctx) { int32_t offset; uint16_t insn; int rt, rs, rd, rr; int16_t imm; uint32_t op, minor, minor2, mips32_op; uint32_t cond, fmt, cc; insn = translator_lduw(env, &ctx->base, ctx->base.pc_next + 2); ctx->opcode = (ctx->opcode << 16) | insn; rt = (ctx->opcode >> 21) & 0x1f; rs = (ctx->opcode >> 16) & 0x1f; rd = (ctx->opcode >> 11) & 0x1f; rr = (ctx->opcode >> 6) & 0x1f; imm = (int16_t) ctx->opcode; op = (ctx->opcode >> 26) & 0x3f; switch (op) { case POOL32A: minor = ctx->opcode & 0x3f; switch (minor) { case 0x00: minor = (ctx->opcode >> 6) & 0xf; switch (minor) { case SLL32: mips32_op = OPC_SLL; goto do_shifti; case SRA: mips32_op = OPC_SRA; goto do_shifti; case SRL32: mips32_op = OPC_SRL; goto do_shifti; case ROTR: mips32_op = OPC_ROTR; do_shifti: gen_shift_imm(ctx, mips32_op, rt, rs, rd); break; case SELEQZ: check_insn(ctx, ISA_MIPS_R6); gen_cond_move(ctx, OPC_SELEQZ, rd, rs, rt); break; case SELNEZ: check_insn(ctx, ISA_MIPS_R6); gen_cond_move(ctx, OPC_SELNEZ, rd, rs, rt); break; case R6_RDHWR: check_insn(ctx, ISA_MIPS_R6); gen_rdhwr(ctx, rt, rs, extract32(ctx->opcode, 11, 3)); break; default: goto pool32a_invalid; } break; case 0x10: minor = (ctx->opcode >> 6) & 0xf; switch (minor) { /* Arithmetic */ case ADD: mips32_op = OPC_ADD; goto do_arith; case ADDU32: mips32_op = OPC_ADDU; goto do_arith; case SUB: mips32_op = OPC_SUB; goto do_arith; case SUBU32: mips32_op = OPC_SUBU; goto do
[RFC PATCH 17/78] ui/sdl2.c: add fallthrough pseudo-keyword
In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- ui/sdl2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/sdl2.c b/ui/sdl2.c index fbfdb64e90..3d157a14aa 100644 --- a/ui/sdl2.c +++ b/ui/sdl2.c @@ -576,78 +576,78 @@ static void handle_mousewheel(SDL_Event *ev) static void handle_windowevent(SDL_Event *ev) { struct sdl2_console *scon = get_scon_from_window(ev->window.windowID); bool allow_close = true; if (!scon) { return; } switch (ev->window.event) { case SDL_WINDOWEVENT_RESIZED: { QemuUIInfo info; memset(&info, 0, sizeof(info)); info.width = ev->window.data1; info.height = ev->window.data2; dpy_set_ui_info(scon->dcl.con, &info, true); } sdl2_redraw(scon); break; case SDL_WINDOWEVENT_EXPOSED: sdl2_redraw(scon); break; case SDL_WINDOWEVENT_FOCUS_GAINED: win32_kbd_set_grab(gui_grab); if (qemu_console_is_graphic(scon->dcl.con)) { win32_kbd_set_window(sdl2_win32_get_hwnd(scon)); } -/* fall through */ +fallthrough; case SDL_WINDOWEVENT_ENTER: if (!gui_grab && (qemu_input_is_absolute(scon->dcl.con) || absolute_enabled)) { absolute_mouse_grab(scon); } /* If a new console window opened using a hotkey receives the * focus, SDL sends another KEYDOWN event to the new window, * closing the console window immediately after. * * Work around this by ignoring further hotkey events until a * key is released. */ scon->ignore_hotkeys = get_mod_state(); break; case SDL_WINDOWEVENT_FOCUS_LOST: if (qemu_console_is_graphic(scon->dcl.con)) { win32_kbd_set_window(NULL); } if (gui_grab && !gui_fullscreen) { sdl_grab_end(scon); } break; case SDL_WINDOWEVENT_RESTORED: update_displaychangelistener(&scon->dcl, GUI_REFRESH_INTERVAL_DEFAULT); break; case SDL_WINDOWEVENT_MINIMIZED: update_displaychangelistener(&scon->dcl, 500); break; case SDL_WINDOWEVENT_CLOSE: if (qemu_console_is_graphic(scon->dcl.con)) { if (scon->opts->has_window_close && !scon->opts->window_close) { allow_close = false; } if (allow_close) { shutdown_action = SHUTDOWN_ACTION_POWEROFF; qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI); } } else { SDL_HideWindow(scon->real_window); scon->hidden = true; } break; case SDL_WINDOWEVENT_SHOWN: scon->hidden = false; break; case SDL_WINDOWEVENT_HIDDEN: scon->hidden = true; break; } } -- 2.39.2
[RFC PATCH v2 02/78] block: add fallthrough pseudo-keyword
In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- block/block-copy.c| 1 + block/file-posix.c| 1 + block/io.c| 1 + block/iscsi.c | 1 + block/qcow2-cluster.c | 5 - block/vhdx.c | 17 + 6 files changed, 21 insertions(+), 5 deletions(-) diff --git a/block/block-copy.c b/block/block-copy.c index 1c60368d72..b4ceb6a079 100644 --- a/block/block-copy.c +++ b/block/block-copy.c @@ -473,78 +473,79 @@ static int coroutine_fn GRAPH_RDLOCK block_copy_do_copy(BlockCopyState *s, int64_t offset, int64_t bytes, BlockCopyMethod *method, bool *error_is_read) { int ret; int64_t nbytes = MIN(offset + bytes, s->len) - offset; void *bounce_buffer = NULL; assert(offset >= 0 && bytes > 0 && INT64_MAX - offset >= bytes); assert(QEMU_IS_ALIGNED(offset, s->cluster_size)); assert(QEMU_IS_ALIGNED(bytes, s->cluster_size)); assert(offset < s->len); assert(offset + bytes <= s->len || offset + bytes == QEMU_ALIGN_UP(s->len, s->cluster_size)); assert(nbytes < INT_MAX); switch (*method) { case COPY_WRITE_ZEROES: ret = bdrv_co_pwrite_zeroes(s->target, offset, nbytes, s->write_flags & ~BDRV_REQ_WRITE_COMPRESSED); if (ret < 0) { trace_block_copy_write_zeroes_fail(s, offset, ret); *error_is_read = false; } return ret; case COPY_RANGE_SMALL: case COPY_RANGE_FULL: ret = bdrv_co_copy_range(s->source, offset, s->target, offset, nbytes, 0, s->write_flags); if (ret >= 0) { /* Successful copy-range, increase chunk size. */ *method = COPY_RANGE_FULL; return 0; } trace_block_copy_copy_range_fail(s, offset, ret); *method = COPY_READ_WRITE; /* Fall through to read+write with allocated buffer */ +fallthrough; case COPY_READ_WRITE_CLUSTER: case COPY_READ_WRITE: /* * In case of failed copy_range request above, we may proceed with * buffered request larger than BLOCK_COPY_MAX_BUFFER. * Still, further requests will be properly limited, so don't care too * much. Moreover the most likely case (copy_range is unsupported for * the configuration, so the very first copy_range request fails) * is handled by setting large copy_size only after first successful * copy_range. */ bounce_buffer = qemu_blockalign(s->source->bs, nbytes); ret = bdrv_co_pread(s->source, offset, nbytes, bounce_buffer, 0); if (ret < 0) { trace_block_copy_read_fail(s, offset, ret); *error_is_read = true; goto out; } ret = bdrv_co_pwrite(s->target, offset, nbytes, bounce_buffer, s->write_flags); if (ret < 0) { trace_block_copy_write_fail(s, offset, ret); *error_is_read = false; goto out; } out: qemu_vfree(bounce_buffer); break; default: abort(); } return ret; } diff --git a/block/file-posix.c b/block/file-posix.c index 50e2b20d5c..31c7719da5 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -972,69 +972,70 @@ static int raw_check_lock_bytes(int fd, uint64_t perm, uint64_t shared_perm, static int raw_handle_perm_lock(BlockDriverState *bs, RawPermLockOp op, uint64_t new_perm, uint64_t new_shared, Error **errp) { BDRVRawState *s = bs->opaque; int ret = 0; Error *local_err = NULL; if (!s->use_lock) { return 0; } if (bdrv_get_flags(bs) & BDRV_O_INACTIVE) { return 0; } switch (op) { case RAW_PL_PREPARE: if ((s->perm | new_perm) == s->perm && (s->shared_perm & new_shared) == s->shared_perm) { /* * We are going to unlock bytes, it should not fail. If it fail due * to some fs-dependent permission-unrelated reasons (which occurs * sometimes on NFS and leads to abort in bdrv_replace_child) we * can't prevent such errors by any check here. And we ignore them * anyway in ABORT and COMMIT. */ return 0; } ret = raw_apply_lock_bytes(s, s->fd, s->perm | new_perm, ~s->shared_perm | ~new_shared, false, errp); if (!ret) { ret = raw_check_lock_bytes(s->fd, new_perm, new_shared, errp); if (!ret) { return 0;
Re: [RFC PATCH 22/78] target/ppc: add fallthrough pseudo-keyword
On 10/13/23 09:47, Emmanouil Pitsidianakis wrote: In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- target/ppc/cpu_init.c| 8 target/ppc/excp_helper.c | 6 +++--- target/ppc/mmu-radix64.c | 6 +++--- target/ppc/mmu_common.c | 12 ++-- target/ppc/translate.c | 6 +++--- 5 files changed, 19 insertions(+), 19 deletions(-) Reviewed-by: Cédric Le Goater Thanks, C.
[RFC PATCH v2 10/78] hw/ide/atapi.c: add fallthrough pseudo-keyword
In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- hw/ide/atapi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c index dcc39df9a4..85c74a5ffe 100644 --- a/hw/ide/atapi.c +++ b/hw/ide/atapi.c @@ -1189,53 +1189,54 @@ static void cmd_read_disc_information(IDEState *s, uint8_t* buf) static void cmd_read_dvd_structure(IDEState *s, uint8_t* buf) { int max_len; int media = buf[1]; int format = buf[7]; int ret; max_len = lduw_be_p(buf + 8); if (format < 0xff) { if (media_is_cd(s)) { ide_atapi_cmd_error(s, ILLEGAL_REQUEST, ASC_INCOMPATIBLE_FORMAT); return; } else if (!media_present(s)) { ide_atapi_cmd_error(s, ILLEGAL_REQUEST, ASC_INV_FIELD_IN_CMD_PACKET); return; } } memset(buf, 0, max_len > IDE_DMA_BUF_SECTORS * BDRV_SECTOR_SIZE + 4 ? IDE_DMA_BUF_SECTORS * BDRV_SECTOR_SIZE + 4 : max_len); switch (format) { case 0x00 ... 0x7f: case 0xff: if (media == 0) { ret = ide_dvd_read_structure(s, format, buf, buf); if (ret < 0) { ide_atapi_cmd_error(s, ILLEGAL_REQUEST, -ret); } else { ide_atapi_cmd_reply(s, ret, max_len); } break; } /* TODO: BD support, fall through for now */ +fallthrough; /* Generic disk structures */ case 0x80: /* TODO: AACS volume identifier */ case 0x81: /* TODO: AACS media serial number */ case 0x82: /* TODO: AACS media identifier */ case 0x83: /* TODO: AACS media key block */ case 0x90: /* TODO: List of recognized format layers */ case 0xc0: /* TODO: Write protection status */ default: ide_atapi_cmd_error(s, ILLEGAL_REQUEST, ASC_INV_FIELD_IN_CMD_PACKET); break; } } -- 2.39.2
[RFC PATCH v2 53/78] nbd: add fallthrough pseudo-keyword
In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- nbd/client.c | 4 ++-- nbd/common.c | 2 +- qemu-nbd.c | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/nbd/client.c b/nbd/client.c index 29ffc609a4..04507249b2 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -1014,102 +1014,102 @@ static int nbd_negotiate_finish_oldstyle(QIOChannel *ioc, NBDExportInfo *info, /* * nbd_receive_negotiate: * Connect to server, complete negotiation, and move into transmission phase. * Returns: negative errno: failure talking to server * 0: server is connected */ int nbd_receive_negotiate(QIOChannel *ioc, QCryptoTLSCreds *tlscreds, const char *hostname, QIOChannel **outioc, NBDExportInfo *info, Error **errp) { ERRP_GUARD(); int result; bool zeroes; bool base_allocation = info->base_allocation; assert(info->name && strlen(info->name) <= NBD_MAX_STRING_SIZE); trace_nbd_receive_negotiate_name(info->name); result = nbd_start_negotiate(ioc, tlscreds, hostname, outioc, info->mode, &zeroes, errp); if (result < 0) { return result; } info->mode = result; info->base_allocation = false; if (tlscreds && *outioc) { ioc = *outioc; } switch (info->mode) { case NBD_MODE_EXTENDED: case NBD_MODE_STRUCTURED: if (base_allocation) { result = nbd_negotiate_simple_meta_context(ioc, info, errp); if (result < 0) { return -EINVAL; } info->base_allocation = result == 1; } -/* fall through */ +fallthrough; case NBD_MODE_SIMPLE: /* Try NBD_OPT_GO first - if it works, we are done (it * also gives us a good message if the server requires * TLS). If it is not available, fall back to * NBD_OPT_LIST for nicer error messages about a missing * export, then use NBD_OPT_EXPORT_NAME. */ result = nbd_opt_info_or_go(ioc, NBD_OPT_GO, info, errp); if (result < 0) { return -EINVAL; } if (result > 0) { return 0; } /* Check our desired export is present in the * server export list. Since NBD_OPT_EXPORT_NAME * cannot return an error message, running this * query gives us better error reporting if the * export name is not available. */ if (nbd_receive_query_exports(ioc, info->name, errp) < 0) { return -EINVAL; } -/* fall through */ +fallthrough; case NBD_MODE_EXPORT_NAME: /* write the export name request */ if (nbd_send_option_request(ioc, NBD_OPT_EXPORT_NAME, -1, info->name, errp) < 0) { return -EINVAL; } /* Read the response */ if (nbd_read64(ioc, &info->size, "export length", errp) < 0) { return -EINVAL; } if (nbd_read16(ioc, &info->flags, "export flags", errp) < 0) { return -EINVAL; } break; case NBD_MODE_OLDSTYLE: if (*info->name) { error_setg(errp, "Server does not support non-empty export names"); return -EINVAL; } if (nbd_negotiate_finish_oldstyle(ioc, info, errp) < 0) { return -EINVAL; } break; default: g_assert_not_reached(); } trace_nbd_receive_negotiate_size_flags(info->size, info->flags); if (zeroes && nbd_drop(ioc, 124, errp) < 0) { error_prepend(errp, "Failed to read reserved block: "); return -EINVAL; } return 0; } /* Clean up result of nbd_receive_export_list */ diff --git a/nbd/common.c b/nbd/common.c index 3247c1d618..1140ea0888 100644 --- a/nbd/common.c +++ b/nbd/common.c @@ -222,37 +222,37 @@ const char *nbd_err_lookup(int err) int nbd_errno_to_system_errno(int err) { int ret; switch (err) { case NBD_SUCCESS: ret = 0; break; case NBD_EPERM: ret = EPERM; break; case NBD_EIO: ret = EIO; break; case NBD_ENOMEM: ret = ENOMEM; break; case NBD_ENOSPC: ret = ENOSPC; break; case NBD_EOVERFLOW: ret = EOVERFLOW; break; case NBD_ENOTSUP: ret = ENOTSUP; break; case NBD_ESHUTDOWN: ret = ESHUTDOWN; break; default: trace_nbd_unknown_error(err); -/* fallthrough */ +fallthrough; case NBD_EINVAL: ret = EINVAL; break; } return ret; } diff --git a/qemu-nbd.c b/qemu-nbd.c index 186e6468b1..41e50208a5 100644 ---
Re: [RFC PATCH 42/75] hw/misc: add fallthrough pseudo-keyword
On 10/13/23 09:47, Emmanouil Pitsidianakis wrote: Signed-off-by: Emmanouil Pitsidianakis --- hw/misc/a9scu.c| 2 ++ hw/misc/aspeed_scu.c | 2 +- hw/misc/bcm2835_property.c | 12 ++-- hw/misc/mos6522.c | 4 ++-- 4 files changed, 11 insertions(+), 9 deletions(-) For aspeed, Reviewed-by: Cédric Le Goater Thanks, C. diff --git a/hw/misc/a9scu.c b/hw/misc/a9scu.c index a375ebc987..b422bec3c4 100644 --- a/hw/misc/a9scu.c +++ b/hw/misc/a9scu.c @@ -21,26 +21,27 @@ static uint64_t a9_scu_read(void *opaque, hwaddr offset, unsigned size) { A9SCUState *s = (A9SCUState *)opaque; switch (offset) { case 0x00: /* Control */ return s->control; case 0x04: /* Configuration */ return (((1 << s->num_cpu) - 1) << 4) | (s->num_cpu - 1); case 0x08: /* CPU Power Status */ return s->status; case 0x0c: /* Invalidate All Registers In Secure State */ return 0; case 0x40: /* Filtering Start Address Register */ case 0x44: /* Filtering End Address Register */ /* RAZ/WI, like an implementation with only one AXI master */ return 0; case 0x50: /* SCU Access Control Register */ case 0x54: /* SCU Non-secure Access Control Register */ /* unimplemented, fall through */ +fallthrough; default: qemu_log_mask(LOG_UNIMP, "%s: Unsupported offset 0x%"HWADDR_PRIx"\n", __func__, offset); return 0; } } @@ -48,31 +49,32 @@ static uint64_t a9_scu_read(void *opaque, hwaddr offset, static void a9_scu_write(void *opaque, hwaddr offset, uint64_t value, unsigned size) { A9SCUState *s = (A9SCUState *)opaque; switch (offset) { case 0x00: /* Control */ s->control = value & 1; break; case 0x4: /* Configuration: RO */ break; case 0x08: case 0x09: case 0x0A: case 0x0B: /* Power Control */ s->status = value; break; case 0x0c: /* Invalidate All Registers In Secure State */ /* no-op as we do not implement caches */ break; case 0x40: /* Filtering Start Address Register */ case 0x44: /* Filtering End Address Register */ /* RAZ/WI, like an implementation with only one AXI master */ break; case 0x50: /* SCU Access Control Register */ case 0x54: /* SCU Non-secure Access Control Register */ /* unimplemented, fall through */ +fallthrough; default: qemu_log_mask(LOG_UNIMP, "%s: Unsupported offset 0x%"HWADDR_PRIx " value 0x%"PRIx64"\n", __func__, offset, value); break; } } diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c index 8335364906..4a1ea2fa21 100644 --- a/hw/misc/aspeed_scu.c +++ b/hw/misc/aspeed_scu.c @@ -645,65 +645,65 @@ static uint64_t aspeed_ast2600_scu_read(void *opaque, hwaddr offset, static void aspeed_ast2600_scu_write(void *opaque, hwaddr offset, uint64_t data64, unsigned size) { AspeedSCUState *s = ASPEED_SCU(opaque); int reg = TO_REG(offset); /* Truncate here so bitwise operations below behave as expected */ uint32_t data = data64; if (reg >= ASPEED_AST2600_SCU_NR_REGS) { qemu_log_mask(LOG_GUEST_ERROR, "%s: Out-of-bounds write at offset 0x%" HWADDR_PRIx "\n", __func__, offset); return; } if (reg > PROT_KEY && !s->regs[PROT_KEY]) { qemu_log_mask(LOG_GUEST_ERROR, "%s: SCU is locked!\n", __func__); } trace_aspeed_scu_write(offset, size, data); switch (reg) { case AST2600_PROT_KEY: s->regs[reg] = (data == ASPEED_SCU_PROT_KEY) ? 1 : 0; return; case AST2600_HW_STRAP1: case AST2600_HW_STRAP2: if (s->regs[reg + 2]) { return; } -/* fall through */ +fallthrough; case AST2600_SYS_RST_CTRL: case AST2600_SYS_RST_CTRL2: case AST2600_CLK_STOP_CTRL: case AST2600_CLK_STOP_CTRL2: /* W1S (Write 1 to set) registers */ s->regs[reg] |= data; return; case AST2600_SYS_RST_CTRL_CLR: case AST2600_SYS_RST_CTRL2_CLR: case AST2600_CLK_STOP_CTRL_CLR: case AST2600_CLK_STOP_CTRL2_CLR: case AST2600_HW_STRAP1_CLR: case AST2600_HW_STRAP2_CLR: /* * W1C (Write 1 to clear) registers are offset by one address from * the data register */ s->regs[reg - 1] &= ~data; return; case AST2600_RNG_DATA: case AST2600_SILICON_REV: case AST2600_SILICON_REV2: case AST2600_CHIP_ID0: case AST2600_CHIP_ID1: /* Add read only registers here */ qemu_log_m
Re: [RFC PATCH 01/78] include/qemu/compiler.h: replace QEMU_FALLTHROUGH with fallthrough
On Fri, Oct 13, 2023 at 10:47:05AM +0300, Emmanouil Pitsidianakis wrote: > Signed-off-by: Emmanouil Pitsidianakis > --- > audio/pwaudio.c | 8 > hw/arm/smmuv3.c | 2 +- > include/qemu/compiler.h | 30 +++--- > include/qemu/osdep.h | 4 ++-- > target/loongarch/cpu.c | 4 ++-- > target/loongarch/translate.c | 2 +- > tcg/optimize.c | 8 > 7 files changed, 37 insertions(+), 21 deletions(-) This patch (and all the others in the series) have a ridiculously large context either side of the change. It makes this horrible to review as it requires wading through pages of pre-existing code trying to spot the change. Please send patches with the default git context lines setting. > > diff --git a/audio/pwaudio.c b/audio/pwaudio.c > index 3ce5f6507b..bf26fadb06 100644 > --- a/audio/pwaudio.c > +++ b/audio/pwaudio.c > @@ -1,29 +1,29 @@ > /* > * QEMU PipeWire audio driver > * > * Copyright (c) 2023 Red Hat Inc. > * > * Author: Dorinda Bassey > * > * SPDX-License-Identifier: GPL-2.0-or-later > */ > > +#include > +#include > +#include > +#include > #include "qemu/osdep.h" > #include "qemu/module.h" > #include "audio.h" > #include > #include "qemu/error-report.h" > #include "qapi/error.h" > -#include > -#include > -#include > -#include > > #include > #include "trace.h" > > #define AUDIO_CAP "pipewire" > #define RINGBUFFER_SIZE(1u << 22) > #define RINGBUFFER_MASK(RINGBUFFER_SIZE - 1) > > #include "audio_int.h" > diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c > index 6f2b2bd45f..545d82ff04 100644 > --- a/hw/arm/smmuv3.c > +++ b/hw/arm/smmuv3.c > @@ -1166,210 +1166,210 @@ smmuv3_invalidate_ste(gpointer key, gpointer value, > gpointer user_data) > static int smmuv3_cmdq_consume(SMMUv3State *s) > { > SMMUState *bs = ARM_SMMU(s); > SMMUCmdError cmd_error = SMMU_CERROR_NONE; > SMMUQueue *q = &s->cmdq; > SMMUCommandType type = 0; > > if (!smmuv3_cmdq_enabled(s)) { > return 0; > } > /* > * some commands depend on register values, typically CR0. In case those > * register values change while handling the command, spec says it > * is UNPREDICTABLE whether the command is interpreted under the new > * or old value. > */ > > while (!smmuv3_q_empty(q)) { > uint32_t pending = s->gerror ^ s->gerrorn; > Cmd cmd; > > trace_smmuv3_cmdq_consume(Q_PROD(q), Q_CONS(q), >Q_PROD_WRAP(q), Q_CONS_WRAP(q)); > > if (FIELD_EX32(pending, GERROR, CMDQ_ERR)) { > break; > } > > if (queue_read(q, &cmd) != MEMTX_OK) { > cmd_error = SMMU_CERROR_ABT; > break; > } > > type = CMD_TYPE(&cmd); > > trace_smmuv3_cmdq_opcode(smmu_cmd_string(type)); > > qemu_mutex_lock(&s->mutex); > switch (type) { > case SMMU_CMD_SYNC: > if (CMD_SYNC_CS(&cmd) & CMD_SYNC_SIG_IRQ) { > smmuv3_trigger_irq(s, SMMU_IRQ_CMD_SYNC, 0); > } > break; > case SMMU_CMD_PREFETCH_CONFIG: > case SMMU_CMD_PREFETCH_ADDR: > break; > case SMMU_CMD_CFGI_STE: > { > uint32_t sid = CMD_SID(&cmd); > IOMMUMemoryRegion *mr = smmu_iommu_mr(bs, sid); > SMMUDevice *sdev; > > if (CMD_SSEC(&cmd)) { > cmd_error = SMMU_CERROR_ILL; > break; > } > > if (!mr) { > break; > } > > trace_smmuv3_cmdq_cfgi_ste(sid); > sdev = container_of(mr, SMMUDevice, iommu); > smmuv3_flush_config(sdev); > > break; > } > case SMMU_CMD_CFGI_STE_RANGE: /* same as SMMU_CMD_CFGI_ALL */ > { > uint32_t sid = CMD_SID(&cmd), mask; > uint8_t range = CMD_STE_RANGE(&cmd); > SMMUSIDRange sid_range; > > if (CMD_SSEC(&cmd)) { > cmd_error = SMMU_CERROR_ILL; > break; > } > > mask = (1ULL << (range + 1)) - 1; > sid_range.start = sid & ~mask; > sid_range.end = sid_range.start + mask; > > trace_smmuv3_cmdq_cfgi_ste_range(sid_range.start, sid_range.end); > g_hash_table_foreach_remove(bs->configs, smmuv3_invalidate_ste, > &sid_range); > break; > } > case SMMU_CMD_CFGI_CD: > case SMMU_CMD_CFGI_CD_ALL: > { > uint32_t sid = CMD_SID(&cmd); > IOMMUMemoryRegion *mr = smmu_iommu_mr(bs, sid); > SMMUDevice *sdev; > > if (CMD_SSEC(&cmd)) { > cmd_error = SMMU_CERROR_ILL; >
Re: [RFC PATCH 13/78] hw/adc: add fallthrough pseudo-keyword
On 10/13/23 09:47, Emmanouil Pitsidianakis wrote: In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- hw/adc/aspeed_adc.c | 12 ++-- hw/adc/zynq-xadc.c | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) For aspeed, Reviewed-by: Cédric Le Goater Thanks, C.
Re: [RFC PATCH 00/78] Strict disable implicit fallthrough
On Fri, Oct 13, 2023 at 10:47:04AM +0300, Emmanouil Pitsidianakis wrote: > > Main questions this RFC poses > = > > - Is this change desirable and net-positive. Yes, IMHO it is worth standardizing on use of the attribute. The allowed use of comments was a nice thing by the compiler for coping with pre-existing code, but using the attribute is best long term for a consistent style. > - Should the `fallthrough;` pseudo-keyword be defined like in the Linux > kernel, or use glib's G_GNUC_FALLTHROUGH, or keep the already existing > QEMU_FALLTHROUGH macro. As a general rule, if glib provides functionality we aim o use that and not reinvent the wheel. IOW, we should just use G_GNUC_FALLTHROUGH. With 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 :|
[RFC PATCH 38/78] system/rtc.c: add fallthrough pseudo-keyword
In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- system/rtc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/rtc.c b/system/rtc.c index 4904581abe..bb406542c8 100644 --- a/system/rtc.c +++ b/system/rtc.c @@ -48,22 +48,22 @@ QEMUClockType rtc_clock; /***/ /* RTC reference time/date access */ static time_t qemu_ref_timedate(QEMUClockType clock) { time_t value = qemu_clock_get_ms(clock) / 1000; switch (clock) { case QEMU_CLOCK_REALTIME: value -= rtc_realtime_clock_offset; -/* fall through */ +fallthrough; case QEMU_CLOCK_VIRTUAL: value += rtc_ref_start_datetime; break; case QEMU_CLOCK_HOST: if (rtc_base_type == RTC_BASE_DATETIME) { value -= rtc_host_datetime_offset; } break; default: assert(0); } return value; } -- 2.39.2
[RFC PATCH 49/75] hw/audio: add fallthrough pseudo-keyword
Signed-off-by: Emmanouil Pitsidianakis --- hw/audio/cs4231a.c| 2 +- hw/audio/gusemu_hal.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/audio/cs4231a.c b/hw/audio/cs4231a.c index 3aa105748d..3bf0116c68 100644 --- a/hw/audio/cs4231a.c +++ b/hw/audio/cs4231a.c @@ -272,90 +272,90 @@ static void cs_audio_callback (void *opaque, int free) static void cs_reset_voices (CSState *s, uint32_t val) { int xtal; struct audsettings as; IsaDmaClass *k = ISADMA_GET_CLASS(s->isa_dma); #ifdef DEBUG_XLAW if (val == 0 || val == 32) val = (1 << 4) | (1 << 5); #endif xtal = val & 1; as.freq = freqs[xtal][(val >> 1) & 7]; if (as.freq == -1) { lerr ("unsupported frequency (val=%#x)\n", val); goto error; } as.nchannels = (val & (1 << 4)) ? 2 : 1; as.endianness = 0; s->tab = NULL; switch ((val >> 5) & ((s->dregs[MODE_And_ID] & MODE2) ? 7 : 3)) { case 0: as.fmt = AUDIO_FORMAT_U8; s->shift = as.nchannels == 2; break; case 1: s->tab = MuLawDecompressTable; goto x_law; case 3: s->tab = ALawDecompressTable; x_law: as.fmt = AUDIO_FORMAT_S16; as.endianness = AUDIO_HOST_ENDIANNESS; s->shift = as.nchannels == 2; break; case 6: as.endianness = 1; -/* fall through */ +fallthrough; case 2: as.fmt = AUDIO_FORMAT_S16; s->shift = as.nchannels; break; case 7: case 4: lerr ("attempt to use reserved format value (%#x)\n", val); goto error; case 5: lerr ("ADPCM 4 bit IMA compatible format is not supported\n"); goto error; } s->voice = AUD_open_out ( &s->card, s->voice, "cs4231a", s, cs_audio_callback, &as ); if (s->dregs[Interface_Configuration] & PEN) { if (!s->dma_running) { k->hold_DREQ(s->isa_dma, s->dma); AUD_set_active_out (s->voice, 1); s->transferred = 0; } s->dma_running = 1; } else { if (s->dma_running) { k->release_DREQ(s->isa_dma, s->dma); AUD_set_active_out (s->voice, 0); } s->dma_running = 0; } return; error: if (s->dma_running) { k->release_DREQ(s->isa_dma, s->dma); AUD_set_active_out (s->voice, 0); } } diff --git a/hw/audio/gusemu_hal.c b/hw/audio/gusemu_hal.c index f159978b49..76dd906ea1 100644 --- a/hw/audio/gusemu_hal.c +++ b/hw/audio/gusemu_hal.c @@ -190,311 +190,311 @@ unsigned int gus_read(GUSEmuState * state, int port, int size) void gus_write(GUSEmuState * state, int port, int size, unsigned int data) { uint8_t*gusptr; gusptr = state->gusdatapos; GUSregd(portaccesses)++; switch (port & 0xff0f) { case 0x200: /* MixerCtrlReg */ GUSregb(MixerCtrlReg2x0) = (uint8_t) data; break; case 0x206: /* IRQstatReg / SB2x6IRQ */ if (GUSregb(GUS45TimerCtrl) & 0x20) /* SB IRQ enabled? -> set 2x6IRQ bit */ { GUSregb(TimerStatus2x8) |= 0x08; GUSregb(IRQStatReg2x6) = 0x10; GUS_irqrequest(state, state->gusirq, 1); } break; case 0x308:/* AdLib 388h */ case 0x208:/* AdLibCommandReg */ GUSregb(AdLibCommand2xA) = (uint8_t) data; break; case 0x309:/* AdLib 389h */ case 0x209:/* AdLibDataReg */ if ((GUSregb(AdLibCommand2xA) == 0x04) && (!(GUSregb(GUS45TimerCtrl) & 1))) /* GUS auto timer mode enabled? */ { if (data & 0x80) GUSregb(TimerStatus2x8) &= 0x1f; /* AdLib IRQ reset? -> clear maskable adl. timer int regs */ else GUSregb(TimerDataReg2x9) = (uint8_t) data; } else { GUSregb(AdLibData2x9) = (uint8_t) data; if (GUSregb(GUS45TimerCtrl) & 0x02) { GUSregb(TimerStatus2x8) |= 0x01; GUSregb(IRQStatReg2x6) = 0x10; GUS_irqrequest(state, state->gusirq, 1); } } break; case 0x20A: GUSregb(AdLibStatus2x8) = (uint8_t) data; break; /* AdLibStatus2x8 */ case 0x20B:/* GUS hidden registers */ switch (GUSregb(RegCtrl_2xF) & 0x7) { case 0: if (GUSregb(MixerCtrlReg2x0) & 0x40) GUSregb(IRQ_2xB) = (uint8_t) data; /* control register select bit */ else GUSregb(DMA_2xB) = (uint8_t) data; break; /* case 1-4: general purpose emulation regs */ case 5:/* clear
[RFC PATCH 27/78] target/riscv: add fallthrough pseudo-keyword
In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- target/riscv/insn_trans/trans_rvi.c.inc | 2 +- target/riscv/insn_trans/trans_rvzce.c.inc | 22 +++--- target/riscv/translate.c | 4 ++-- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc index 25cb60558a..98dd2e3cf6 100644 --- a/target/riscv/insn_trans/trans_rvi.c.inc +++ b/target/riscv/insn_trans/trans_rvi.c.inc @@ -89,61 +89,61 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a) static TCGCond gen_compare_i128(bool bz, TCGv rl, TCGv al, TCGv ah, TCGv bl, TCGv bh, TCGCond cond) { TCGv rh = tcg_temp_new(); bool invert = false; switch (cond) { case TCG_COND_EQ: case TCG_COND_NE: if (bz) { tcg_gen_or_tl(rl, al, ah); } else { tcg_gen_xor_tl(rl, al, bl); tcg_gen_xor_tl(rh, ah, bh); tcg_gen_or_tl(rl, rl, rh); } break; case TCG_COND_GE: case TCG_COND_LT: if (bz) { tcg_gen_mov_tl(rl, ah); } else { TCGv tmp = tcg_temp_new(); tcg_gen_sub2_tl(rl, rh, al, ah, bl, bh); tcg_gen_xor_tl(rl, rh, ah); tcg_gen_xor_tl(tmp, ah, bh); tcg_gen_and_tl(rl, rl, tmp); tcg_gen_xor_tl(rl, rh, rl); } break; case TCG_COND_LTU: invert = true; -/* fallthrough */ +fallthrough; case TCG_COND_GEU: { TCGv tmp = tcg_temp_new(); TCGv zero = tcg_constant_tl(0); TCGv one = tcg_constant_tl(1); cond = TCG_COND_NE; /* borrow in to second word */ tcg_gen_setcond_tl(TCG_COND_LTU, tmp, al, bl); /* seed third word with 1, which will be result */ tcg_gen_sub2_tl(tmp, rh, ah, one, tmp, zero); tcg_gen_sub2_tl(tmp, rl, tmp, rh, bh, zero); } break; default: g_assert_not_reached(); } if (invert) { cond = tcg_invert_cond(cond); } return cond; } diff --git a/target/riscv/insn_trans/trans_rvzce.c.inc b/target/riscv/insn_trans/trans_rvzce.c.inc index 2d992e14c4..f0bcbb4f72 100644 --- a/target/riscv/insn_trans/trans_rvzce.c.inc +++ b/target/riscv/insn_trans/trans_rvzce.c.inc @@ -116,52 +116,52 @@ static bool trans_c_sh(DisasContext *ctx, arg_c_sh *a) static uint32_t decode_push_pop_list(DisasContext *ctx, target_ulong rlist) { uint32_t reg_bitmap = 0; if (has_ext(ctx, RVE) && rlist > 6) { return 0; } switch (rlist) { case 15: reg_bitmap |= 1 << (X_Sn + 11) ; reg_bitmap |= 1 << (X_Sn + 10) ; -/* FALL THROUGH */ +fallthrough; case 14: reg_bitmap |= 1 << (X_Sn + 9) ; -/* FALL THROUGH */ +fallthrough; case 13: reg_bitmap |= 1 << (X_Sn + 8) ; -/* FALL THROUGH */ +fallthrough; case 12: reg_bitmap |= 1 << (X_Sn + 7) ; -/* FALL THROUGH */ +fallthrough; case 11: reg_bitmap |= 1 << (X_Sn + 6) ; -/* FALL THROUGH */ +fallthrough; case 10: reg_bitmap |= 1 << (X_Sn + 5) ; -/* FALL THROUGH */ +fallthrough; case 9: reg_bitmap |= 1 << (X_Sn + 4) ; -/* FALL THROUGH */ +fallthrough; case 8: reg_bitmap |= 1 << (X_Sn + 3) ; -/* FALL THROUGH */ +fallthrough; case 7: reg_bitmap |= 1 << (X_Sn + 2) ; -/* FALL THROUGH */ +fallthrough; case 6: reg_bitmap |= 1 << X_S1 ; -/* FALL THROUGH */ +fallthrough; case 5: reg_bitmap |= 1 << X_S0; -/* FALL THROUGH */ +fallthrough; case 4: reg_bitmap |= 1 << xRA; break; default: break; } return reg_bitmap; } diff --git a/target/riscv/translate.c b/target/riscv/translate.c index f0be79bb16..c99e513221 100644 --- a/target/riscv/translate.c +++ b/target/riscv/translate.c @@ -431,26 +431,26 @@ static void gen_set_gpr128(DisasContext *ctx, int reg_num, TCGv rl, TCGv rh) static TCGv_i64 get_fpr_hs(DisasContext *ctx, int reg_num) { if (!ctx->cfg_ptr->ext_zfinx) { return cpu_fpr[reg_num]; } if (reg_num == 0) { return tcg_constant_i64(0); } switch (get_xl(ctx)) { case MXL_RV32: #ifdef TARGET_RISCV32 { TCGv_i64 t = tcg_temp_new_i64(); tcg_gen_ext_i32_i64(t, cpu_gpr[reg_num]); return t; } #else -/* fall through */ +fallthrough; case MXL_RV64: return cpu_gpr[reg_nu
[RFC PATCH v2 08/78] hw/block: add fallthrough pseudo-keyword
In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- hw/block/dataplane/xen-block.c | 4 ++-- hw/block/m25p80.c | 2 +- hw/block/onenand.c | 2 +- hw/block/pflash_cfi01.c| 1 + hw/block/pflash_cfi02.c| 6 -- 5 files changed, 9 insertions(+), 6 deletions(-) diff --git a/hw/block/dataplane/xen-block.c b/hw/block/dataplane/xen-block.c index 3b6f2b0aa2..1ae25a73b2 100644 --- a/hw/block/dataplane/xen-block.c +++ b/hw/block/dataplane/xen-block.c @@ -144,59 +144,59 @@ static void xen_block_complete_request(XenBlockRequest *request) /* * translate request into iovec + start offset * do sanity checks along the way */ static int xen_block_parse_request(XenBlockRequest *request) { XenBlockDataPlane *dataplane = request->dataplane; size_t len; int i; switch (request->req.operation) { case BLKIF_OP_READ: break; case BLKIF_OP_FLUSH_DISKCACHE: request->presync = 1; if (!request->req.nr_segments) { return 0; } -/* fall through */ +fallthrough; case BLKIF_OP_WRITE: break; case BLKIF_OP_DISCARD: return 0; default: error_report("error: unknown operation (%d)", request->req.operation); goto err; }; if (request->req.operation != BLKIF_OP_READ && !blk_is_writable(dataplane->blk)) { error_report("error: write req for ro device"); goto err; } request->start = request->req.sector_number * dataplane->sector_size; for (i = 0; i < request->req.nr_segments; i++) { if (i == BLKIF_MAX_SEGMENTS_PER_REQUEST) { error_report("error: nr_segments too big"); goto err; } if (request->req.seg[i].first_sect > request->req.seg[i].last_sect) { error_report("error: first > last sector"); goto err; } if (request->req.seg[i].last_sect * dataplane->sector_size >= XEN_PAGE_SIZE) { error_report("error: page crossing"); goto err; } len = (request->req.seg[i].last_sect - request->req.seg[i].first_sect + 1) * dataplane->sector_size; request->size += len; } if (request->start + request->size > blk_getlength(dataplane->blk)) { error_report("error: access beyond end of file"); goto err; } return 0; @@ -257,63 +257,63 @@ static int xen_block_do_aio(XenBlockRequest *request); static void xen_block_complete_aio(void *opaque, int ret) { XenBlockRequest *request = opaque; XenBlockDataPlane *dataplane = request->dataplane; aio_context_acquire(dataplane->ctx); if (ret != 0) { error_report("%s I/O error", request->req.operation == BLKIF_OP_READ ? "read" : "write"); request->aio_errors++; } request->aio_inflight--; if (request->presync) { request->presync = 0; xen_block_do_aio(request); goto done; } if (request->aio_inflight > 0) { goto done; } switch (request->req.operation) { case BLKIF_OP_READ: /* in case of failure request->aio_errors is increased */ if (ret == 0) { xen_block_copy_request(request); } break; case BLKIF_OP_WRITE: case BLKIF_OP_FLUSH_DISKCACHE: default: break; } request->status = request->aio_errors ? BLKIF_RSP_ERROR : BLKIF_RSP_OKAY; switch (request->req.operation) { case BLKIF_OP_WRITE: case BLKIF_OP_FLUSH_DISKCACHE: if (!request->req.nr_segments) { break; } -/* fall through */ +fallthrough; case BLKIF_OP_READ: if (request->status == BLKIF_RSP_OKAY) { block_acct_done(blk_get_stats(dataplane->blk), &request->acct); } else { block_acct_failed(blk_get_stats(dataplane->blk), &request->acct); } break; case BLKIF_OP_DISCARD: default: break; } xen_block_complete_request(request); if (dataplane->more_work) { qemu_bh_schedule(dataplane->bh); } diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c index afc3fdf4d6..523c34da71 100644 --- a/hw/block/m25p80.c +++ b/hw/block/m25p80.c @@ -1117,360 +1117,360 @@ static bool is_valid_aai_cmd(uint32_t cmd) static void decode_new_cmd(Flash *s, uint32_t value) { int i; s->cmd_in_progress = value; trace_m25p80_command_decoded(s, value); if (value != RESET_MEMORY) { s->reset_enable = false; } if (get_man(s) == MAN_SST && s->aai_enable && !is_valid_aai_cmd(value)) { qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Invalid cmd within
[RFC PATCH 55/78] hw/display: add fallthrough pseudo-keyword
In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- hw/display/cg3.c| 2 +- hw/display/cirrus_vga.c | 2 +- hw/display/tcx.c| 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/display/cg3.c b/hw/display/cg3.c index 2e9656ae1c..53eb9831b2 100644 --- a/hw/display/cg3.c +++ b/hw/display/cg3.c @@ -199,65 +199,65 @@ static uint64_t cg3_reg_read(void *opaque, hwaddr addr, unsigned size) static void cg3_reg_write(void *opaque, hwaddr addr, uint64_t val, unsigned size) { CG3State *s = opaque; uint8_t regval; int i; trace_cg3_write(addr, val, size); switch (addr) { case CG3_REG_BT458_ADDR: s->dac_index = val; s->dac_state = 0; break; case CG3_REG_BT458_COLMAP: /* This register can be written to as either a long word or a byte */ if (size == 1) { val <<= 24; } for (i = 0; i < size; i++) { regval = val >> 24; switch (s->dac_state) { case 0: s->r[s->dac_index] = regval; s->dac_state++; break; case 1: s->g[s->dac_index] = regval; s->dac_state++; break; case 2: s->b[s->dac_index] = regval; /* Index autoincrement */ s->dac_index = (s->dac_index + 1) & 0xff; -/* fall through */ +fallthrough; default: s->dac_state = 0; break; } val <<= 8; } s->full_update = 1; break; case CG3_REG_FBC_CTRL: s->regs[0] = val; break; case CG3_REG_FBC_STATUS: if (s->regs[1] & CG3_SR_PENDING_INT) { /* clear interrupt */ s->regs[1] &= ~CG3_SR_PENDING_INT; qemu_irq_lower(s->irq); } break; case CG3_REG_FBC_CURSTART ... CG3_REG_SIZE - 1: s->regs[addr - 0x10] = val; break; default: qemu_log_mask(LOG_UNIMP, "cg3: Unimplemented register write " "reg 0x%" HWADDR_PRIx " size 0x%x value 0x%" PRIx64 "\n", addr, size, val); break; } } diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c index b80f98b6c4..f1513a084c 100644 --- a/hw/display/cirrus_vga.c +++ b/hw/display/cirrus_vga.c @@ -1319,97 +1319,97 @@ static int cirrus_vga_read_sr(CirrusVGAState * s) static void cirrus_vga_write_sr(CirrusVGAState * s, uint32_t val) { switch (s->vga.sr_index) { case 0x00: // Standard VGA case 0x01: // Standard VGA case 0x02: // Standard VGA case 0x03: // Standard VGA case 0x04: // Standard VGA s->vga.sr[s->vga.sr_index] = val & sr_mask[s->vga.sr_index]; if (s->vga.sr_index == 1) s->vga.update_retrace_info(&s->vga); break; case 0x06: // Unlock Cirrus extensions val &= 0x17; if (val == 0x12) { s->vga.sr[s->vga.sr_index] = 0x12; } else { s->vga.sr[s->vga.sr_index] = 0x0f; } break; case 0x10: case 0x30: case 0x50: case 0x70: // Graphics Cursor X case 0x90: case 0xb0: case 0xd0: case 0xf0: // Graphics Cursor X s->vga.sr[0x10] = val; s->vga.hw_cursor_x = (val << 3) | (s->vga.sr_index >> 5); break; case 0x11: case 0x31: case 0x51: case 0x71: // Graphics Cursor Y case 0x91: case 0xb1: case 0xd1: case 0xf1: // Graphics Cursor Y s->vga.sr[0x11] = val; s->vga.hw_cursor_y = (val << 3) | (s->vga.sr_index >> 5); break; case 0x07: // Extended Sequencer Mode cirrus_update_memory_access(s); -/* fall through */ +fallthrough; case 0x08: // EEPROM Control case 0x09: // Scratch Register 0 case 0x0a: // Scratch Register 1 case 0x0b: // VCLK 0 case 0x0c: // VCLK 1 case 0x0d: // VCLK 2 case 0x0e: // VCLK 3 case 0x0f: // DRAM Control case 0x13: // Graphics Cursor Pattern Address case 0x14: // Scratch Register 2 case 0x15: // Scratch Register 3 case 0x16: // Performance Tuning Register case 0x18: // Signature Generator Control case 0x19: // S
[RFC PATCH 42/75] hw/misc: add fallthrough pseudo-keyword
Signed-off-by: Emmanouil Pitsidianakis --- hw/misc/a9scu.c| 2 ++ hw/misc/aspeed_scu.c | 2 +- hw/misc/bcm2835_property.c | 12 ++-- hw/misc/mos6522.c | 4 ++-- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/hw/misc/a9scu.c b/hw/misc/a9scu.c index a375ebc987..b422bec3c4 100644 --- a/hw/misc/a9scu.c +++ b/hw/misc/a9scu.c @@ -21,26 +21,27 @@ static uint64_t a9_scu_read(void *opaque, hwaddr offset, unsigned size) { A9SCUState *s = (A9SCUState *)opaque; switch (offset) { case 0x00: /* Control */ return s->control; case 0x04: /* Configuration */ return (((1 << s->num_cpu) - 1) << 4) | (s->num_cpu - 1); case 0x08: /* CPU Power Status */ return s->status; case 0x0c: /* Invalidate All Registers In Secure State */ return 0; case 0x40: /* Filtering Start Address Register */ case 0x44: /* Filtering End Address Register */ /* RAZ/WI, like an implementation with only one AXI master */ return 0; case 0x50: /* SCU Access Control Register */ case 0x54: /* SCU Non-secure Access Control Register */ /* unimplemented, fall through */ +fallthrough; default: qemu_log_mask(LOG_UNIMP, "%s: Unsupported offset 0x%"HWADDR_PRIx"\n", __func__, offset); return 0; } } @@ -48,31 +49,32 @@ static uint64_t a9_scu_read(void *opaque, hwaddr offset, static void a9_scu_write(void *opaque, hwaddr offset, uint64_t value, unsigned size) { A9SCUState *s = (A9SCUState *)opaque; switch (offset) { case 0x00: /* Control */ s->control = value & 1; break; case 0x4: /* Configuration: RO */ break; case 0x08: case 0x09: case 0x0A: case 0x0B: /* Power Control */ s->status = value; break; case 0x0c: /* Invalidate All Registers In Secure State */ /* no-op as we do not implement caches */ break; case 0x40: /* Filtering Start Address Register */ case 0x44: /* Filtering End Address Register */ /* RAZ/WI, like an implementation with only one AXI master */ break; case 0x50: /* SCU Access Control Register */ case 0x54: /* SCU Non-secure Access Control Register */ /* unimplemented, fall through */ +fallthrough; default: qemu_log_mask(LOG_UNIMP, "%s: Unsupported offset 0x%"HWADDR_PRIx " value 0x%"PRIx64"\n", __func__, offset, value); break; } } diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c index 8335364906..4a1ea2fa21 100644 --- a/hw/misc/aspeed_scu.c +++ b/hw/misc/aspeed_scu.c @@ -645,65 +645,65 @@ static uint64_t aspeed_ast2600_scu_read(void *opaque, hwaddr offset, static void aspeed_ast2600_scu_write(void *opaque, hwaddr offset, uint64_t data64, unsigned size) { AspeedSCUState *s = ASPEED_SCU(opaque); int reg = TO_REG(offset); /* Truncate here so bitwise operations below behave as expected */ uint32_t data = data64; if (reg >= ASPEED_AST2600_SCU_NR_REGS) { qemu_log_mask(LOG_GUEST_ERROR, "%s: Out-of-bounds write at offset 0x%" HWADDR_PRIx "\n", __func__, offset); return; } if (reg > PROT_KEY && !s->regs[PROT_KEY]) { qemu_log_mask(LOG_GUEST_ERROR, "%s: SCU is locked!\n", __func__); } trace_aspeed_scu_write(offset, size, data); switch (reg) { case AST2600_PROT_KEY: s->regs[reg] = (data == ASPEED_SCU_PROT_KEY) ? 1 : 0; return; case AST2600_HW_STRAP1: case AST2600_HW_STRAP2: if (s->regs[reg + 2]) { return; } -/* fall through */ +fallthrough; case AST2600_SYS_RST_CTRL: case AST2600_SYS_RST_CTRL2: case AST2600_CLK_STOP_CTRL: case AST2600_CLK_STOP_CTRL2: /* W1S (Write 1 to set) registers */ s->regs[reg] |= data; return; case AST2600_SYS_RST_CTRL_CLR: case AST2600_SYS_RST_CTRL2_CLR: case AST2600_CLK_STOP_CTRL_CLR: case AST2600_CLK_STOP_CTRL2_CLR: case AST2600_HW_STRAP1_CLR: case AST2600_HW_STRAP2_CLR: /* * W1C (Write 1 to clear) registers are offset by one address from * the data register */ s->regs[reg - 1] &= ~data; return; case AST2600_RNG_DATA: case AST2600_SILICON_REV: case AST2600_SILICON_REV2: case AST2600_CHIP_ID0: case AST2600_CHIP_ID1: /* Add read only registers here */ qemu_log_mask(LOG_GUEST_ERROR, "%s: Write to read-only offset 0x%" HWADDR_PRIx "\n", __func__, offset); return; } s->regs[reg] = data; } diff --git a/hw/misc/bcm2835_property.c b/hw
Re: [RFC PATCH 07/78] hw/virtio/virtio-balloon.c: add fallthrough pseudo-keyword
On 13.10.23 09:47, Emmanouil Pitsidianakis wrote: In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- Reviewed-by: David Hildenbrand -- Cheers, David / dhildenb
[RFC PATCH v2 39/78] hw/scsi: add fallthrough pseudo-keyword
In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- hw/scsi/esp.c | 2 +- hw/scsi/megasas.c | 2 +- hw/scsi/scsi-bus.c | 4 ++-- hw/scsi/scsi-disk.c | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c index 9b11d8c573..d6c8298f51 100644 --- a/hw/scsi/esp.c +++ b/hw/scsi/esp.c @@ -1022,130 +1022,130 @@ uint64_t esp_reg_read(ESPState *s, uint32_t saddr) void esp_reg_write(ESPState *s, uint32_t saddr, uint64_t val) { trace_esp_mem_writeb(saddr, s->wregs[saddr], val); switch (saddr) { case ESP_TCHI: s->tchi_written = true; -/* fall through */ +fallthrough; case ESP_TCLO: case ESP_TCMID: s->rregs[ESP_RSTAT] &= ~STAT_TC; break; case ESP_FIFO: if (s->do_cmd) { esp_fifo_push(&s->cmdfifo, val); /* * If any unexpected message out/command phase data is * transferred using non-DMA, raise the interrupt */ if (s->rregs[ESP_CMD] == CMD_TI) { s->rregs[ESP_RINTR] |= INTR_BS; esp_raise_irq(s); } } else { esp_fifo_push(&s->fifo, val); } break; case ESP_CMD: s->rregs[saddr] = val; if (val & CMD_DMA) { s->dma = 1; /* Reload DMA counter. */ if (esp_get_stc(s) == 0) { esp_set_tc(s, 0x1); } else { esp_set_tc(s, esp_get_stc(s)); } } else { s->dma = 0; } switch (val & CMD_CMD) { case CMD_NOP: trace_esp_mem_writeb_cmd_nop(val); break; case CMD_FLUSH: trace_esp_mem_writeb_cmd_flush(val); fifo8_reset(&s->fifo); break; case CMD_RESET: trace_esp_mem_writeb_cmd_reset(val); esp_soft_reset(s); break; case CMD_BUSRESET: trace_esp_mem_writeb_cmd_bus_reset(val); esp_bus_reset(s); if (!(s->wregs[ESP_CFG1] & CFG1_RESREPT)) { s->rregs[ESP_RINTR] |= INTR_RST; esp_raise_irq(s); } break; case CMD_TI: trace_esp_mem_writeb_cmd_ti(val); handle_ti(s); break; case CMD_ICCS: trace_esp_mem_writeb_cmd_iccs(val); write_response(s); s->rregs[ESP_RINTR] |= INTR_FC; s->rregs[ESP_RSTAT] |= STAT_MI; break; case CMD_MSGACC: trace_esp_mem_writeb_cmd_msgacc(val); s->rregs[ESP_RINTR] |= INTR_DC; s->rregs[ESP_RSEQ] = 0; s->rregs[ESP_RFLAGS] = 0; esp_raise_irq(s); break; case CMD_PAD: trace_esp_mem_writeb_cmd_pad(val); s->rregs[ESP_RSTAT] = STAT_TC; s->rregs[ESP_RINTR] |= INTR_FC; s->rregs[ESP_RSEQ] = 0; break; case CMD_SATN: trace_esp_mem_writeb_cmd_satn(val); break; case CMD_RSTATN: trace_esp_mem_writeb_cmd_rstatn(val); break; case CMD_SEL: trace_esp_mem_writeb_cmd_sel(val); handle_s_without_atn(s); break; case CMD_SELATN: trace_esp_mem_writeb_cmd_selatn(val); handle_satn(s); break; case CMD_SELATNS: trace_esp_mem_writeb_cmd_selatns(val); handle_satn_stop(s); break; case CMD_ENSEL: trace_esp_mem_writeb_cmd_ensel(val); s->rregs[ESP_RINTR] = 0; break; case CMD_DISSEL: trace_esp_mem_writeb_cmd_dissel(val); s->rregs[ESP_RINTR] = 0; esp_raise_irq(s); break; default: trace_esp_error_unhandled_command(val); break; } break; case ESP_WBUSID ... ESP_WSYNO: break; case ESP_CFG1: case ESP_CFG2: case ESP_CFG3: case ESP_RES3: case ESP_RES4: s->rregs[saddr] = val; break; case ESP_WCCF ... ESP_WTEST: break; default: trace_esp_error_invalid_write(val, saddr); return; } s->wregs[saddr] = val; } diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c index 32c70c9e99..54e4d7c8b6 100644 --- a/hw/scsi/megasas.c +++ b/hw/scsi/megasas.c @@ -2084,113 +2084,113 @@ static int adp_reset_seq[] = {0x00, 0x04, 0x0b, 0x02, 0x07, 0x0d}; static void megasas_mmio_write(void *opaque, hwaddr addr, uint64_t val, unsigned size) { MegasasState *s = opaque; PCIDevice *pci_dev = PCI_DEVICE(s);
[RFC PATCH 50/78] hw/audio: add fallthrough pseudo-keyword
In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- hw/audio/asc.c| 2 +- hw/audio/cs4231a.c| 2 +- hw/audio/gusemu_hal.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/audio/asc.c b/hw/audio/asc.c index 0f36b4ce9b..336da09509 100644 --- a/hw/audio/asc.c +++ b/hw/audio/asc.c @@ -154,126 +154,126 @@ static uint8_t asc_fifo_get(ASCFIFOState *fs) static int generate_fifo(ASCState *s, int maxsamples) { int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); uint8_t *buf = s->mixbuf; int i, wcount = 0; while (wcount < maxsamples) { uint8_t val; int16_t d, f0, f1; int32_t t; int shift, filter; bool hasdata = false; for (i = 0; i < 2; i++) { ASCFIFOState *fs = &s->fifos[i]; switch (fs->extregs[ASC_EXTREGS_FIFOCTRL] & 0x83) { case 0x82: /* * CD-XA BRR mode: decompress 15 bytes into 28 16-bit * samples */ if (!fs->cnt) { val = 0x80; break; } if (fs->xa_cnt == -1) { /* Start of packet, get flags */ fs->xa_flags = asc_fifo_get(fs); fs->xa_cnt = 0; } shift = fs->xa_flags & 0xf; filter = fs->xa_flags >> 4; f0 = (int8_t)fs->extregs[ASC_EXTREGS_CDXA_DECOMP_FILT + (filter << 1) + 1]; f1 = (int8_t)fs->extregs[ASC_EXTREGS_CDXA_DECOMP_FILT + (filter << 1)]; if ((fs->xa_cnt & 1) == 0) { if (!fs->cnt) { val = 0x80; break; } fs->xa_val = asc_fifo_get(fs); d = (fs->xa_val & 0xf) << 12; } else { d = (fs->xa_val & 0xf0) << 8; } t = (d >> shift) + (((fs->xa_last[0] * f0) + (fs->xa_last[1] * f1) + 32) >> 6); if (t < -32768) { t = -32768; } else if (t > 32767) { t = 32767; } /* * CD-XA BRR generates 16-bit signed output, so convert to * 8-bit before writing to buffer. Does real hardware do the * same? */ val = (uint8_t)(t / 256) ^ 0x80; hasdata = true; fs->xa_cnt++; fs->xa_last[1] = fs->xa_last[0]; fs->xa_last[0] = (int16_t)t; if (fs->xa_cnt == 28) { /* End of packet */ fs->xa_cnt = -1; } break; default: -/* fallthrough */ +fallthrough; case 0x80: /* Raw mode */ if (fs->cnt) { val = asc_fifo_get(fs); hasdata = true; } else { val = 0x80; } break; } buf[wcount * 2 + i] = val; } if (!hasdata) { break; } wcount++; } /* * MacOS (un)helpfully leaves the FIFO engine running even when it has * finished writing out samples, but still expects the FIFO empty * interrupts to be generated for each FIFO cycle (without these interrupts * MacOS will freeze) */ if (s->fifos[0].cnt == 0 && s->fifos[1].cnt == 0) { if (!s->fifo_empty_ns) { /* FIFO has completed first empty cycle */ s->fifo_empty_ns = now; } else if (now > (s->fifo_empty_ns + ASC_FIFO_CYCLE_TIME)) { /* FIFO has completed entire cycle with no data */ s->fifos[0].int_status |= ASC_FIFO_STATUS_HALF_FULL | ASC_FIFO_STATUS_FULL_EMPTY; s->fifos[1].int_status |= ASC_FIFO_STATUS_HALF_FULL | ASC_FIFO_STATUS_FULL_EMPTY; s->fifo_empty_ns = now; asc_raise_irq(s); } } else { /* FIFO contains data, reset empty time */ s->fifo_empty_ns = 0; } return wcount; } diff --git a/hw/audio/cs4231a.c b/hw/audio/cs4231a.c index 3aa105748d..3bf0116c68 100644 --- a/hw/audio/cs4231a.c +++ b/hw/audio/cs4231a.c @@ -272,90 +272,90 @@ static void cs_audio_callback (void *opaque, int free) static void cs_reset_voices (CSState *s, uint32_t val) { int xtal; struct audsettings a
[RFC PATCH 54/78] hw/core: add fallthrough pseudo-keyword
In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- hw/core/loader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/core/loader.c b/hw/core/loader.c index 4dd5a71fb7..559d63a1e2 100644 --- a/hw/core/loader.c +++ b/hw/core/loader.c @@ -625,138 +625,138 @@ toosmall: /* Load a U-Boot image. */ static ssize_t load_uboot_image(const char *filename, hwaddr *ep, hwaddr *loadaddr, int *is_linux, uint8_t image_type, uint64_t (*translate_fn)(void *, uint64_t), void *translate_opaque, AddressSpace *as) { int fd; ssize_t size; hwaddr address; uboot_image_header_t h; uboot_image_header_t *hdr = &h; uint8_t *data = NULL; int ret = -1; int do_uncompress = 0; fd = open(filename, O_RDONLY | O_BINARY); if (fd < 0) return -1; size = read(fd, hdr, sizeof(uboot_image_header_t)); if (size < sizeof(uboot_image_header_t)) { goto out; } bswap_uboot_header(hdr); if (hdr->ih_magic != IH_MAGIC) goto out; if (hdr->ih_type != image_type) { if (!(image_type == IH_TYPE_KERNEL && hdr->ih_type == IH_TYPE_KERNEL_NOLOAD)) { fprintf(stderr, "Wrong image type %d, expected %d\n", hdr->ih_type, image_type); goto out; } } /* TODO: Implement other image types. */ switch (hdr->ih_type) { case IH_TYPE_KERNEL_NOLOAD: if (!loadaddr || *loadaddr == LOAD_UIMAGE_LOADADDR_INVALID) { fprintf(stderr, "this image format (kernel_noload) cannot be " "loaded on this machine type"); goto out; } hdr->ih_load = *loadaddr + sizeof(*hdr); hdr->ih_ep += hdr->ih_load; -/* fall through */ +fallthrough; case IH_TYPE_KERNEL: address = hdr->ih_load; if (translate_fn) { address = translate_fn(translate_opaque, address); } if (loadaddr) { *loadaddr = hdr->ih_load; } switch (hdr->ih_comp) { case IH_COMP_NONE: break; case IH_COMP_GZIP: do_uncompress = 1; break; default: fprintf(stderr, "Unable to load u-boot images with compression type %d\n", hdr->ih_comp); goto out; } if (ep) { *ep = hdr->ih_ep; } /* TODO: Check CPU type. */ if (is_linux) { if (hdr->ih_os == IH_OS_LINUX) { *is_linux = 1; } else if (hdr->ih_os == IH_OS_VXWORKS) { /* * VxWorks 7 uses the same boot interface as the Linux kernel * on Arm (64-bit only), PowerPC and RISC-V architectures. */ switch (hdr->ih_arch) { case IH_ARCH_ARM64: case IH_ARCH_PPC: case IH_ARCH_RISCV: *is_linux = 1; break; default: *is_linux = 0; break; } } else { *is_linux = 0; } } break; case IH_TYPE_RAMDISK: address = *loadaddr; break; default: fprintf(stderr, "Unsupported u-boot image type %d\n", hdr->ih_type); goto out; } data = g_malloc(hdr->ih_size); if (read(fd, data, hdr->ih_size) != hdr->ih_size) { fprintf(stderr, "Error reading file\n"); goto out; } if (do_uncompress) { uint8_t *compressed_data; size_t max_bytes; ssize_t bytes; compressed_data = data; max_bytes = UBOOT_MAX_GUNZIP_BYTES; data = g_malloc(max_bytes); bytes = gunzip(data, max_bytes, compressed_data, hdr->ih_size); g_free(compressed_data); if (bytes < 0) { fprintf(stderr, "Unable to decompress gzipped image!\n"); goto out; } hdr->ih_size = bytes; } rom_add_blob_fixed_as(filename, data, hdr->ih_size, address, as); ret = hdr->ih_size; -- 2.39.2
[RFC PATCH 53/78] nbd: add fallthrough pseudo-keyword
In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- nbd/client.c | 4 ++-- nbd/common.c | 2 +- qemu-nbd.c | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/nbd/client.c b/nbd/client.c index 29ffc609a4..04507249b2 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -1014,102 +1014,102 @@ static int nbd_negotiate_finish_oldstyle(QIOChannel *ioc, NBDExportInfo *info, /* * nbd_receive_negotiate: * Connect to server, complete negotiation, and move into transmission phase. * Returns: negative errno: failure talking to server * 0: server is connected */ int nbd_receive_negotiate(QIOChannel *ioc, QCryptoTLSCreds *tlscreds, const char *hostname, QIOChannel **outioc, NBDExportInfo *info, Error **errp) { ERRP_GUARD(); int result; bool zeroes; bool base_allocation = info->base_allocation; assert(info->name && strlen(info->name) <= NBD_MAX_STRING_SIZE); trace_nbd_receive_negotiate_name(info->name); result = nbd_start_negotiate(ioc, tlscreds, hostname, outioc, info->mode, &zeroes, errp); if (result < 0) { return result; } info->mode = result; info->base_allocation = false; if (tlscreds && *outioc) { ioc = *outioc; } switch (info->mode) { case NBD_MODE_EXTENDED: case NBD_MODE_STRUCTURED: if (base_allocation) { result = nbd_negotiate_simple_meta_context(ioc, info, errp); if (result < 0) { return -EINVAL; } info->base_allocation = result == 1; } -/* fall through */ +fallthrough; case NBD_MODE_SIMPLE: /* Try NBD_OPT_GO first - if it works, we are done (it * also gives us a good message if the server requires * TLS). If it is not available, fall back to * NBD_OPT_LIST for nicer error messages about a missing * export, then use NBD_OPT_EXPORT_NAME. */ result = nbd_opt_info_or_go(ioc, NBD_OPT_GO, info, errp); if (result < 0) { return -EINVAL; } if (result > 0) { return 0; } /* Check our desired export is present in the * server export list. Since NBD_OPT_EXPORT_NAME * cannot return an error message, running this * query gives us better error reporting if the * export name is not available. */ if (nbd_receive_query_exports(ioc, info->name, errp) < 0) { return -EINVAL; } -/* fall through */ +fallthrough; case NBD_MODE_EXPORT_NAME: /* write the export name request */ if (nbd_send_option_request(ioc, NBD_OPT_EXPORT_NAME, -1, info->name, errp) < 0) { return -EINVAL; } /* Read the response */ if (nbd_read64(ioc, &info->size, "export length", errp) < 0) { return -EINVAL; } if (nbd_read16(ioc, &info->flags, "export flags", errp) < 0) { return -EINVAL; } break; case NBD_MODE_OLDSTYLE: if (*info->name) { error_setg(errp, "Server does not support non-empty export names"); return -EINVAL; } if (nbd_negotiate_finish_oldstyle(ioc, info, errp) < 0) { return -EINVAL; } break; default: g_assert_not_reached(); } trace_nbd_receive_negotiate_size_flags(info->size, info->flags); if (zeroes && nbd_drop(ioc, 124, errp) < 0) { error_prepend(errp, "Failed to read reserved block: "); return -EINVAL; } return 0; } /* Clean up result of nbd_receive_export_list */ diff --git a/nbd/common.c b/nbd/common.c index 3247c1d618..1140ea0888 100644 --- a/nbd/common.c +++ b/nbd/common.c @@ -222,37 +222,37 @@ const char *nbd_err_lookup(int err) int nbd_errno_to_system_errno(int err) { int ret; switch (err) { case NBD_SUCCESS: ret = 0; break; case NBD_EPERM: ret = EPERM; break; case NBD_EIO: ret = EIO; break; case NBD_ENOMEM: ret = ENOMEM; break; case NBD_ENOSPC: ret = ENOSPC; break; case NBD_EOVERFLOW: ret = EOVERFLOW; break; case NBD_ENOTSUP: ret = ENOTSUP; break; case NBD_ESHUTDOWN: ret = ESHUTDOWN; break; default: trace_nbd_unknown_error(err); -/* fallthrough */ +fallthrough; case NBD_EINVAL: ret = EINVAL; break; } return ret; } diff --git a/qemu-nbd.c b/qemu-nbd.c index 186e6468b1..41e50208a5 100644 ---
[RFC PATCH 40/78] hw/sd/sdhci.c: add fallthrough pseudo-keyword
In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- hw/sd/sdhci.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c index 5564765a9b..5c641d24de 100644 --- a/hw/sd/sdhci.c +++ b/hw/sd/sdhci.c @@ -75,138 +75,138 @@ static bool sdhci_check_capab_freq_range(SDHCIState *s, const char *desc, static void sdhci_check_capareg(SDHCIState *s, Error **errp) { uint64_t msk = s->capareg; uint32_t val; bool y; switch (s->sd_spec_version) { case 4: val = FIELD_EX64(s->capareg, SDHC_CAPAB, BUS64BIT_V4); trace_sdhci_capareg("64-bit system bus (v4)", val); msk = FIELD_DP64(msk, SDHC_CAPAB, BUS64BIT_V4, 0); val = FIELD_EX64(s->capareg, SDHC_CAPAB, UHS_II); trace_sdhci_capareg("UHS-II", val); msk = FIELD_DP64(msk, SDHC_CAPAB, UHS_II, 0); val = FIELD_EX64(s->capareg, SDHC_CAPAB, ADMA3); trace_sdhci_capareg("ADMA3", val); msk = FIELD_DP64(msk, SDHC_CAPAB, ADMA3, 0); -/* fallthrough */ +fallthrough; case 3: val = FIELD_EX64(s->capareg, SDHC_CAPAB, ASYNC_INT); trace_sdhci_capareg("async interrupt", val); msk = FIELD_DP64(msk, SDHC_CAPAB, ASYNC_INT, 0); val = FIELD_EX64(s->capareg, SDHC_CAPAB, SLOT_TYPE); if (val) { error_setg(errp, "slot-type not supported"); return; } trace_sdhci_capareg("slot type", val); msk = FIELD_DP64(msk, SDHC_CAPAB, SLOT_TYPE, 0); if (val != 2) { val = FIELD_EX64(s->capareg, SDHC_CAPAB, EMBEDDED_8BIT); trace_sdhci_capareg("8-bit bus", val); } msk = FIELD_DP64(msk, SDHC_CAPAB, EMBEDDED_8BIT, 0); val = FIELD_EX64(s->capareg, SDHC_CAPAB, BUS_SPEED); trace_sdhci_capareg("bus speed mask", val); msk = FIELD_DP64(msk, SDHC_CAPAB, BUS_SPEED, 0); val = FIELD_EX64(s->capareg, SDHC_CAPAB, DRIVER_STRENGTH); trace_sdhci_capareg("driver strength mask", val); msk = FIELD_DP64(msk, SDHC_CAPAB, DRIVER_STRENGTH, 0); val = FIELD_EX64(s->capareg, SDHC_CAPAB, TIMER_RETUNING); trace_sdhci_capareg("timer re-tuning", val); msk = FIELD_DP64(msk, SDHC_CAPAB, TIMER_RETUNING, 0); val = FIELD_EX64(s->capareg, SDHC_CAPAB, SDR50_TUNING); trace_sdhci_capareg("use SDR50 tuning", val); msk = FIELD_DP64(msk, SDHC_CAPAB, SDR50_TUNING, 0); val = FIELD_EX64(s->capareg, SDHC_CAPAB, RETUNING_MODE); trace_sdhci_capareg("re-tuning mode", val); msk = FIELD_DP64(msk, SDHC_CAPAB, RETUNING_MODE, 0); val = FIELD_EX64(s->capareg, SDHC_CAPAB, CLOCK_MULT); trace_sdhci_capareg("clock multiplier", val); msk = FIELD_DP64(msk, SDHC_CAPAB, CLOCK_MULT, 0); -/* fallthrough */ +fallthrough; case 2: /* default version */ val = FIELD_EX64(s->capareg, SDHC_CAPAB, ADMA2); trace_sdhci_capareg("ADMA2", val); msk = FIELD_DP64(msk, SDHC_CAPAB, ADMA2, 0); val = FIELD_EX64(s->capareg, SDHC_CAPAB, ADMA1); trace_sdhci_capareg("ADMA1", val); msk = FIELD_DP64(msk, SDHC_CAPAB, ADMA1, 0); val = FIELD_EX64(s->capareg, SDHC_CAPAB, BUS64BIT); trace_sdhci_capareg("64-bit system bus (v3)", val); msk = FIELD_DP64(msk, SDHC_CAPAB, BUS64BIT, 0); -/* fallthrough */ +fallthrough; case 1: y = FIELD_EX64(s->capareg, SDHC_CAPAB, TOUNIT); msk = FIELD_DP64(msk, SDHC_CAPAB, TOUNIT, 0); val = FIELD_EX64(s->capareg, SDHC_CAPAB, TOCLKFREQ); trace_sdhci_capareg(y ? "timeout (MHz)" : "Timeout (KHz)", val); if (sdhci_check_capab_freq_range(s, "timeout", val, errp)) { return; } msk = FIELD_DP64(msk, SDHC_CAPAB, TOCLKFREQ, 0); val = FIELD_EX64(s->capareg, SDHC_CAPAB, BASECLKFREQ); trace_sdhci_capareg(y ? "base (MHz)" : "Base (KHz)", val); if (sdhci_check_capab_freq_range(s, "base", val, errp)) { return; } msk = FIELD_DP64(msk, SDHC_CAPAB, BASECLKFREQ, 0); val = FIELD_EX64(s->capareg, SDHC_CAPAB, MAXBLOCKLENGTH); if (val >= 3) { error_setg(errp, "block size can be 512, 1024 or 2048 only"); return; } trace_sdhci_capareg("max block length", sdhci_get_fifolen(s)); msk = FIELD_DP64(msk, SDHC_CAPAB, MAXBLOCKLENGTH, 0); val = FIELD_EX64(s->capareg, SDHC_CAPAB, HIGHSPEED); trace_sdhci_capareg("high speed", val); msk = FIELD_DP64(msk, SDHC_CAPAB, HIGHSPEED, 0); val = FIELD_EX64(s->capareg, SDHC_CAPAB, SDMA); trace_sdhci_capareg("SDMA", val); msk = FIELD_DP64(msk, SDHC_CAPAB, SDMA, 0);
[RFC PATCH 49/78] hw/arm: add fallthrough pseudo-keyword
In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- hw/arm/omap1.c | 8 hw/arm/pxa2xx.c| 6 +++--- hw/arm/stellaris.c | 1 + 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/hw/arm/omap1.c b/hw/arm/omap1.c index d5438156ee..c54a4ec553 100644 --- a/hw/arm/omap1.c +++ b/hw/arm/omap1.c @@ -531,46 +531,46 @@ static struct omap_32khz_timer_s *omap_os_timer_init(MemoryRegion *memory, /* Ultra Low-Power Device Module */ static uint64_t omap_ulpd_pm_read(void *opaque, hwaddr addr, unsigned size) { struct omap_mpu_state_s *s = opaque; uint16_t ret; if (size != 2) { return omap_badwidth_read16(opaque, addr); } switch (addr) { case 0x14: /* IT_STATUS */ ret = s->ulpd_pm_regs[addr >> 2]; s->ulpd_pm_regs[addr >> 2] = 0; qemu_irq_lower(qdev_get_gpio_in(s->ih[1], OMAP_INT_GAUGE_32K)); return ret; case 0x18: /* Reserved */ case 0x1c: /* Reserved */ case 0x20: /* Reserved */ case 0x28: /* Reserved */ case 0x2c: /* Reserved */ OMAP_BAD_REG(addr); -/* fall through */ +fallthrough; case 0x00: /* COUNTER_32_LSB */ case 0x04: /* COUNTER_32_MSB */ case 0x08: /* COUNTER_HIGH_FREQ_LSB */ case 0x0c: /* COUNTER_HIGH_FREQ_MSB */ case 0x10: /* GAUGING_CTRL */ case 0x24: /* SETUP_ANALOG_CELL3_ULPD1 */ case 0x30: /* CLOCK_CTRL */ case 0x34: /* SOFT_REQ */ case 0x38: /* COUNTER_32_FIQ */ case 0x3c: /* DPLL_CTRL */ case 0x40: /* STATUS_REQ */ /* XXX: check clk::usecount state for every clock */ case 0x48: /* LOCL_TIME */ case 0x4c: /* APLL_CTRL */ case 0x50: /* POWER_CTRL */ return s->ulpd_pm_regs[addr >> 2]; } OMAP_BAD_REG(addr); return 0; } @@ -600,120 +600,120 @@ static inline void omap_ulpd_req_update(struct omap_mpu_state_s *s, static void omap_ulpd_pm_write(void *opaque, hwaddr addr, uint64_t value, unsigned size) { struct omap_mpu_state_s *s = opaque; int64_t now, ticks; int div, mult; static const int bypass_div[4] = { 1, 2, 4, 4 }; uint16_t diff; if (size != 2) { omap_badwidth_write16(opaque, addr, value); return; } switch (addr) { case 0x00: /* COUNTER_32_LSB */ case 0x04: /* COUNTER_32_MSB */ case 0x08: /* COUNTER_HIGH_FREQ_LSB */ case 0x0c: /* COUNTER_HIGH_FREQ_MSB */ case 0x14: /* IT_STATUS */ case 0x40: /* STATUS_REQ */ OMAP_RO_REG(addr); break; case 0x10: /* GAUGING_CTRL */ /* Bits 0 and 1 seem to be confused in the OMAP 310 TRM */ if ((s->ulpd_pm_regs[addr >> 2] ^ value) & 1) { now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); if (value & 1) s->ulpd_gauge_start = now; else { now -= s->ulpd_gauge_start; /* 32-kHz ticks */ ticks = muldiv64(now, 32768, NANOSECONDS_PER_SECOND); s->ulpd_pm_regs[0x00 >> 2] = (ticks >> 0) & 0x; s->ulpd_pm_regs[0x04 >> 2] = (ticks >> 16) & 0x; if (ticks >> 32) /* OVERFLOW_32K */ s->ulpd_pm_regs[0x14 >> 2] |= 1 << 2; /* High frequency ticks */ ticks = muldiv64(now, 1200, NANOSECONDS_PER_SECOND); s->ulpd_pm_regs[0x08 >> 2] = (ticks >> 0) & 0x; s->ulpd_pm_regs[0x0c >> 2] = (ticks >> 16) & 0x; if (ticks >> 32) /* OVERFLOW_HI_FREQ */ s->ulpd_pm_regs[0x14 >> 2] |= 1 << 1; s->ulpd_pm_regs[0x14 >> 2] |= 1 << 0; /* IT_GAUGING */ qemu_irq_raise(qdev_get_gpio_in(s->ih[1], OMAP_INT_GAUGE_32K)); } } s->ulpd_pm_regs[addr >> 2] = value; break; case 0x18: /* Reserved */ case 0x1c: /* Reserved */ case 0x20: /* Reserved */ case 0x28: /* Reserved */ case 0x2c: /* Reserved */ OMAP_BAD_REG(addr); -/* fall through */ +fallthrough; case 0x24: /* SETUP_ANALOG_CELL3_ULPD1 */ case 0x38: /* COUNTER_32_FIQ */ case 0x48: /* LOCL_TIME */ case 0x50: /* POWER_CTRL */ s->ulpd_pm_regs[addr >> 2] = value; break; case 0x30: /* CLOCK_CTRL */ diff = s->ulpd_pm_regs[addr >> 2] ^ value; s->ulpd_pm_regs[addr >> 2] = value & 0x3f; omap_ulpd_clk_update(s, diff, value); break; case 0x34: /* SOFT_REQ */ diff = s->ulpd_pm_regs[addr >> 2] ^ value; s->ulpd_pm_regs[addr >> 2] = value & 0x1f; omap_ulpd_req_update(s, diff, value); break; case 0x3c: /* DPLL_CTRL */ /* XXX: OM
[RFC PATCH 24/78] target/alpha: add fallthrough pseudo-keyword
In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- target/alpha/helper.c| 6 +++--- target/alpha/translate.c | 4 +++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/target/alpha/helper.c b/target/alpha/helper.c index 970c869771..1afdc1beec 100644 --- a/target/alpha/helper.c +++ b/target/alpha/helper.c @@ -436,45 +436,45 @@ void alpha_cpu_do_interrupt(CPUState *cs) bool alpha_cpu_exec_interrupt(CPUState *cs, int interrupt_request) { AlphaCPU *cpu = ALPHA_CPU(cs); CPUAlphaState *env = &cpu->env; int idx = -1; /* We never take interrupts while in PALmode. */ if (env->flags & ENV_FLAG_PAL_MODE) { return false; } /* Fall through the switch, collecting the highest priority interrupt that isn't masked by the processor status IPL. */ /* ??? This hard-codes the OSF/1 interrupt levels. */ switch ((env->flags >> ENV_FLAG_PS_SHIFT) & PS_INT_MASK) { case 0 ... 3: if (interrupt_request & CPU_INTERRUPT_HARD) { idx = EXCP_DEV_INTERRUPT; } -/* FALLTHRU */ +fallthrough; case 4: if (interrupt_request & CPU_INTERRUPT_TIMER) { idx = EXCP_CLK_INTERRUPT; } -/* FALLTHRU */ +fallthrough; case 5: if (interrupt_request & CPU_INTERRUPT_SMP) { idx = EXCP_SMP_INTERRUPT; } -/* FALLTHRU */ +fallthrough; case 6: if (interrupt_request & CPU_INTERRUPT_MCHK) { idx = EXCP_MCHK; } } if (idx >= 0) { cs->exception_index = idx; env->error_code = 0; alpha_cpu_do_interrupt(cs); return true; } return false; } #endif /* !CONFIG_USER_ONLY */ diff --git a/target/alpha/translate.c b/target/alpha/translate.c index 32333081d8..19e1d2ed86 100644 --- a/target/alpha/translate.c +++ b/target/alpha/translate.c @@ -1377,1493 +1377,1493 @@ static DisasJumpType gen_mtpr(DisasContext *ctx, TCGv vb, int regno) static DisasJumpType translate_one(DisasContext *ctx, uint32_t insn) { int32_t disp21, disp16, disp12 __attribute__((unused)); uint16_t fn11; uint8_t opc, ra, rb, rc, fpfn, fn7, lit; bool islit, real_islit; TCGv va, vb, vc, tmp, tmp2; TCGv_i32 t32; DisasJumpType ret; /* Decode all instruction fields */ opc = extract32(insn, 26, 6); ra = extract32(insn, 21, 5); rb = extract32(insn, 16, 5); rc = extract32(insn, 0, 5); real_islit = islit = extract32(insn, 12, 1); lit = extract32(insn, 13, 8); disp21 = sextract32(insn, 0, 21); disp16 = sextract32(insn, 0, 16); disp12 = sextract32(insn, 0, 12); fn11 = extract32(insn, 5, 11); fpfn = extract32(insn, 5, 6); fn7 = extract32(insn, 5, 7); if (rb == 31 && !islit) { islit = true; lit = 0; } ret = DISAS_NEXT; switch (opc) { case 0x00: /* CALL_PAL */ ret = gen_call_pal(ctx, insn & 0x03ff); break; case 0x01: /* OPC01 */ goto invalid_opc; case 0x02: /* OPC02 */ goto invalid_opc; case 0x03: /* OPC03 */ goto invalid_opc; case 0x04: /* OPC04 */ goto invalid_opc; case 0x05: /* OPC05 */ goto invalid_opc; case 0x06: /* OPC06 */ goto invalid_opc; case 0x07: /* OPC07 */ goto invalid_opc; case 0x09: /* LDAH */ disp16 = (uint32_t)disp16 << 16; -/* fall through */ +fallthrough; case 0x08: /* LDA */ va = dest_gpr(ctx, ra); /* It's worth special-casing immediate loads. */ if (rb == 31) { tcg_gen_movi_i64(va, disp16); } else { tcg_gen_addi_i64(va, load_gpr(ctx, rb), disp16); } break; case 0x0A: /* LDBU */ REQUIRE_AMASK(BWX); gen_load_int(ctx, ra, rb, disp16, MO_UB, 0, 0); break; case 0x0B: /* LDQ_U */ gen_load_int(ctx, ra, rb, disp16, MO_LEUQ, 1, 0); break; case 0x0C: /* LDWU */ REQUIRE_AMASK(BWX); gen_load_int(ctx, ra, rb, disp16, MO_LEUW, 0, 0); break; case 0x0D: /* STW */ REQUIRE_AMASK(BWX); gen_store_int(ctx, ra, rb, disp16, MO_LEUW, 0); break; case 0x0E: /* STB */ REQUIRE_AMASK(BWX); gen_store_int(ctx, ra, rb, disp16, MO_UB, 0); break; case 0x0F: /* STQ_U */ gen_store_int(ctx, ra, rb, disp16, MO_LEUQ, 1); break; case 0x10: vc = dest_gpr(ctx, rc); vb = load_gpr_lit(ctx, rb, lit, islit); if (ra == 31) { if (fn7 == 0x00) {
[RFC PATCH 54/75] hw/display: add fallthrough pseudo-keyword
Signed-off-by: Emmanouil Pitsidianakis --- hw/display/cg3.c| 2 +- hw/display/cirrus_vga.c | 2 +- hw/display/tcx.c| 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/display/cg3.c b/hw/display/cg3.c index 2e9656ae1c..53eb9831b2 100644 --- a/hw/display/cg3.c +++ b/hw/display/cg3.c @@ -199,65 +199,65 @@ static uint64_t cg3_reg_read(void *opaque, hwaddr addr, unsigned size) static void cg3_reg_write(void *opaque, hwaddr addr, uint64_t val, unsigned size) { CG3State *s = opaque; uint8_t regval; int i; trace_cg3_write(addr, val, size); switch (addr) { case CG3_REG_BT458_ADDR: s->dac_index = val; s->dac_state = 0; break; case CG3_REG_BT458_COLMAP: /* This register can be written to as either a long word or a byte */ if (size == 1) { val <<= 24; } for (i = 0; i < size; i++) { regval = val >> 24; switch (s->dac_state) { case 0: s->r[s->dac_index] = regval; s->dac_state++; break; case 1: s->g[s->dac_index] = regval; s->dac_state++; break; case 2: s->b[s->dac_index] = regval; /* Index autoincrement */ s->dac_index = (s->dac_index + 1) & 0xff; -/* fall through */ +fallthrough; default: s->dac_state = 0; break; } val <<= 8; } s->full_update = 1; break; case CG3_REG_FBC_CTRL: s->regs[0] = val; break; case CG3_REG_FBC_STATUS: if (s->regs[1] & CG3_SR_PENDING_INT) { /* clear interrupt */ s->regs[1] &= ~CG3_SR_PENDING_INT; qemu_irq_lower(s->irq); } break; case CG3_REG_FBC_CURSTART ... CG3_REG_SIZE - 1: s->regs[addr - 0x10] = val; break; default: qemu_log_mask(LOG_UNIMP, "cg3: Unimplemented register write " "reg 0x%" HWADDR_PRIx " size 0x%x value 0x%" PRIx64 "\n", addr, size, val); break; } } diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c index b80f98b6c4..f1513a084c 100644 --- a/hw/display/cirrus_vga.c +++ b/hw/display/cirrus_vga.c @@ -1319,97 +1319,97 @@ static int cirrus_vga_read_sr(CirrusVGAState * s) static void cirrus_vga_write_sr(CirrusVGAState * s, uint32_t val) { switch (s->vga.sr_index) { case 0x00: // Standard VGA case 0x01: // Standard VGA case 0x02: // Standard VGA case 0x03: // Standard VGA case 0x04: // Standard VGA s->vga.sr[s->vga.sr_index] = val & sr_mask[s->vga.sr_index]; if (s->vga.sr_index == 1) s->vga.update_retrace_info(&s->vga); break; case 0x06: // Unlock Cirrus extensions val &= 0x17; if (val == 0x12) { s->vga.sr[s->vga.sr_index] = 0x12; } else { s->vga.sr[s->vga.sr_index] = 0x0f; } break; case 0x10: case 0x30: case 0x50: case 0x70: // Graphics Cursor X case 0x90: case 0xb0: case 0xd0: case 0xf0: // Graphics Cursor X s->vga.sr[0x10] = val; s->vga.hw_cursor_x = (val << 3) | (s->vga.sr_index >> 5); break; case 0x11: case 0x31: case 0x51: case 0x71: // Graphics Cursor Y case 0x91: case 0xb1: case 0xd1: case 0xf1: // Graphics Cursor Y s->vga.sr[0x11] = val; s->vga.hw_cursor_y = (val << 3) | (s->vga.sr_index >> 5); break; case 0x07: // Extended Sequencer Mode cirrus_update_memory_access(s); -/* fall through */ +fallthrough; case 0x08: // EEPROM Control case 0x09: // Scratch Register 0 case 0x0a: // Scratch Register 1 case 0x0b: // VCLK 0 case 0x0c: // VCLK 1 case 0x0d: // VCLK 2 case 0x0e: // VCLK 3 case 0x0f: // DRAM Control case 0x13: // Graphics Cursor Pattern Address case 0x14: // Scratch Register 2 case 0x15: // Scratch Register 3 case 0x16: // Performance Tuning Register case 0x18: // Signature Generator Control case 0x19: // Signature Generator Result case 0x1a: // Signature Generator Result case 0x1b: // VCLK 0 Denom
[RFC PATCH v2 65/78] hw/nvme: add fallthrough pseudo-keyword
In preparation of raising -Wimplicit-fallthrough to 5, replace all fall-through comments with the fallthrough attribute pseudo-keyword. Signed-off-by: Emmanouil Pitsidianakis --- hw/nvme/ctrl.c | 24 hw/nvme/dif.c | 4 ++-- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index f026245d1e..acb2012fb9 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -1912,29 +1912,29 @@ static uint16_t nvme_check_zone_read(NvmeNamespace *ns, uint64_t slba, static uint16_t nvme_zrm_finish(NvmeNamespace *ns, NvmeZone *zone) { switch (nvme_get_zone_state(zone)) { case NVME_ZONE_STATE_FULL: return NVME_SUCCESS; case NVME_ZONE_STATE_IMPLICITLY_OPEN: case NVME_ZONE_STATE_EXPLICITLY_OPEN: nvme_aor_dec_open(ns); -/* fallthrough */ +fallthrough; case NVME_ZONE_STATE_CLOSED: nvme_aor_dec_active(ns); if (zone->d.za & NVME_ZA_ZRWA_VALID) { zone->d.za &= ~NVME_ZA_ZRWA_VALID; if (ns->params.numzrwa) { ns->zns.numzrwa++; } } -/* fallthrough */ +fallthrough; case NVME_ZONE_STATE_EMPTY: nvme_assign_zone_state(ns, zone, NVME_ZONE_STATE_FULL); return NVME_SUCCESS; default: return NVME_ZONE_INVAL_TRANSITION; } } @@ -1942,15 +1942,15 @@ static uint16_t nvme_zrm_finish(NvmeNamespace *ns, NvmeZone *zone) static uint16_t nvme_zrm_close(NvmeNamespace *ns, NvmeZone *zone) { switch (nvme_get_zone_state(zone)) { case NVME_ZONE_STATE_EXPLICITLY_OPEN: case NVME_ZONE_STATE_IMPLICITLY_OPEN: nvme_aor_dec_open(ns); nvme_assign_zone_state(ns, zone, NVME_ZONE_STATE_CLOSED); -/* fall through */ +fallthrough; case NVME_ZONE_STATE_CLOSED: return NVME_SUCCESS; default: return NVME_ZONE_INVAL_TRANSITION; } } @@ -1958,29 +1958,29 @@ static uint16_t nvme_zrm_close(NvmeNamespace *ns, NvmeZone *zone) static uint16_t nvme_zrm_reset(NvmeNamespace *ns, NvmeZone *zone) { switch (nvme_get_zone_state(zone)) { case NVME_ZONE_STATE_EXPLICITLY_OPEN: case NVME_ZONE_STATE_IMPLICITLY_OPEN: nvme_aor_dec_open(ns); -/* fallthrough */ +fallthrough; case NVME_ZONE_STATE_CLOSED: nvme_aor_dec_active(ns); if (zone->d.za & NVME_ZA_ZRWA_VALID) { if (ns->params.numzrwa) { ns->zns.numzrwa++; } } -/* fallthrough */ +fallthrough; case NVME_ZONE_STATE_FULL: zone->w_ptr = zone->d.zslba; zone->d.wp = zone->w_ptr; nvme_assign_zone_state(ns, zone, NVME_ZONE_STATE_EMPTY); -/* fallthrough */ +fallthrough; case NVME_ZONE_STATE_EMPTY: return NVME_SUCCESS; default: return NVME_ZONE_INVAL_TRANSITION; } } @@ -2010,57 +2010,57 @@ enum { static uint16_t nvme_zrm_open_flags(NvmeCtrl *n, NvmeNamespace *ns, NvmeZone *zone, int flags) { int act = 0; uint16_t status; switch (nvme_get_zone_state(zone)) { case NVME_ZONE_STATE_EMPTY: act = 1; -/* fallthrough */ +fallthrough; case NVME_ZONE_STATE_CLOSED: if (n->params.auto_transition_zones) { nvme_zrm_auto_transition_zone(ns); } status = nvme_zns_check_resources(ns, act, 1, (flags & NVME_ZRM_ZRWA) ? 1 : 0); if (status) { return status; } if (act) { nvme_aor_inc_active(ns); } nvme_aor_inc_open(ns); if (flags & NVME_ZRM_AUTO) { nvme_assign_zone_state(ns, zone, NVME_ZONE_STATE_IMPLICITLY_OPEN); return NVME_SUCCESS; } -/* fallthrough */ +fallthrough; case NVME_ZONE_STATE_IMPLICITLY_OPEN: if (flags & NVME_ZRM_AUTO) { return NVME_SUCCESS; } nvme_assign_zone_state(ns, zone, NVME_ZONE_STATE_EXPLICITLY_OPEN); -/* fallthrough */ +fallthrough; case NVME_ZONE_STATE_EXPLICITLY_OPEN: if (flags & NVME_ZRM_ZRWA) { ns->zns.numzrwa--; zone->d.za |= NVME_ZA_ZRWA_VALID; } return NVME_SUCCESS; default: return NVME_ZONE_INVAL_TRANSITION; } } @@ -3508,135 +3508,135 @@ static void nvme_do_write_fdp(NvmeCtrl *n, NvmeRequest *req, uint64_t slba, static uint16_t nvme_do_write(NvmeCtrl *n, NvmeRequest *req, bool append, bool wrz) { NvmeRwCmd *rw = (NvmeRwCmd *)&req->cmd; NvmeNamespace *ns = req->ns; uint64_t slba = le64_to_cpu(rw->slba); uint32_t nlb = (uint32_t)le16_to_cpu(rw->nlb) + 1; uint16_t ctrl = le16_to_cpu(rw->control); uint8_t prinfo = NVME_RW_PRINFO(ctrl); uint64