All of lore.kernel.org
 help / color / mirror / Atom feed
* [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10
@ 2020-10-10  7:57 Paolo Bonzini
  2020-10-10  7:57 ` [PULL 01/39] meson.build: Add comments to clarify code organization Paolo Bonzini
                   ` (40 more replies)
  0 siblings, 41 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel

The following changes since commit 4a7c0bd9dcb08798c6f82e55b5a3423f7ee669f1:

  Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-5.2-20201009' into staging (2020-10-09 15:48:04 +0100)

are available in the Git repository at:

  https://gitlab.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to 1340ff2adb2624e61c5fcb0eb1889b932b76f669:

  meson: identify more sections of meson.build (2020-10-09 13:19:50 -0400)

----------------------------------------------------------------
* qtest documentation improvements (Eduardo, myself)
* libqtest event buffering (Maxim)
* use RCU for list of children of a bus (Maxim)
* move more files to softmmu/ (myself)
* meson.build cleanups, qemu-storage-daemon fix (Philippe)

----------------------------------------------------------------
Eduardo Habkost (3):
      docs: Move QTest documentation to its own document
      docs/devel/qtest: Include protocol spec in document
      docs/devel/qtest: Include libqtest API reference

Huacai Chen (1):
      meson.build: Re-enable KVM support for MIPS

Marc-André Lureau (1):
      build-sys: fix git version from -version

Maxim Levitsky (11):
      qtest: rename qtest_qmp_receive to qtest_qmp_receive_dict
      qtest: Reintroduce qtest_qmp_receive
      qtest: remove qtest_qmp_receive_success
      qtest: switch users back to qtest_qmp_receive
      scsi/scsi_bus: switch search direction in scsi_device_find
      device_core: use drain_call_rcu in in qmp_device_add
      device-core: use RCU for list of children of a bus
      device-core: use atomic_set on .realized property
      scsi/scsi_bus: Add scsi_device_get
      virtio-scsi: use scsi_device_get
      scsi/scsi_bus: fix races in REPORT LUNS

Paolo Bonzini (13):
      softmmu: move more files to softmmu/
      exec: split out non-softmmu-specific parts
      qom: fix objects with improper parent type
      configure: fix performance regression due to PIC objects
      qtest: unify extra_qtest_srcs and extra_qtest_deps
      docs/devel: update instruction on how to add new unit tests
      device-plug-test: use qtest_qmp to send the device_del command
      qtest: check that drives are really appearing and disappearing
      qemu-iotests, qtest: rewrite test 067 as a qtest
      qdev: add "check if address free" callback for buses
      scsi: switch to bus->check_address
      scsi/scsi-bus: scsi_device_find: don't return unrealized devices
      meson: identify more sections of meson.build

Philippe Mathieu-Daudé (10):
      meson.build: Add comments to clarify code organization
      meson.build: Sort sourcesets alphabetically
      hw/core: Move the creation of the library to the main meson.build
      chardev: Move the creation of the library to the main meson.build
      migration: Move the creation of the library to the main meson.build
      io: Move the creation of the library to the main meson.build
      crypto: Move the creation of the library to the main meson.build
      authz: Move the creation of the library to the main meson.build
      qom: Move the creation of the library to the main meson.build
      hw/nvram: Always register FW_CFG_DATA_GENERATOR_INTERFACE

 .gitlab-ci.yml                           |   2 +-
 MAINTAINERS                              |  16 +-
 authz/meson.build                        |  10 -
 chardev/meson.build                      |   6 -
 configure                                |   1 +
 cpu.c                                    | 452 ++++++++++++++++++++++++++++++
 crypto/meson.build                       |  10 -
 docs/devel/index.rst                     |   1 +
 docs/devel/qtest.rst                     |  84 ++++++
 docs/devel/testing.rst                   |  64 +----
 hw/core/bus.c                            |  28 +-
 hw/core/meson.build                      |   6 -
 hw/core/qdev.c                           |  73 +++--
 hw/net/virtio-net.c                      |   2 +-
 hw/nvram/fw_cfg-interface.c              |  23 ++
 hw/nvram/fw_cfg.c                        |   7 -
 hw/nvram/meson.build                     |   3 +
 hw/scsi/scsi-bus.c                       | 262 +++++++++++-------
 hw/scsi/virtio-scsi.c                    |  27 +-
 hw/sd/core.c                             |   3 +-
 include/exec/cpu-common.h                |   3 +
 include/hw/acpi/vmgenid.h                |   2 +-
 include/hw/misc/vmcoreinfo.h             |   2 +-
 include/hw/qdev-core.h                   |  24 +-
 include/hw/scsi/scsi.h                   |   1 +
 include/net/can_host.h                   |   2 +-
 io/meson.build                           |  10 -
 meson.build                              | 126 +++++++--
 migration/meson.build                    |   8 +-
 qom/meson.build                          |   8 -
 scripts/coccinelle/qom-parent-type.cocci |  26 ++
 scripts/qemu-version.sh                  |   2 +-
 bootdevice.c => softmmu/bootdevice.c     |   0
 device_tree.c => softmmu/device_tree.c   |   0
 dma-helpers.c => softmmu/dma-helpers.c   |   0
 softmmu/meson.build                      |  11 +
 exec.c => softmmu/physmem.c              | 454 +------------------------------
 qdev-monitor.c => softmmu/qdev-monitor.c |  12 +
 qemu-seccomp.c => softmmu/qemu-seccomp.c |   0
 softmmu/qtest.c                          |  71 ++++-
 tpm.c => softmmu/tpm.c                   |   0
 tests/qemu-iotests/067                   | 157 -----------
 tests/qemu-iotests/067.out               | 414 ----------------------------
 tests/qemu-iotests/group                 |   2 +-
 tests/qtest/device-plug-test.c           |  32 +--
 tests/qtest/drive_del-test.c             | 244 +++++++++++++++--
 tests/qtest/libqos/libqtest.h            |  54 ++--
 tests/qtest/libqtest.c                   | 110 ++++----
 tests/qtest/meson.build                  |  59 ++--
 tests/qtest/migration-helpers.c          |  25 +-
 tests/qtest/pvpanic-test.c               |   4 +-
 tests/qtest/qmp-test.c                   |  18 +-
 tests/qtest/tpm-util.c                   |   8 +-
 53 files changed, 1472 insertions(+), 1497 deletions(-)
 create mode 100644 cpu.c
 create mode 100644 docs/devel/qtest.rst
 create mode 100644 hw/nvram/fw_cfg-interface.c
 create mode 100644 scripts/coccinelle/qom-parent-type.cocci
 rename bootdevice.c => softmmu/bootdevice.c (100%)
 rename device_tree.c => softmmu/device_tree.c (100%)
 rename dma-helpers.c => softmmu/dma-helpers.c (100%)
 rename exec.c => softmmu/physmem.c (91%)
 rename qdev-monitor.c => softmmu/qdev-monitor.c (98%)
 rename qemu-seccomp.c => softmmu/qemu-seccomp.c (100%)
 rename tpm.c => softmmu/tpm.c (100%)
 delete mode 100755 tests/qemu-iotests/067
 delete mode 100644 tests/qemu-iotests/067.out
-- 
2.26.2



^ permalink raw reply	[flat|nested] 43+ messages in thread

* [PULL 01/39] meson.build: Add comments to clarify code organization
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 02/39] meson.build: Sort sourcesets alphabetically Paolo Bonzini
                   ` (39 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20201006125602.2311423-2-philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/meson.build b/meson.build
index 17c89c87c6..68eaf8ce2f 100644
--- a/meson.build
+++ b/meson.build
@@ -1446,6 +1446,10 @@ qemu_syms = custom_target('qemu.syms', output: 'qemu.syms',
                              capture: true,
                              command: [undefsym, nm, '@INPUT@'])
 
+########################
+# Library dependencies #
+########################
+
 block_ss = block_ss.apply(config_host, strict: false)
 libblock = static_library('block', block_ss.sources() + genh,
                           dependencies: block_ss.dependencies(),
@@ -1465,6 +1469,10 @@ libqmp = static_library('qmp', qmp_ss.sources() + genh,
 
 qmp = declare_dependency(link_whole: [libqmp])
 
+###########
+# Targets #
+###########
+
 foreach m : block_mods + softmmu_mods
   shared_module(m.name(),
                 name_prefix: '',
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 02/39] meson.build: Sort sourcesets alphabetically
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
  2020-10-10  7:57 ` [PULL 01/39] meson.build: Add comments to clarify code organization Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 03/39] hw/core: Move the creation of the library to the main meson.build Paolo Bonzini
                   ` (38 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20201006125602.2311423-3-philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/meson.build b/meson.build
index 68eaf8ce2f..adcfb494a8 100644
--- a/meson.build
+++ b/meson.build
@@ -1182,19 +1182,19 @@ sphinx_extn_depends = [ meson.source_root() / 'docs/sphinx/depfile.py',
 
 # Collect sourcesets.
 
-util_ss = ss.source_set()
-stub_ss = ss.source_set()
-trace_ss = ss.source_set()
-block_ss = ss.source_set()
 blockdev_ss = ss.source_set()
-qmp_ss = ss.source_set()
-common_ss = ss.source_set()
-softmmu_ss = ss.source_set()
-user_ss = ss.source_set()
+block_ss = ss.source_set()
 bsd_user_ss = ss.source_set()
+common_ss = ss.source_set()
 linux_user_ss = ss.source_set()
-specific_ss = ss.source_set()
+qmp_ss = ss.source_set()
+softmmu_ss = ss.source_set()
 specific_fuzz_ss = ss.source_set()
+specific_ss = ss.source_set()
+stub_ss = ss.source_set()
+trace_ss = ss.source_set()
+user_ss = ss.source_set()
+util_ss = ss.source_set()
 
 modules = {}
 hw_arch = {}
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 03/39] hw/core: Move the creation of the library to the main meson.build
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
  2020-10-10  7:57 ` [PULL 01/39] meson.build: Add comments to clarify code organization Paolo Bonzini
  2020-10-10  7:57 ` [PULL 02/39] meson.build: Sort sourcesets alphabetically Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 04/39] chardev: " Paolo Bonzini
                   ` (37 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Be consistent creating all the libraries in the main meson.build file.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20201006125602.2311423-4-philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/meson.build | 6 ------
 meson.build         | 6 ++++++
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/core/meson.build b/hw/core/meson.build
index fc91f98075..4a744f3b5e 100644
--- a/hw/core/meson.build
+++ b/hw/core/meson.build
@@ -14,12 +14,6 @@ hwcore_files = files(
   'qdev-clock.c',
 )
 
-libhwcore = static_library('hwcore', sources: hwcore_files + genh,
-                           name_suffix: 'fa',
-                           build_by_default: false)
-hwcore = declare_dependency(link_whole: libhwcore)
-common_ss.add(hwcore)
-
 common_ss.add(files('cpu.c'))
 common_ss.add(when: 'CONFIG_FITLOADER', if_true: files('loader-fit.c'))
 common_ss.add(when: 'CONFIG_GENERIC_LOADER', if_true: files('generic-loader.c'))
diff --git a/meson.build b/meson.build
index adcfb494a8..6c760b4163 100644
--- a/meson.build
+++ b/meson.build
@@ -1469,6 +1469,12 @@ libqmp = static_library('qmp', qmp_ss.sources() + genh,
 
 qmp = declare_dependency(link_whole: [libqmp])
 
+libhwcore = static_library('hwcore', sources: hwcore_files + genh,
+                           name_suffix: 'fa',
+                           build_by_default: false)
+hwcore = declare_dependency(link_whole: libhwcore)
+common_ss.add(hwcore)
+
 ###########
 # Targets #
 ###########
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 04/39] chardev: Move the creation of the library to the main meson.build
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (2 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 03/39] hw/core: Move the creation of the library to the main meson.build Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 05/39] migration: " Paolo Bonzini
                   ` (36 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Be consistent creating all the libraries in the main meson.build file.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20201006125602.2311423-5-philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 chardev/meson.build | 6 ------
 meson.build         | 7 +++++++
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/chardev/meson.build b/chardev/meson.build
index 54e88d0310..dd2699a11b 100644
--- a/chardev/meson.build
+++ b/chardev/meson.build
@@ -1,4 +1,3 @@
-chardev_ss = ss.source_set()
 chardev_ss.add(files(
   'char-fe.c',
   'char-file.c',
@@ -25,11 +24,6 @@ chardev_ss.add(when: 'CONFIG_WIN32', if_true: files(
 ))
 
 chardev_ss = chardev_ss.apply(config_host, strict: false)
-libchardev = static_library('chardev', chardev_ss.sources() + genh,
-                            name_suffix: 'fa',
-                            build_by_default: false)
-
-chardev = declare_dependency(link_whole: libchardev)
 
 softmmu_ss.add(files('chardev-sysemu.c', 'msmouse.c', 'wctablet.c', 'testdev.c'))
 softmmu_ss.add(when: ['CONFIG_SPICE', spice], if_true: files('spice.c'))
diff --git a/meson.build b/meson.build
index 6c760b4163..39fc074977 100644
--- a/meson.build
+++ b/meson.build
@@ -1185,6 +1185,7 @@ sphinx_extn_depends = [ meson.source_root() / 'docs/sphinx/depfile.py',
 blockdev_ss = ss.source_set()
 block_ss = ss.source_set()
 bsd_user_ss = ss.source_set()
+chardev_ss = ss.source_set()
 common_ss = ss.source_set()
 linux_user_ss = ss.source_set()
 qmp_ss = ss.source_set()
@@ -1469,6 +1470,12 @@ libqmp = static_library('qmp', qmp_ss.sources() + genh,
 
 qmp = declare_dependency(link_whole: [libqmp])
 
+libchardev = static_library('chardev', chardev_ss.sources() + genh,
+                            name_suffix: 'fa',
+                            build_by_default: false)
+
+chardev = declare_dependency(link_whole: libchardev)
+
 libhwcore = static_library('hwcore', sources: hwcore_files + genh,
                            name_suffix: 'fa',
                            build_by_default: false)
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 05/39] migration: Move the creation of the library to the main meson.build
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (3 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 04/39] chardev: " Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 06/39] io: " Paolo Bonzini
                   ` (35 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Be consistent creating all the libraries in the main meson.build file.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20201006125602.2311423-6-philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build           | 7 +++++++
 migration/meson.build | 8 +-------
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/meson.build b/meson.build
index 39fc074977..dce48d8304 100644
--- a/meson.build
+++ b/meson.build
@@ -1451,6 +1451,13 @@ qemu_syms = custom_target('qemu.syms', output: 'qemu.syms',
 # Library dependencies #
 ########################
 
+libmigration = static_library('migration', sources: migration_files + genh,
+                              name_suffix: 'fa',
+                              build_by_default: false)
+migration = declare_dependency(link_with: libmigration,
+                               dependencies: [zlib, qom, io])
+softmmu_ss.add(migration)
+
 block_ss = block_ss.apply(config_host, strict: false)
 libblock = static_library('block', block_ss.sources() + genh,
                           dependencies: block_ss.dependencies(),
diff --git a/migration/meson.build b/migration/meson.build
index b5b71c8060..980e37865c 100644
--- a/migration/meson.build
+++ b/migration/meson.build
@@ -8,13 +8,7 @@ migration_files = files(
   'qemu-file.c',
   'qjson.c',
 )
-
-libmigration = static_library('migration', sources: migration_files + genh,
-                              name_suffix: 'fa',
-                              build_by_default: false)
-migration = declare_dependency(link_with: libmigration,
-                               dependencies: [zlib, qom, io])
-softmmu_ss.add(migration)
+softmmu_ss.add(migration_files)
 
 softmmu_ss.add(files(
   'block-dirty-bitmap.c',
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 06/39] io: Move the creation of the library to the main meson.build
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (4 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 05/39] migration: " Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 07/39] crypto: " Paolo Bonzini
                   ` (34 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Be consistent creating all the libraries in the main meson.build file.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20201006125602.2311423-7-philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 io/meson.build | 10 ----------
 meson.build    | 10 ++++++++++
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/io/meson.build b/io/meson.build
index 768c1b5ec3..bcd8b1e737 100644
--- a/io/meson.build
+++ b/io/meson.build
@@ -1,4 +1,3 @@
-io_ss = ss.source_set()
 io_ss.add(genh)
 io_ss.add(files(
   'channel-buffer.c',
@@ -14,12 +13,3 @@ io_ss.add(files(
   'net-listener.c',
   'task.c',
 ))
-
-io_ss = io_ss.apply(config_host, strict: false)
-libio = static_library('io', io_ss.sources() + genh,
-                       dependencies: [io_ss.dependencies()],
-                       link_with: libqemuutil,
-                       name_suffix: 'fa',
-                       build_by_default: false)
-
-io = declare_dependency(link_whole: libio, dependencies: [crypto, qom])
diff --git a/meson.build b/meson.build
index dce48d8304..28f31e6022 100644
--- a/meson.build
+++ b/meson.build
@@ -1187,6 +1187,7 @@ block_ss = ss.source_set()
 bsd_user_ss = ss.source_set()
 chardev_ss = ss.source_set()
 common_ss = ss.source_set()
+io_ss = ss.source_set()
 linux_user_ss = ss.source_set()
 qmp_ss = ss.source_set()
 softmmu_ss = ss.source_set()
@@ -1451,6 +1452,15 @@ qemu_syms = custom_target('qemu.syms', output: 'qemu.syms',
 # Library dependencies #
 ########################
 
+io_ss = io_ss.apply(config_host, strict: false)
+libio = static_library('io', io_ss.sources() + genh,
+                       dependencies: [io_ss.dependencies()],
+                       link_with: libqemuutil,
+                       name_suffix: 'fa',
+                       build_by_default: false)
+
+io = declare_dependency(link_whole: libio, dependencies: [crypto, qom])
+
 libmigration = static_library('migration', sources: migration_files + genh,
                               name_suffix: 'fa',
                               build_by_default: false)
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 07/39] crypto: Move the creation of the library to the main meson.build
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (5 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 06/39] io: " Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 08/39] authz: " Paolo Bonzini
                   ` (33 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Be consistent creating all the libraries in the main meson.build file.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20201006125602.2311423-8-philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 crypto/meson.build | 10 ----------
 meson.build        | 10 ++++++++++
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/crypto/meson.build b/crypto/meson.build
index f6f5ce1ecd..7f37b5d335 100644
--- a/crypto/meson.build
+++ b/crypto/meson.build
@@ -1,4 +1,3 @@
-crypto_ss = ss.source_set()
 crypto_ss.add(genh)
 crypto_ss.add(files(
   'afsplit.c',
@@ -52,15 +51,6 @@ if 'CONFIG_GNUTLS' in config_host
 endif
 
 
-crypto_ss = crypto_ss.apply(config_host, strict: false)
-libcrypto = static_library('crypto', crypto_ss.sources() + genh,
-                           dependencies: [crypto_ss.dependencies()],
-                           name_suffix: 'fa',
-                           build_by_default: false)
-
-crypto = declare_dependency(link_whole: libcrypto,
-                            dependencies: [authz, qom])
-
 util_ss.add(files('aes.c'))
 util_ss.add(files('init.c'))
 
diff --git a/meson.build b/meson.build
index 28f31e6022..9c199195bf 100644
--- a/meson.build
+++ b/meson.build
@@ -1187,6 +1187,7 @@ block_ss = ss.source_set()
 bsd_user_ss = ss.source_set()
 chardev_ss = ss.source_set()
 common_ss = ss.source_set()
+crypto_ss = ss.source_set()
 io_ss = ss.source_set()
 linux_user_ss = ss.source_set()
 qmp_ss = ss.source_set()
@@ -1452,6 +1453,15 @@ qemu_syms = custom_target('qemu.syms', output: 'qemu.syms',
 # Library dependencies #
 ########################
 
+crypto_ss = crypto_ss.apply(config_host, strict: false)
+libcrypto = static_library('crypto', crypto_ss.sources() + genh,
+                           dependencies: [crypto_ss.dependencies()],
+                           name_suffix: 'fa',
+                           build_by_default: false)
+
+crypto = declare_dependency(link_whole: libcrypto,
+                            dependencies: [authz, qom])
+
 io_ss = io_ss.apply(config_host, strict: false)
 libio = static_library('io', io_ss.sources() + genh,
                        dependencies: [io_ss.dependencies()],
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 08/39] authz: Move the creation of the library to the main meson.build
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (6 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 07/39] crypto: " Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 09/39] qom: " Paolo Bonzini
                   ` (32 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Be consistent creating all the libraries in the main meson.build file.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20201006125602.2311423-9-philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 authz/meson.build | 10 ----------
 meson.build       | 10 ++++++++++
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/authz/meson.build b/authz/meson.build
index 516d71f2e2..88fa7769cb 100644
--- a/authz/meson.build
+++ b/authz/meson.build
@@ -1,4 +1,3 @@
-authz_ss = ss.source_set()
 authz_ss.add(genh)
 authz_ss.add(files(
   'base.c',
@@ -8,12 +7,3 @@ authz_ss.add(files(
 ))
 
 authz_ss.add(when: ['CONFIG_AUTH_PAM', pam], if_true: files('pamacct.c'))
-
-authz_ss = authz_ss.apply(config_host, strict: false)
-libauthz = static_library('authz', authz_ss.sources() + genh,
-                          dependencies: [authz_ss.dependencies()],
-                          name_suffix: 'fa',
-                          build_by_default: false)
-
-authz = declare_dependency(link_whole: libauthz,
-                           dependencies: qom)
diff --git a/meson.build b/meson.build
index 9c199195bf..2736f74b8f 100644
--- a/meson.build
+++ b/meson.build
@@ -1182,6 +1182,7 @@ sphinx_extn_depends = [ meson.source_root() / 'docs/sphinx/depfile.py',
 
 # Collect sourcesets.
 
+authz_ss = ss.source_set()
 blockdev_ss = ss.source_set()
 block_ss = ss.source_set()
 bsd_user_ss = ss.source_set()
@@ -1453,6 +1454,15 @@ qemu_syms = custom_target('qemu.syms', output: 'qemu.syms',
 # Library dependencies #
 ########################
 
+authz_ss = authz_ss.apply(config_host, strict: false)
+libauthz = static_library('authz', authz_ss.sources() + genh,
+                          dependencies: [authz_ss.dependencies()],
+                          name_suffix: 'fa',
+                          build_by_default: false)
+
+authz = declare_dependency(link_whole: libauthz,
+                           dependencies: qom)
+
 crypto_ss = crypto_ss.apply(config_host, strict: false)
 libcrypto = static_library('crypto', crypto_ss.sources() + genh,
                            dependencies: [crypto_ss.dependencies()],
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 09/39] qom: Move the creation of the library to the main meson.build
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (7 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 08/39] authz: " Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 10/39] hw/nvram: Always register FW_CFG_DATA_GENERATOR_INTERFACE Paolo Bonzini
                   ` (31 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Be consistent creating all the libraries in the main meson.build file.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20201006125602.2311423-10-philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build     | 8 ++++++++
 qom/meson.build | 8 --------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/meson.build b/meson.build
index 2736f74b8f..f4ef3b83f3 100644
--- a/meson.build
+++ b/meson.build
@@ -1192,6 +1192,7 @@ crypto_ss = ss.source_set()
 io_ss = ss.source_set()
 linux_user_ss = ss.source_set()
 qmp_ss = ss.source_set()
+qom_ss = ss.source_set()
 softmmu_ss = ss.source_set()
 specific_fuzz_ss = ss.source_set()
 specific_ss = ss.source_set()
@@ -1454,6 +1455,13 @@ qemu_syms = custom_target('qemu.syms', output: 'qemu.syms',
 # Library dependencies #
 ########################
 
+qom_ss = qom_ss.apply(config_host, strict: false)
+libqom = static_library('qom', qom_ss.sources() + genh,
+                        dependencies: [qom_ss.dependencies()],
+                        name_suffix: 'fa')
+
+qom = declare_dependency(link_whole: libqom)
+
 authz_ss = authz_ss.apply(config_host, strict: false)
 libauthz = static_library('authz', authz_ss.sources() + genh,
                           dependencies: [authz_ss.dependencies()],
diff --git a/qom/meson.build b/qom/meson.build
index a1cd03c82c..062a3789d8 100644
--- a/qom/meson.build
+++ b/qom/meson.build
@@ -1,4 +1,3 @@
-qom_ss = ss.source_set()
 qom_ss.add(genh)
 qom_ss.add(files(
   'container.c',
@@ -9,10 +8,3 @@ qom_ss.add(files(
 
 qmp_ss.add(files('qom-qmp-cmds.c'))
 softmmu_ss.add(files('qom-hmp-cmds.c'))
-
-qom_ss = qom_ss.apply(config_host, strict: false)
-libqom = static_library('qom', qom_ss.sources() + genh,
-                        dependencies: [qom_ss.dependencies()],
-                        name_suffix: 'fa')
-
-qom = declare_dependency(link_whole: libqom)
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 10/39] hw/nvram: Always register FW_CFG_DATA_GENERATOR_INTERFACE
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (8 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 09/39] qom: " Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 11/39] softmmu: move more files to softmmu/ Paolo Bonzini
                   ` (30 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Laszlo Ersek, Philippe Mathieu-Daudé

From: Philippe Mathieu-Daudé <philmd@redhat.com>

While the FW_CFG_DATA_GENERATOR_INTERFACE is only consumed
by a device only available using system-mode (fw_cfg), it is
implemented by a crypto component (tls-cipher-suites) which
is always available when crypto is used.

Commit 69699f3055 introduced the following error in the
qemu-storage-daemon binary:

  $ echo -e \
    '{"execute": "qmp_capabilities"}\r\n{"execute": "qom-list-types"}\r\n{"execute": "quit"}\r\n' \
    | storage-daemon/qemu-storage-daemon --chardev stdio,id=qmp0  --monitor qmp0
  {"QMP": {"version": {"qemu": {"micro": 50, "minor": 1, "major": 5}, "package": ""}, "capabilities": ["oob"]}}
  {"return": {}}
  missing interface 'fw_cfg-data-generator' for object 'tls-creds'
  Aborted (core dumped)

Since QOM dependencies are resolved at runtime, this issue
could not be triggered at linktime, and we don't have test
running the qemu-storage-daemon binary.

Fix by always registering the QOM interface.

Reported-by: Kevin Wolf <kwolf@redhat.com>
Fixes: 69699f3055 ("crypto/tls-cipher-suites: Produce fw_cfg consumable blob")
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20201006111909.2302081-2-philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 MAINTAINERS                 |  2 +-
 hw/nvram/fw_cfg-interface.c | 23 +++++++++++++++++++++++
 hw/nvram/fw_cfg.c           |  7 -------
 hw/nvram/meson.build        |  3 +++
 4 files changed, 27 insertions(+), 8 deletions(-)
 create mode 100644 hw/nvram/fw_cfg-interface.c

diff --git a/MAINTAINERS b/MAINTAINERS
index e9d85cc873..8d50b96c33 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2055,7 +2055,7 @@ R: Laszlo Ersek <lersek@redhat.com>
 R: Gerd Hoffmann <kraxel@redhat.com>
 S: Supported
 F: docs/specs/fw_cfg.txt
-F: hw/nvram/fw_cfg.c
+F: hw/nvram/fw_cfg*.c
 F: stubs/fw_cfg.c
 F: include/hw/nvram/fw_cfg.h
 F: include/standard-headers/linux/qemu_fw_cfg.h
diff --git a/hw/nvram/fw_cfg-interface.c b/hw/nvram/fw_cfg-interface.c
new file mode 100644
index 0000000000..0e93feeae5
--- /dev/null
+++ b/hw/nvram/fw_cfg-interface.c
@@ -0,0 +1,23 @@
+/*
+ * QEMU Firmware configuration device emulation (QOM interfaces)
+ *
+ * Copyright 2020 Red Hat, Inc.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "hw/nvram/fw_cfg.h"
+
+static const TypeInfo fw_cfg_data_generator_interface_info = {
+    .parent = TYPE_INTERFACE,
+    .name = TYPE_FW_CFG_DATA_GENERATOR_INTERFACE,
+    .class_size = sizeof(FWCfgDataGeneratorClass),
+};
+
+static void fw_cfg_register_interfaces(void)
+{
+    type_register_static(&fw_cfg_data_generator_interface_info);
+}
+
+type_init(fw_cfg_register_interfaces)
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 0e95d057fd..08539a1aab 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -1360,18 +1360,11 @@ static const TypeInfo fw_cfg_mem_info = {
     .class_init    = fw_cfg_mem_class_init,
 };
 
-static const TypeInfo fw_cfg_data_generator_interface_info = {
-    .parent = TYPE_INTERFACE,
-    .name = TYPE_FW_CFG_DATA_GENERATOR_INTERFACE,
-    .class_size = sizeof(FWCfgDataGeneratorClass),
-};
-
 static void fw_cfg_register_types(void)
 {
     type_register_static(&fw_cfg_info);
     type_register_static(&fw_cfg_io_info);
     type_register_static(&fw_cfg_mem_info);
-    type_register_static(&fw_cfg_data_generator_interface_info);
 }
 
 type_init(fw_cfg_register_types)
diff --git a/hw/nvram/meson.build b/hw/nvram/meson.build
index 1f2ed013b2..fd2951a860 100644
--- a/hw/nvram/meson.build
+++ b/hw/nvram/meson.build
@@ -1,3 +1,6 @@
+# QOM interfaces must be available anytime QOM is used.
+qom_ss.add(files('fw_cfg-interface.c'))
+
 softmmu_ss.add(files('fw_cfg.c'))
 softmmu_ss.add(when: 'CONFIG_CHRP_NVRAM', if_true: files('chrp_nvram.c'))
 softmmu_ss.add(when: 'CONFIG_DS1225Y', if_true: files('ds1225y.c'))
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 11/39] softmmu: move more files to softmmu/
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (9 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 10/39] hw/nvram: Always register FW_CFG_DATA_GENERATOR_INTERFACE Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 12/39] exec: split out non-softmmu-specific parts Paolo Bonzini
                   ` (29 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel

Keep most softmmu_ss files into the system-emulation-specific
directory.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 MAINTAINERS                              |  9 +++++----
 meson.build                              | 10 ----------
 bootdevice.c => softmmu/bootdevice.c     |  0
 device_tree.c => softmmu/device_tree.c   |  0
 dma-helpers.c => softmmu/dma-helpers.c   |  0
 softmmu/meson.build                      | 10 ++++++++++
 qdev-monitor.c => softmmu/qdev-monitor.c |  0
 qemu-seccomp.c => softmmu/qemu-seccomp.c |  0
 tpm.c => softmmu/tpm.c                   |  0
 9 files changed, 15 insertions(+), 14 deletions(-)
 rename bootdevice.c => softmmu/bootdevice.c (100%)
 rename device_tree.c => softmmu/device_tree.c (100%)
 rename dma-helpers.c => softmmu/dma-helpers.c (100%)
 rename qdev-monitor.c => softmmu/qdev-monitor.c (100%)
 rename qemu-seccomp.c => softmmu/qemu-seccomp.c (100%)
 rename tpm.c => softmmu/tpm.c (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8d50b96c33..dda54f000d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2235,7 +2235,7 @@ Device Tree
 M: Alistair Francis <alistair.francis@wdc.com>
 R: David Gibson <david@gibson.dropbear.id.au>
 S: Maintained
-F: device_tree.c
+F: softmmu/device_tree.c
 F: include/sysemu/device_tree.h
 
 Dump
@@ -2281,6 +2281,7 @@ F: include/exec/memop.h
 F: include/exec/memory.h
 F: include/exec/ram_addr.h
 F: include/exec/ramblock.h
+F: softmmu/dma-helpers.c
 F: softmmu/ioport.c
 F: softmmu/memory.c
 F: include/exec/memory-internal.h
@@ -2461,7 +2462,7 @@ F: include/monitor/qdev.h
 F: include/qom/
 F: qapi/qom.json
 F: qapi/qdev.json
-F: qdev-monitor.c
+F: softmmu/qdev-monitor.c
 F: qom/
 F: tests/check-qom-interface.c
 F: tests/check-qom-proplist.c
@@ -2591,7 +2592,7 @@ F: docs/interop/dbus-vmstate.rst
 Seccomp
 M: Eduardo Otubo <otubo@redhat.com>
 S: Supported
-F: qemu-seccomp.c
+F: softmmu/qemu-seccomp.c
 F: include/sysemu/seccomp.h
 
 Cryptography
@@ -2957,7 +2958,7 @@ T: git https://github.com/stefanha/qemu.git block
 Bootdevice
 M: Gonglei <arei.gonglei@huawei.com>
 S: Maintained
-F: bootdevice.c
+F: softmmu/bootdevice.c
 
 Quorum
 M: Alberto Garcia <berto@igalia.com>
diff --git a/meson.build b/meson.build
index f4ef3b83f3..a280e4cf21 100644
--- a/meson.build
+++ b/meson.build
@@ -1365,17 +1365,7 @@ blockdev_ss.add(files(
 # os-win32.c does not
 blockdev_ss.add(when: 'CONFIG_POSIX', if_true: files('os-posix.c'))
 softmmu_ss.add(when: 'CONFIG_WIN32', if_true: [files('os-win32.c')])
-
 softmmu_ss.add_all(blockdev_ss)
-softmmu_ss.add(files(
-  'bootdevice.c',
-  'dma-helpers.c',
-  'qdev-monitor.c',
-), sdl)
-
-softmmu_ss.add(when: 'CONFIG_TPM', if_true: files('tpm.c'))
-softmmu_ss.add(when: 'CONFIG_SECCOMP', if_true: [files('qemu-seccomp.c'), seccomp])
-softmmu_ss.add(when: fdt, if_true: files('device_tree.c'))
 
 common_ss.add(files('cpus-common.c'))
 
diff --git a/bootdevice.c b/softmmu/bootdevice.c
similarity index 100%
rename from bootdevice.c
rename to softmmu/bootdevice.c
diff --git a/device_tree.c b/softmmu/device_tree.c
similarity index 100%
rename from device_tree.c
rename to softmmu/device_tree.c
diff --git a/dma-helpers.c b/softmmu/dma-helpers.c
similarity index 100%
rename from dma-helpers.c
rename to softmmu/dma-helpers.c
diff --git a/softmmu/meson.build b/softmmu/meson.build
index 36c96e7b15..862ab24878 100644
--- a/softmmu/meson.build
+++ b/softmmu/meson.build
@@ -14,3 +14,13 @@ specific_ss.add(when: 'CONFIG_SOFTMMU', if_true: [files(
 specific_ss.add(when: ['CONFIG_SOFTMMU', 'CONFIG_TCG'], if_true: [files(
   'icount.c'
 )])
+
+softmmu_ss.add(files(
+  'bootdevice.c',
+  'dma-helpers.c',
+  'qdev-monitor.c',
+), sdl)
+
+softmmu_ss.add(when: 'CONFIG_TPM', if_true: files('tpm.c'))
+softmmu_ss.add(when: 'CONFIG_SECCOMP', if_true: [files('qemu-seccomp.c'), seccomp])
+softmmu_ss.add(when: fdt, if_true: files('device_tree.c'))
diff --git a/qdev-monitor.c b/softmmu/qdev-monitor.c
similarity index 100%
rename from qdev-monitor.c
rename to softmmu/qdev-monitor.c
diff --git a/qemu-seccomp.c b/softmmu/qemu-seccomp.c
similarity index 100%
rename from qemu-seccomp.c
rename to softmmu/qemu-seccomp.c
diff --git a/tpm.c b/softmmu/tpm.c
similarity index 100%
rename from tpm.c
rename to softmmu/tpm.c
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 12/39] exec: split out non-softmmu-specific parts
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (10 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 11/39] softmmu: move more files to softmmu/ Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 13/39] qom: fix objects with improper parent type Paolo Bonzini
                   ` (28 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel

Over the years, most parts of exec.c that were not specific to softmmu
have been moved to accel/tcg; what's left is mostly the low-level part
of the memory API, which includes RAMBlock and AddressSpaceDispatch.
However exec.c also hosts 4-500 lines of code for the target specific
parts of the CPU QOM object, plus a few functions for user-mode
emulation that do not have a better place (they are not TCG-specific so
accel/tcg/user-exec.c is not a good place either).

Move these parts to a new file, so that exec.c can be moved to
softmmu/physmem.c.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 MAINTAINERS                 |   4 +-
 cpu.c                       | 452 +++++++++++++++++++++++++++++++++++
 include/exec/cpu-common.h   |   3 +
 meson.build                 |   2 +-
 softmmu/meson.build         |   3 +-
 exec.c => softmmu/physmem.c | 454 +-----------------------------------
 6 files changed, 467 insertions(+), 451 deletions(-)
 create mode 100644 cpu.c
 rename exec.c => softmmu/physmem.c (91%)

diff --git a/MAINTAINERS b/MAINTAINERS
index dda54f000d..7ef459a33c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -117,7 +117,6 @@ R: Paolo Bonzini <pbonzini@redhat.com>
 S: Maintained
 F: softmmu/cpus.c
 F: cpus-common.c
-F: exec.c
 F: accel/tcg/
 F: accel/stubs/tcg-stub.c
 F: scripts/decodetree.py
@@ -1525,6 +1524,7 @@ Machine core
 M: Eduardo Habkost <ehabkost@redhat.com>
 M: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
 S: Supported
+F: cpu.c
 F: hw/core/cpu.c
 F: hw/core/machine-qmp-cmds.c
 F: hw/core/machine.c
@@ -2284,8 +2284,8 @@ F: include/exec/ramblock.h
 F: softmmu/dma-helpers.c
 F: softmmu/ioport.c
 F: softmmu/memory.c
+F: softmmu/physmem.c
 F: include/exec/memory-internal.h
-F: exec.c
 F: scripts/coccinelle/memory-region-housekeeping.cocci
 
 SPICE
diff --git a/cpu.c b/cpu.c
new file mode 100644
index 0000000000..0be5dcb6f3
--- /dev/null
+++ b/cpu.c
@@ -0,0 +1,452 @@
+/*
+ * Target-specific parts of the CPU object
+ *
+ *  Copyright (c) 2003 Fabrice Bellard
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "qapi/error.h"
+
+#include "exec/target_page.h"
+#include "hw/qdev-core.h"
+#include "hw/qdev-properties.h"
+#include "qemu/error-report.h"
+#include "migration/vmstate.h"
+#ifdef CONFIG_USER_ONLY
+#include "qemu.h"
+#else
+#include "exec/address-spaces.h"
+#endif
+#include "sysemu/tcg.h"
+#include "sysemu/kvm.h"
+#include "sysemu/replay.h"
+#include "translate-all.h"
+#include "exec/log.h"
+
+uintptr_t qemu_host_page_size;
+intptr_t qemu_host_page_mask;
+
+#ifndef CONFIG_USER_ONLY
+static int cpu_common_post_load(void *opaque, int version_id)
+{
+    CPUState *cpu = opaque;
+
+    /* 0x01 was CPU_INTERRUPT_EXIT. This line can be removed when the
+       version_id is increased. */
+    cpu->interrupt_request &= ~0x01;
+    tlb_flush(cpu);
+
+    /* loadvm has just updated the content of RAM, bypassing the
+     * usual mechanisms that ensure we flush TBs for writes to
+     * memory we've translated code from. So we must flush all TBs,
+     * which will now be stale.
+     */
+    tb_flush(cpu);
+
+    return 0;
+}
+
+static int cpu_common_pre_load(void *opaque)
+{
+    CPUState *cpu = opaque;
+
+    cpu->exception_index = -1;
+
+    return 0;
+}
+
+static bool cpu_common_exception_index_needed(void *opaque)
+{
+    CPUState *cpu = opaque;
+
+    return tcg_enabled() && cpu->exception_index != -1;
+}
+
+static const VMStateDescription vmstate_cpu_common_exception_index = {
+    .name = "cpu_common/exception_index",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = cpu_common_exception_index_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_INT32(exception_index, CPUState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static bool cpu_common_crash_occurred_needed(void *opaque)
+{
+    CPUState *cpu = opaque;
+
+    return cpu->crash_occurred;
+}
+
+static const VMStateDescription vmstate_cpu_common_crash_occurred = {
+    .name = "cpu_common/crash_occurred",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = cpu_common_crash_occurred_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_BOOL(crash_occurred, CPUState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+const VMStateDescription vmstate_cpu_common = {
+    .name = "cpu_common",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .pre_load = cpu_common_pre_load,
+    .post_load = cpu_common_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(halted, CPUState),
+        VMSTATE_UINT32(interrupt_request, CPUState),
+        VMSTATE_END_OF_LIST()
+    },
+    .subsections = (const VMStateDescription*[]) {
+        &vmstate_cpu_common_exception_index,
+        &vmstate_cpu_common_crash_occurred,
+        NULL
+    }
+};
+#endif
+
+void cpu_exec_unrealizefn(CPUState *cpu)
+{
+    CPUClass *cc = CPU_GET_CLASS(cpu);
+
+    tlb_destroy(cpu);
+    cpu_list_remove(cpu);
+
+#ifdef CONFIG_USER_ONLY
+    assert(cc->vmsd == NULL);
+#else
+    if (cc->vmsd != NULL) {
+        vmstate_unregister(NULL, cc->vmsd, cpu);
+    }
+    if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
+        vmstate_unregister(NULL, &vmstate_cpu_common, cpu);
+    }
+    tcg_iommu_free_notifier_list(cpu);
+#endif
+}
+
+Property cpu_common_props[] = {
+#ifndef CONFIG_USER_ONLY
+    /* Create a memory property for softmmu CPU object,
+     * so users can wire up its memory. (This can't go in hw/core/cpu.c
+     * because that file is compiled only once for both user-mode
+     * and system builds.) The default if no link is set up is to use
+     * the system address space.
+     */
+    DEFINE_PROP_LINK("memory", CPUState, memory, TYPE_MEMORY_REGION,
+                     MemoryRegion *),
+#endif
+    DEFINE_PROP_BOOL("start-powered-off", CPUState, start_powered_off, false),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+void cpu_exec_initfn(CPUState *cpu)
+{
+    cpu->as = NULL;
+    cpu->num_ases = 0;
+
+#ifndef CONFIG_USER_ONLY
+    cpu->thread_id = qemu_get_thread_id();
+    cpu->memory = get_system_memory();
+    object_ref(OBJECT(cpu->memory));
+#endif
+}
+
+void cpu_exec_realizefn(CPUState *cpu, Error **errp)
+{
+    CPUClass *cc = CPU_GET_CLASS(cpu);
+    static bool tcg_target_initialized;
+
+    cpu_list_add(cpu);
+
+    if (tcg_enabled() && !tcg_target_initialized) {
+        tcg_target_initialized = true;
+        cc->tcg_initialize();
+    }
+    tlb_init(cpu);
+
+    qemu_plugin_vcpu_init_hook(cpu);
+
+#ifdef CONFIG_USER_ONLY
+    assert(cc->vmsd == NULL);
+#else /* !CONFIG_USER_ONLY */
+    if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
+        vmstate_register(NULL, cpu->cpu_index, &vmstate_cpu_common, cpu);
+    }
+    if (cc->vmsd != NULL) {
+        vmstate_register(NULL, cpu->cpu_index, cc->vmsd, cpu);
+    }
+
+    tcg_iommu_init_notifier_list(cpu);
+#endif
+}
+
+const char *parse_cpu_option(const char *cpu_option)
+{
+    ObjectClass *oc;
+    CPUClass *cc;
+    gchar **model_pieces;
+    const char *cpu_type;
+
+    model_pieces = g_strsplit(cpu_option, ",", 2);
+    if (!model_pieces[0]) {
+        error_report("-cpu option cannot be empty");
+        exit(1);
+    }
+
+    oc = cpu_class_by_name(CPU_RESOLVING_TYPE, model_pieces[0]);
+    if (oc == NULL) {
+        error_report("unable to find CPU model '%s'", model_pieces[0]);
+        g_strfreev(model_pieces);
+        exit(EXIT_FAILURE);
+    }
+
+    cpu_type = object_class_get_name(oc);
+    cc = CPU_CLASS(oc);
+    cc->parse_features(cpu_type, model_pieces[1], &error_fatal);
+    g_strfreev(model_pieces);
+    return cpu_type;
+}
+
+#if defined(CONFIG_USER_ONLY)
+void tb_invalidate_phys_addr(target_ulong addr)
+{
+    mmap_lock();
+    tb_invalidate_phys_page_range(addr, addr + 1);
+    mmap_unlock();
+}
+
+static void breakpoint_invalidate(CPUState *cpu, target_ulong pc)
+{
+    tb_invalidate_phys_addr(pc);
+}
+#else
+void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr, MemTxAttrs attrs)
+{
+    ram_addr_t ram_addr;
+    MemoryRegion *mr;
+    hwaddr l = 1;
+
+    if (!tcg_enabled()) {
+        return;
+    }
+
+    RCU_READ_LOCK_GUARD();
+    mr = address_space_translate(as, addr, &addr, &l, false, attrs);
+    if (!(memory_region_is_ram(mr)
+          || memory_region_is_romd(mr))) {
+        return;
+    }
+    ram_addr = memory_region_get_ram_addr(mr) + addr;
+    tb_invalidate_phys_page_range(ram_addr, ram_addr + 1);
+}
+
+static void breakpoint_invalidate(CPUState *cpu, target_ulong pc)
+{
+    /*
+     * There may not be a virtual to physical translation for the pc
+     * right now, but there may exist cached TB for this pc.
+     * Flush the whole TB cache to force re-translation of such TBs.
+     * This is heavyweight, but we're debugging anyway.
+     */
+    tb_flush(cpu);
+}
+#endif
+
+/* Add a breakpoint.  */
+int cpu_breakpoint_insert(CPUState *cpu, vaddr pc, int flags,
+                          CPUBreakpoint **breakpoint)
+{
+    CPUBreakpoint *bp;
+
+    bp = g_malloc(sizeof(*bp));
+
+    bp->pc = pc;
+    bp->flags = flags;
+
+    /* keep all GDB-injected breakpoints in front */
+    if (flags & BP_GDB) {
+        QTAILQ_INSERT_HEAD(&cpu->breakpoints, bp, entry);
+    } else {
+        QTAILQ_INSERT_TAIL(&cpu->breakpoints, bp, entry);
+    }
+
+    breakpoint_invalidate(cpu, pc);
+
+    if (breakpoint) {
+        *breakpoint = bp;
+    }
+    return 0;
+}
+
+/* Remove a specific breakpoint.  */
+int cpu_breakpoint_remove(CPUState *cpu, vaddr pc, int flags)
+{
+    CPUBreakpoint *bp;
+
+    QTAILQ_FOREACH(bp, &cpu->breakpoints, entry) {
+        if (bp->pc == pc && bp->flags == flags) {
+            cpu_breakpoint_remove_by_ref(cpu, bp);
+            return 0;
+        }
+    }
+    return -ENOENT;
+}
+
+/* Remove a specific breakpoint by reference.  */
+void cpu_breakpoint_remove_by_ref(CPUState *cpu, CPUBreakpoint *breakpoint)
+{
+    QTAILQ_REMOVE(&cpu->breakpoints, breakpoint, entry);
+
+    breakpoint_invalidate(cpu, breakpoint->pc);
+
+    g_free(breakpoint);
+}
+
+/* Remove all matching breakpoints. */
+void cpu_breakpoint_remove_all(CPUState *cpu, int mask)
+{
+    CPUBreakpoint *bp, *next;
+
+    QTAILQ_FOREACH_SAFE(bp, &cpu->breakpoints, entry, next) {
+        if (bp->flags & mask) {
+            cpu_breakpoint_remove_by_ref(cpu, bp);
+        }
+    }
+}
+
+/* enable or disable single step mode. EXCP_DEBUG is returned by the
+   CPU loop after each instruction */
+void cpu_single_step(CPUState *cpu, int enabled)
+{
+    if (cpu->singlestep_enabled != enabled) {
+        cpu->singlestep_enabled = enabled;
+        if (kvm_enabled()) {
+            kvm_update_guest_debug(cpu, 0);
+        } else {
+            /* must flush all the translated code to avoid inconsistencies */
+            /* XXX: only flush what is necessary */
+            tb_flush(cpu);
+        }
+    }
+}
+
+void cpu_abort(CPUState *cpu, const char *fmt, ...)
+{
+    va_list ap;
+    va_list ap2;
+
+    va_start(ap, fmt);
+    va_copy(ap2, ap);
+    fprintf(stderr, "qemu: fatal: ");
+    vfprintf(stderr, fmt, ap);
+    fprintf(stderr, "\n");
+    cpu_dump_state(cpu, stderr, CPU_DUMP_FPU | CPU_DUMP_CCOP);
+    if (qemu_log_separate()) {
+        FILE *logfile = qemu_log_lock();
+        qemu_log("qemu: fatal: ");
+        qemu_log_vprintf(fmt, ap2);
+        qemu_log("\n");
+        log_cpu_state(cpu, CPU_DUMP_FPU | CPU_DUMP_CCOP);
+        qemu_log_flush();
+        qemu_log_unlock(logfile);
+        qemu_log_close();
+    }
+    va_end(ap2);
+    va_end(ap);
+    replay_finish();
+#if defined(CONFIG_USER_ONLY)
+    {
+        struct sigaction act;
+        sigfillset(&act.sa_mask);
+        act.sa_handler = SIG_DFL;
+        act.sa_flags = 0;
+        sigaction(SIGABRT, &act, NULL);
+    }
+#endif
+    abort();
+}
+
+/* physical memory access (slow version, mainly for debug) */
+#if defined(CONFIG_USER_ONLY)
+int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr,
+                        void *ptr, target_ulong len, bool is_write)
+{
+    int flags;
+    target_ulong l, page;
+    void * p;
+    uint8_t *buf = ptr;
+
+    while (len > 0) {
+        page = addr & TARGET_PAGE_MASK;
+        l = (page + TARGET_PAGE_SIZE) - addr;
+        if (l > len)
+            l = len;
+        flags = page_get_flags(page);
+        if (!(flags & PAGE_VALID))
+            return -1;
+        if (is_write) {
+            if (!(flags & PAGE_WRITE))
+                return -1;
+            /* XXX: this code should not depend on lock_user */
+            if (!(p = lock_user(VERIFY_WRITE, addr, l, 0)))
+                return -1;
+            memcpy(p, buf, l);
+            unlock_user(p, addr, l);
+        } else {
+            if (!(flags & PAGE_READ))
+                return -1;
+            /* XXX: this code should not depend on lock_user */
+            if (!(p = lock_user(VERIFY_READ, addr, l, 1)))
+                return -1;
+            memcpy(buf, p, l);
+            unlock_user(p, addr, 0);
+        }
+        len -= l;
+        buf += l;
+        addr += l;
+    }
+    return 0;
+}
+#endif
+
+bool target_words_bigendian(void)
+{
+#if defined(TARGET_WORDS_BIGENDIAN)
+    return true;
+#else
+    return false;
+#endif
+}
+
+void page_size_init(void)
+{
+    /* NOTE: we can always suppose that qemu_host_page_size >=
+       TARGET_PAGE_SIZE */
+    if (qemu_host_page_size == 0) {
+        qemu_host_page_size = qemu_real_host_page_size;
+    }
+    if (qemu_host_page_size < TARGET_PAGE_SIZE) {
+        qemu_host_page_size = TARGET_PAGE_SIZE;
+    }
+    qemu_host_page_mask = -(intptr_t)qemu_host_page_size;
+}
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index d5e285d2b5..19805ed6db 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -14,6 +14,9 @@ void cpu_list_unlock(void);
 
 void tcg_flush_softmmu_tlb(CPUState *cs);
 
+void tcg_iommu_init_notifier_list(CPUState *cpu);
+void tcg_iommu_free_notifier_list(CPUState *cpu);
+
 #if !defined(CONFIG_USER_ONLY)
 
 enum device_endian {
diff --git a/meson.build b/meson.build
index a280e4cf21..26230614ba 100644
--- a/meson.build
+++ b/meson.build
@@ -1372,7 +1372,7 @@ common_ss.add(files('cpus-common.c'))
 subdir('softmmu')
 
 common_ss.add(capstone)
-specific_ss.add(files('disas.c', 'exec.c', 'gdbstub.c'), capstone, libpmem, libdaxctl)
+specific_ss.add(files('cpu.c', 'disas.c', 'gdbstub.c'), capstone)
 specific_ss.add(files('exec-vary.c'))
 specific_ss.add(when: 'CONFIG_TCG', if_true: files(
   'fpu/softfloat.c',
diff --git a/softmmu/meson.build b/softmmu/meson.build
index 862ab24878..8f7210b4f0 100644
--- a/softmmu/meson.build
+++ b/softmmu/meson.build
@@ -3,6 +3,7 @@ specific_ss.add(when: 'CONFIG_SOFTMMU', if_true: [files(
   'balloon.c',
   'cpus.c',
   'cpu-throttle.c',
+  'physmem.c',
   'ioport.c',
   'memory.c',
   'memory_mapping.c',
@@ -19,7 +20,7 @@ softmmu_ss.add(files(
   'bootdevice.c',
   'dma-helpers.c',
   'qdev-monitor.c',
-), sdl)
+), sdl, libpmem, libdaxctl)
 
 softmmu_ss.add(when: 'CONFIG_TPM', if_true: files('tpm.c'))
 softmmu_ss.add(when: 'CONFIG_SECCOMP', if_true: [files('qemu-seccomp.c'), seccomp])
diff --git a/exec.c b/softmmu/physmem.c
similarity index 91%
rename from exec.c
rename to softmmu/physmem.c
index bca441f7fd..e319fb2a1e 100644
--- a/exec.c
+++ b/softmmu/physmem.c
@@ -1,5 +1,5 @@
 /*
- *  Virtual page mapping
+ * RAM allocation and memory access
  *
  *  Copyright (c) 2003 Fabrice Bellard
  *
@@ -28,10 +28,8 @@
 #include "tcg/tcg.h"
 #include "hw/qdev-core.h"
 #include "hw/qdev-properties.h"
-#if !defined(CONFIG_USER_ONLY)
 #include "hw/boards.h"
 #include "hw/xen/xen.h"
-#endif
 #include "sysemu/kvm.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/tcg.h"
@@ -40,9 +38,6 @@
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
 #include "qemu/qemu-print.h"
-#if defined(CONFIG_USER_ONLY)
-#include "qemu.h"
-#else /* !CONFIG_USER_ONLY */
 #include "exec/memory.h"
 #include "exec/ioport.h"
 #include "sysemu/dma.h"
@@ -56,7 +51,6 @@
 #include <linux/falloc.h>
 #endif
 
-#endif
 #include "qemu/rcu_queue.h"
 #include "qemu/main-loop.h"
 #include "translate-all.h"
@@ -83,7 +77,6 @@
 
 //#define DEBUG_SUBPAGE
 
-#if !defined(CONFIG_USER_ONLY)
 /* ram_list is read under rcu_read_lock()/rcu_read_unlock().  Writes
  * are protected by the ramlist lock.
  */
@@ -96,12 +89,6 @@ AddressSpace address_space_io;
 AddressSpace address_space_memory;
 
 static MemoryRegion io_mem_unassigned;
-#endif
-
-uintptr_t qemu_host_page_size;
-intptr_t qemu_host_page_mask;
-
-#if !defined(CONFIG_USER_ONLY)
 
 typedef struct PhysPageEntry PhysPageEntry;
 
@@ -179,10 +166,6 @@ struct DirtyBitmapSnapshot {
     unsigned long dirty[];
 };
 
-#endif
-
-#if !defined(CONFIG_USER_ONLY)
-
 static void phys_map_node_reserve(PhysPageMap *map, unsigned nodes)
 {
     static unsigned alloc_hint = 16;
@@ -661,7 +644,7 @@ static void tcg_register_iommu_notifier(CPUState *cpu,
     }
 }
 
-static void tcg_iommu_free_notifier_list(CPUState *cpu)
+void tcg_iommu_free_notifier_list(CPUState *cpu)
 {
     /* Destroy the CPU's notifier list */
     int i;
@@ -675,6 +658,11 @@ static void tcg_iommu_free_notifier_list(CPUState *cpu)
     g_array_free(cpu->iommu_notifiers, true);
 }
 
+void tcg_iommu_init_notifier_list(CPUState *cpu)
+{
+    cpu->iommu_notifiers = g_array_new(false, true, sizeof(TCGIOMMUNotifier *));
+}
+
 /* Called from RCU critical section */
 MemoryRegionSection *
 address_space_translate_for_iotlb(CPUState *cpu, int asidx, hwaddr addr,
@@ -732,91 +720,6 @@ address_space_translate_for_iotlb(CPUState *cpu, int asidx, hwaddr addr,
 translate_fail:
     return &d->map.sections[PHYS_SECTION_UNASSIGNED];
 }
-#endif
-
-#if !defined(CONFIG_USER_ONLY)
-
-static int cpu_common_post_load(void *opaque, int version_id)
-{
-    CPUState *cpu = opaque;
-
-    /* 0x01 was CPU_INTERRUPT_EXIT. This line can be removed when the
-       version_id is increased. */
-    cpu->interrupt_request &= ~0x01;
-    tlb_flush(cpu);
-
-    /* loadvm has just updated the content of RAM, bypassing the
-     * usual mechanisms that ensure we flush TBs for writes to
-     * memory we've translated code from. So we must flush all TBs,
-     * which will now be stale.
-     */
-    tb_flush(cpu);
-
-    return 0;
-}
-
-static int cpu_common_pre_load(void *opaque)
-{
-    CPUState *cpu = opaque;
-
-    cpu->exception_index = -1;
-
-    return 0;
-}
-
-static bool cpu_common_exception_index_needed(void *opaque)
-{
-    CPUState *cpu = opaque;
-
-    return tcg_enabled() && cpu->exception_index != -1;
-}
-
-static const VMStateDescription vmstate_cpu_common_exception_index = {
-    .name = "cpu_common/exception_index",
-    .version_id = 1,
-    .minimum_version_id = 1,
-    .needed = cpu_common_exception_index_needed,
-    .fields = (VMStateField[]) {
-        VMSTATE_INT32(exception_index, CPUState),
-        VMSTATE_END_OF_LIST()
-    }
-};
-
-static bool cpu_common_crash_occurred_needed(void *opaque)
-{
-    CPUState *cpu = opaque;
-
-    return cpu->crash_occurred;
-}
-
-static const VMStateDescription vmstate_cpu_common_crash_occurred = {
-    .name = "cpu_common/crash_occurred",
-    .version_id = 1,
-    .minimum_version_id = 1,
-    .needed = cpu_common_crash_occurred_needed,
-    .fields = (VMStateField[]) {
-        VMSTATE_BOOL(crash_occurred, CPUState),
-        VMSTATE_END_OF_LIST()
-    }
-};
-
-const VMStateDescription vmstate_cpu_common = {
-    .name = "cpu_common",
-    .version_id = 1,
-    .minimum_version_id = 1,
-    .pre_load = cpu_common_pre_load,
-    .post_load = cpu_common_post_load,
-    .fields = (VMStateField[]) {
-        VMSTATE_UINT32(halted, CPUState),
-        VMSTATE_UINT32(interrupt_request, CPUState),
-        VMSTATE_END_OF_LIST()
-    },
-    .subsections = (const VMStateDescription*[]) {
-        &vmstate_cpu_common_exception_index,
-        &vmstate_cpu_common_crash_occurred,
-        NULL
-    }
-};
 
 void cpu_address_space_init(CPUState *cpu, int asidx,
                             const char *prefix, MemoryRegion *mr)
@@ -860,155 +763,7 @@ AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx)
     /* Return the AddressSpace corresponding to the specified index */
     return cpu->cpu_ases[asidx].as;
 }
-#endif
-
-void cpu_exec_unrealizefn(CPUState *cpu)
-{
-    CPUClass *cc = CPU_GET_CLASS(cpu);
-
-    tlb_destroy(cpu);
-    cpu_list_remove(cpu);
-
-    if (cc->vmsd != NULL) {
-        vmstate_unregister(NULL, cc->vmsd, cpu);
-    }
-    if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
-        vmstate_unregister(NULL, &vmstate_cpu_common, cpu);
-    }
-#ifndef CONFIG_USER_ONLY
-    tcg_iommu_free_notifier_list(cpu);
-#endif
-}
-
-Property cpu_common_props[] = {
-#ifndef CONFIG_USER_ONLY
-    /* Create a memory property for softmmu CPU object,
-     * so users can wire up its memory. (This can't go in hw/core/cpu.c
-     * because that file is compiled only once for both user-mode
-     * and system builds.) The default if no link is set up is to use
-     * the system address space.
-     */
-    DEFINE_PROP_LINK("memory", CPUState, memory, TYPE_MEMORY_REGION,
-                     MemoryRegion *),
-#endif
-    DEFINE_PROP_BOOL("start-powered-off", CPUState, start_powered_off, false),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
-void cpu_exec_initfn(CPUState *cpu)
-{
-    cpu->as = NULL;
-    cpu->num_ases = 0;
-
-#ifndef CONFIG_USER_ONLY
-    cpu->thread_id = qemu_get_thread_id();
-    cpu->memory = system_memory;
-    object_ref(OBJECT(cpu->memory));
-#endif
-}
-
-void cpu_exec_realizefn(CPUState *cpu, Error **errp)
-{
-    CPUClass *cc = CPU_GET_CLASS(cpu);
-    static bool tcg_target_initialized;
-
-    cpu_list_add(cpu);
-
-    if (tcg_enabled() && !tcg_target_initialized) {
-        tcg_target_initialized = true;
-        cc->tcg_initialize();
-    }
-    tlb_init(cpu);
-
-    qemu_plugin_vcpu_init_hook(cpu);
-
-#ifdef CONFIG_USER_ONLY
-    assert(cc->vmsd == NULL);
-#else /* !CONFIG_USER_ONLY */
-    if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
-        vmstate_register(NULL, cpu->cpu_index, &vmstate_cpu_common, cpu);
-    }
-    if (cc->vmsd != NULL) {
-        vmstate_register(NULL, cpu->cpu_index, cc->vmsd, cpu);
-    }
-
-    cpu->iommu_notifiers = g_array_new(false, true, sizeof(TCGIOMMUNotifier *));
-#endif
-}
-
-const char *parse_cpu_option(const char *cpu_option)
-{
-    ObjectClass *oc;
-    CPUClass *cc;
-    gchar **model_pieces;
-    const char *cpu_type;
-
-    model_pieces = g_strsplit(cpu_option, ",", 2);
-    if (!model_pieces[0]) {
-        error_report("-cpu option cannot be empty");
-        exit(1);
-    }
-
-    oc = cpu_class_by_name(CPU_RESOLVING_TYPE, model_pieces[0]);
-    if (oc == NULL) {
-        error_report("unable to find CPU model '%s'", model_pieces[0]);
-        g_strfreev(model_pieces);
-        exit(EXIT_FAILURE);
-    }
-
-    cpu_type = object_class_get_name(oc);
-    cc = CPU_CLASS(oc);
-    cc->parse_features(cpu_type, model_pieces[1], &error_fatal);
-    g_strfreev(model_pieces);
-    return cpu_type;
-}
-
-#if defined(CONFIG_USER_ONLY)
-void tb_invalidate_phys_addr(target_ulong addr)
-{
-    mmap_lock();
-    tb_invalidate_phys_page_range(addr, addr + 1);
-    mmap_unlock();
-}
-
-static void breakpoint_invalidate(CPUState *cpu, target_ulong pc)
-{
-    tb_invalidate_phys_addr(pc);
-}
-#else
-void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr, MemTxAttrs attrs)
-{
-    ram_addr_t ram_addr;
-    MemoryRegion *mr;
-    hwaddr l = 1;
-
-    if (!tcg_enabled()) {
-        return;
-    }
-
-    RCU_READ_LOCK_GUARD();
-    mr = address_space_translate(as, addr, &addr, &l, false, attrs);
-    if (!(memory_region_is_ram(mr)
-          || memory_region_is_romd(mr))) {
-        return;
-    }
-    ram_addr = memory_region_get_ram_addr(mr) + addr;
-    tb_invalidate_phys_page_range(ram_addr, ram_addr + 1);
-}
 
-static void breakpoint_invalidate(CPUState *cpu, target_ulong pc)
-{
-    /*
-     * There may not be a virtual to physical translation for the pc
-     * right now, but there may exist cached TB for this pc.
-     * Flush the whole TB cache to force re-translation of such TBs.
-     * This is heavyweight, but we're debugging anyway.
-     */
-    tb_flush(cpu);
-}
-#endif
-
-#ifndef CONFIG_USER_ONLY
 /* Add a watchpoint.  */
 int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, vaddr len,
                           int flags, CPUWatchpoint **watchpoint)
@@ -1117,123 +872,7 @@ int cpu_watchpoint_address_matches(CPUState *cpu, vaddr addr, vaddr len)
     }
     return ret;
 }
-#endif /* !CONFIG_USER_ONLY */
-
-/* Add a breakpoint.  */
-int cpu_breakpoint_insert(CPUState *cpu, vaddr pc, int flags,
-                          CPUBreakpoint **breakpoint)
-{
-    CPUBreakpoint *bp;
-
-    bp = g_malloc(sizeof(*bp));
-
-    bp->pc = pc;
-    bp->flags = flags;
-
-    /* keep all GDB-injected breakpoints in front */
-    if (flags & BP_GDB) {
-        QTAILQ_INSERT_HEAD(&cpu->breakpoints, bp, entry);
-    } else {
-        QTAILQ_INSERT_TAIL(&cpu->breakpoints, bp, entry);
-    }
-
-    breakpoint_invalidate(cpu, pc);
-
-    if (breakpoint) {
-        *breakpoint = bp;
-    }
-    return 0;
-}
-
-/* Remove a specific breakpoint.  */
-int cpu_breakpoint_remove(CPUState *cpu, vaddr pc, int flags)
-{
-    CPUBreakpoint *bp;
-
-    QTAILQ_FOREACH(bp, &cpu->breakpoints, entry) {
-        if (bp->pc == pc && bp->flags == flags) {
-            cpu_breakpoint_remove_by_ref(cpu, bp);
-            return 0;
-        }
-    }
-    return -ENOENT;
-}
 
-/* Remove a specific breakpoint by reference.  */
-void cpu_breakpoint_remove_by_ref(CPUState *cpu, CPUBreakpoint *breakpoint)
-{
-    QTAILQ_REMOVE(&cpu->breakpoints, breakpoint, entry);
-
-    breakpoint_invalidate(cpu, breakpoint->pc);
-
-    g_free(breakpoint);
-}
-
-/* Remove all matching breakpoints. */
-void cpu_breakpoint_remove_all(CPUState *cpu, int mask)
-{
-    CPUBreakpoint *bp, *next;
-
-    QTAILQ_FOREACH_SAFE(bp, &cpu->breakpoints, entry, next) {
-        if (bp->flags & mask) {
-            cpu_breakpoint_remove_by_ref(cpu, bp);
-        }
-    }
-}
-
-/* enable or disable single step mode. EXCP_DEBUG is returned by the
-   CPU loop after each instruction */
-void cpu_single_step(CPUState *cpu, int enabled)
-{
-    if (cpu->singlestep_enabled != enabled) {
-        cpu->singlestep_enabled = enabled;
-        if (kvm_enabled()) {
-            kvm_update_guest_debug(cpu, 0);
-        } else {
-            /* must flush all the translated code to avoid inconsistencies */
-            /* XXX: only flush what is necessary */
-            tb_flush(cpu);
-        }
-    }
-}
-
-void cpu_abort(CPUState *cpu, const char *fmt, ...)
-{
-    va_list ap;
-    va_list ap2;
-
-    va_start(ap, fmt);
-    va_copy(ap2, ap);
-    fprintf(stderr, "qemu: fatal: ");
-    vfprintf(stderr, fmt, ap);
-    fprintf(stderr, "\n");
-    cpu_dump_state(cpu, stderr, CPU_DUMP_FPU | CPU_DUMP_CCOP);
-    if (qemu_log_separate()) {
-        FILE *logfile = qemu_log_lock();
-        qemu_log("qemu: fatal: ");
-        qemu_log_vprintf(fmt, ap2);
-        qemu_log("\n");
-        log_cpu_state(cpu, CPU_DUMP_FPU | CPU_DUMP_CCOP);
-        qemu_log_flush();
-        qemu_log_unlock(logfile);
-        qemu_log_close();
-    }
-    va_end(ap2);
-    va_end(ap);
-    replay_finish();
-#if defined(CONFIG_USER_ONLY)
-    {
-        struct sigaction act;
-        sigfillset(&act.sa_mask);
-        act.sa_handler = SIG_DFL;
-        act.sa_flags = 0;
-        sigaction(SIGABRT, &act, NULL);
-    }
-#endif
-    abort();
-}
-
-#if !defined(CONFIG_USER_ONLY)
 /* Called from RCU critical section */
 static RAMBlock *qemu_get_ram_block(ram_addr_t addr)
 {
@@ -1420,9 +1059,6 @@ hwaddr memory_region_section_get_iotlb(CPUState *cpu,
     AddressSpaceDispatch *d = flatview_to_dispatch(section->fv);
     return section - d->map.sections;
 }
-#endif /* defined(CONFIG_USER_ONLY) */
-
-#if !defined(CONFIG_USER_ONLY)
 
 static int subpage_register(subpage_t *mmio, uint32_t start, uint32_t end,
                             uint16_t section);
@@ -3023,52 +2659,6 @@ MemoryRegion *get_system_io(void)
     return system_io;
 }
 
-#endif /* !defined(CONFIG_USER_ONLY) */
-
-/* physical memory access (slow version, mainly for debug) */
-#if defined(CONFIG_USER_ONLY)
-int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr,
-                        void *ptr, target_ulong len, bool is_write)
-{
-    int flags;
-    target_ulong l, page;
-    void * p;
-    uint8_t *buf = ptr;
-
-    while (len > 0) {
-        page = addr & TARGET_PAGE_MASK;
-        l = (page + TARGET_PAGE_SIZE) - addr;
-        if (l > len)
-            l = len;
-        flags = page_get_flags(page);
-        if (!(flags & PAGE_VALID))
-            return -1;
-        if (is_write) {
-            if (!(flags & PAGE_WRITE))
-                return -1;
-            /* XXX: this code should not depend on lock_user */
-            if (!(p = lock_user(VERIFY_WRITE, addr, l, 0)))
-                return -1;
-            memcpy(p, buf, l);
-            unlock_user(p, addr, l);
-        } else {
-            if (!(flags & PAGE_READ))
-                return -1;
-            /* XXX: this code should not depend on lock_user */
-            if (!(p = lock_user(VERIFY_READ, addr, l, 1)))
-                return -1;
-            memcpy(buf, p, l);
-            unlock_user(p, addr, 0);
-        }
-        len -= l;
-        buf += l;
-        addr += l;
-    }
-    return 0;
-}
-
-#else
-
 static void invalidate_and_set_dirty(MemoryRegion *mr, hwaddr addr,
                                      hwaddr length)
 {
@@ -3857,18 +3447,7 @@ int qemu_target_page_bits_min(void)
 {
     return TARGET_PAGE_BITS_MIN;
 }
-#endif
-
-bool target_words_bigendian(void)
-{
-#if defined(TARGET_WORDS_BIGENDIAN)
-    return true;
-#else
-    return false;
-#endif
-}
 
-#ifndef CONFIG_USER_ONLY
 bool cpu_physical_memory_is_io(hwaddr phys_addr)
 {
     MemoryRegion*mr;
@@ -3998,23 +3577,6 @@ bool ramblock_is_pmem(RAMBlock *rb)
     return rb->flags & RAM_PMEM;
 }
 
-#endif
-
-void page_size_init(void)
-{
-    /* NOTE: we can always suppose that qemu_host_page_size >=
-       TARGET_PAGE_SIZE */
-    if (qemu_host_page_size == 0) {
-        qemu_host_page_size = qemu_real_host_page_size;
-    }
-    if (qemu_host_page_size < TARGET_PAGE_SIZE) {
-        qemu_host_page_size = TARGET_PAGE_SIZE;
-    }
-    qemu_host_page_mask = -(intptr_t)qemu_host_page_size;
-}
-
-#if !defined(CONFIG_USER_ONLY)
-
 static void mtree_print_phys_entries(int start, int end, int skip, int ptr)
 {
     if (start == end - 1) {
@@ -4147,5 +3709,3 @@ bool ram_block_discard_is_required(void)
 {
     return qatomic_read(&ram_block_discard_disabled) < 0;
 }
-
-#endif
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 13/39] qom: fix objects with improper parent type
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (11 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 12/39] exec: split out non-softmmu-specific parts Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 14/39] configure: fix performance regression due to PIC objects Paolo Bonzini
                   ` (27 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Sergey Nizovtsev

Some objects accidentally inherit ObjectClass instead of Object.
They compile silently but may crash after downcasting.

In this patch, we introduce a coccinelle script to find broken
declarations and fix them manually with proper base type.

Signed-off-by: Sergey Nizovtsev <snizovtsev@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 MAINTAINERS                              |  1 +
 include/hw/acpi/vmgenid.h                |  2 +-
 include/hw/misc/vmcoreinfo.h             |  2 +-
 include/net/can_host.h                   |  2 +-
 scripts/coccinelle/qom-parent-type.cocci | 26 ++++++++++++++++++++++++
 5 files changed, 30 insertions(+), 3 deletions(-)
 create mode 100644 scripts/coccinelle/qom-parent-type.cocci

diff --git a/MAINTAINERS b/MAINTAINERS
index 7ef459a33c..fcb2c03c2b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2462,6 +2462,7 @@ F: include/monitor/qdev.h
 F: include/qom/
 F: qapi/qom.json
 F: qapi/qdev.json
+F: scripts/coccinelle/qom-parent-type.cocci
 F: softmmu/qdev-monitor.c
 F: qom/
 F: tests/check-qom-interface.c
diff --git a/include/hw/acpi/vmgenid.h b/include/hw/acpi/vmgenid.h
index d50fbacb8e..cb4ad37fc5 100644
--- a/include/hw/acpi/vmgenid.h
+++ b/include/hw/acpi/vmgenid.h
@@ -19,7 +19,7 @@
 OBJECT_DECLARE_SIMPLE_TYPE(VmGenIdState, VMGENID)
 
 struct VmGenIdState {
-    DeviceClass parent_obj;
+    DeviceState parent_obj;
     QemuUUID guid;                /* The 128-bit GUID seen by the guest */
     uint8_t vmgenid_addr_le[8];   /* Address of the GUID (little-endian) */
 };
diff --git a/include/hw/misc/vmcoreinfo.h b/include/hw/misc/vmcoreinfo.h
index ebada6617a..0b7b55d400 100644
--- a/include/hw/misc/vmcoreinfo.h
+++ b/include/hw/misc/vmcoreinfo.h
@@ -24,7 +24,7 @@ DECLARE_INSTANCE_CHECKER(VMCoreInfoState, VMCOREINFO,
 typedef struct fw_cfg_vmcoreinfo FWCfgVMCoreInfo;
 
 struct VMCoreInfoState {
-    DeviceClass parent_obj;
+    DeviceState parent_obj;
 
     bool has_vmcoreinfo;
     FWCfgVMCoreInfo vmcoreinfo;
diff --git a/include/net/can_host.h b/include/net/can_host.h
index 4e3ce3f954..caab71bdda 100644
--- a/include/net/can_host.h
+++ b/include/net/can_host.h
@@ -35,7 +35,7 @@
 OBJECT_DECLARE_TYPE(CanHostState, CanHostClass, CAN_HOST)
 
 struct CanHostState {
-    ObjectClass oc;
+    Object oc;
 
     CanBusState *bus;
     CanBusClientState bus_client;
diff --git a/scripts/coccinelle/qom-parent-type.cocci b/scripts/coccinelle/qom-parent-type.cocci
new file mode 100644
index 0000000000..9afb3edd97
--- /dev/null
+++ b/scripts/coccinelle/qom-parent-type.cocci
@@ -0,0 +1,26 @@
+// Highlight object declarations that don't look like object class but
+// accidentally inherit from it.
+
+@match@
+identifier obj_t, fld;
+type parent_t =~ ".*Class$";
+@@
+struct obj_t {
+    parent_t fld;
+    ...
+};
+
+@script:python filter depends on match@
+obj_t << match.obj_t;
+@@
+is_class_obj = obj_t.endswith('Class')
+cocci.include_match(not is_class_obj)
+
+@replacement depends on filter@
+identifier match.obj_t, match.fld;
+type match.parent_t;
+@@
+struct obj_t {
+*   parent_t fld;
+    ...
+};
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 14/39] configure: fix performance regression due to PIC objects
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (12 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 13/39] qom: fix objects with improper parent type Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 15/39] docs: Move QTest documentation to its own document Paolo Bonzini
                   ` (26 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Ahmed Karaman

Because most files in QEMU are grouped into static libraries, Meson conservatively
compiles them with -fPIC.  This is overkill and produces slowdowns up to 20% on
some TCG tests.

As a stopgap measure, use the b_staticpic option to limit the slowdown to
--enable-pie.  https://github.com/mesonbuild/meson/pull/7760 will allow
us to use b_staticpic=false and let Meson do the right thing.

Reported-by: Ahmed Karaman <ahmedkrmn@outlook.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20200919155639.1045857-1-pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 configure | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configure b/configure
index 28df227db5..b553288c5e 100755
--- a/configure
+++ b/configure
@@ -7209,6 +7209,7 @@ NINJA=${ninja:-$PWD/ninjatool} $meson setup \
         -Dwerror=$(if test "$werror" = yes; then echo true; else echo false; fi) \
         -Dstrip=$(if test "$strip_opt" = yes; then echo true; else echo false; fi) \
         -Db_pie=$(if test "$pie" = yes; then echo true; else echo false; fi) \
+        -Db_staticpic=$(if test "$pie" = yes; then echo true; else echo false; fi) \
         -Db_coverage=$(if test "$gcov" = yes; then echo true; else echo false; fi) \
 	-Dmalloc=$malloc -Dmalloc_trim=$malloc_trim -Dsparse=$sparse \
 	-Dkvm=$kvm -Dhax=$hax -Dwhpx=$whpx -Dhvf=$hvf \
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 15/39] docs: Move QTest documentation to its own document
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (13 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 14/39] configure: fix performance regression due to PIC objects Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 16/39] docs/devel/qtest: Include protocol spec in document Paolo Bonzini
                   ` (25 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Thomas Huth, Eduardo Habkost

From: Eduardo Habkost <ehabkost@redhat.com>

The qtest and libqtest doc comments will be parsed to generate
API documentation, so move QTest documentation to its own
document where the API and format documentation and will be
included.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Acked-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20201005205228.697463-2-ehabkost@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 docs/devel/index.rst   |  1 +
 docs/devel/qtest.rst   | 58 ++++++++++++++++++++++++++++++++++++++++++
 docs/devel/testing.rst | 47 ++--------------------------------
 3 files changed, 61 insertions(+), 45 deletions(-)
 create mode 100644 docs/devel/qtest.rst

diff --git a/docs/devel/index.rst b/docs/devel/index.rst
index 5fda2d3509..77baae5c77 100644
--- a/docs/devel/index.rst
+++ b/docs/devel/index.rst
@@ -21,6 +21,7 @@ Contents:
    atomics
    stable-process
    testing
+   qtest
    decodetree
    secure-coding-practices
    tcg
diff --git a/docs/devel/qtest.rst b/docs/devel/qtest.rst
new file mode 100644
index 0000000000..86dec84a0b
--- /dev/null
+++ b/docs/devel/qtest.rst
@@ -0,0 +1,58 @@
+========================================
+QTest Device Emulation Testing Framework
+========================================
+
+QTest is a device emulation testing framework.  It can be very useful to test
+device models; it could also control certain aspects of QEMU (such as virtual
+clock stepping), with a special purpose "qtest" protocol.  Refer to the
+documentation in ``qtest.c`` for more details of the protocol.
+
+QTest cases can be executed with
+
+.. code::
+
+   make check-qtest
+
+The QTest library is implemented by ``tests/qtest/libqtest.c`` and the API is
+defined in ``tests/qtest/libqtest.h``.
+
+Consider adding a new QTest case when you are introducing a new virtual
+hardware, or extending one if you are adding functionalities to an existing
+virtual device.
+
+On top of libqtest, a higher level library, ``libqos``, was created to
+encapsulate common tasks of device drivers, such as memory management and
+communicating with system buses or devices. Many virtual device tests use
+libqos instead of directly calling into libqtest.
+
+Steps to add a new QTest case are:
+
+1. Create a new source file for the test. (More than one file can be added as
+   necessary.) For example, ``tests/qtest/foo-test.c``.
+
+2. Write the test code with the glib and libqtest/libqos API. See also existing
+   tests and the library headers for reference.
+
+3. Register the new test in ``tests/qtest/Makefile.include``. Add the test
+   executable name to an appropriate ``check-qtest-*-y`` variable. For example:
+
+   ``check-qtest-generic-y = tests/qtest/foo-test$(EXESUF)``
+
+4. Add object dependencies of the executable in the Makefile, including the
+   test source file(s) and other interesting objects. For example:
+
+   ``tests/qtest/foo-test$(EXESUF): tests/qtest/foo-test.o $(libqos-obj-y)``
+
+Debugging a QTest failure is slightly harder than the unit test because the
+tests look up QEMU program names in the environment variables, such as
+``QTEST_QEMU_BINARY`` and ``QTEST_QEMU_IMG``, and also because it is not easy
+to attach gdb to the QEMU process spawned from the test. But manual invoking
+and using gdb on the test is still simple to do: find out the actual command
+from the output of
+
+.. code::
+
+  make check-qtest V=1
+
+which you can run manually.
+
diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index bd64c1bdcd..a171494b4e 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -70,8 +70,8 @@ QTest
 
 QTest is a device emulation testing framework.  It can be very useful to test
 device models; it could also control certain aspects of QEMU (such as virtual
-clock stepping), with a special purpose "qtest" protocol.  Refer to the
-documentation in ``qtest.c`` for more details of the protocol.
+clock stepping), with a special purpose "qtest" protocol.  Refer to
+:doc:`qtest` for more details.
 
 QTest cases can be executed with
 
@@ -79,49 +79,6 @@ QTest cases can be executed with
 
    make check-qtest
 
-The QTest library is implemented by ``tests/qtest/libqtest.c`` and the API is
-defined in ``tests/qtest/libqtest.h``.
-
-Consider adding a new QTest case when you are introducing a new virtual
-hardware, or extending one if you are adding functionalities to an existing
-virtual device.
-
-On top of libqtest, a higher level library, ``libqos``, was created to
-encapsulate common tasks of device drivers, such as memory management and
-communicating with system buses or devices. Many virtual device tests use
-libqos instead of directly calling into libqtest.
-
-Steps to add a new QTest case are:
-
-1. Create a new source file for the test. (More than one file can be added as
-   necessary.) For example, ``tests/qtest/foo-test.c``.
-
-2. Write the test code with the glib and libqtest/libqos API. See also existing
-   tests and the library headers for reference.
-
-3. Register the new test in ``tests/qtest/Makefile.include``. Add the test
-   executable name to an appropriate ``check-qtest-*-y`` variable. For example:
-
-   ``check-qtest-generic-y = tests/qtest/foo-test$(EXESUF)``
-
-4. Add object dependencies of the executable in the Makefile, including the
-   test source file(s) and other interesting objects. For example:
-
-   ``tests/qtest/foo-test$(EXESUF): tests/qtest/foo-test.o $(libqos-obj-y)``
-
-Debugging a QTest failure is slightly harder than the unit test because the
-tests look up QEMU program names in the environment variables, such as
-``QTEST_QEMU_BINARY`` and ``QTEST_QEMU_IMG``, and also because it is not easy
-to attach gdb to the QEMU process spawned from the test. But manual invoking
-and using gdb on the test is still simple to do: find out the actual command
-from the output of
-
-.. code::
-
-  make check-qtest V=1
-
-which you can run manually.
-
 QAPI schema tests
 -----------------
 
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 16/39] docs/devel/qtest: Include protocol spec in document
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (14 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 15/39] docs: Move QTest documentation to its own document Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 17/39] docs/devel/qtest: Include libqtest API reference Paolo Bonzini
                   ` (24 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Thomas Huth, Eduardo Habkost

From: Eduardo Habkost <ehabkost@redhat.com>

Include the QTest Protocol doc string in docs/devel/qtest.rst,
after converting it to use Sphinx syntax.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Acked-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20201005205228.697463-3-ehabkost@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 docs/devel/qtest.rst | 12 ++++++--
 softmmu/qtest.c      | 71 +++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 73 insertions(+), 10 deletions(-)

diff --git a/docs/devel/qtest.rst b/docs/devel/qtest.rst
index 86dec84a0b..3bf9ebee7f 100644
--- a/docs/devel/qtest.rst
+++ b/docs/devel/qtest.rst
@@ -4,8 +4,8 @@ QTest Device Emulation Testing Framework
 
 QTest is a device emulation testing framework.  It can be very useful to test
 device models; it could also control certain aspects of QEMU (such as virtual
-clock stepping), with a special purpose "qtest" protocol.  Refer to the
-documentation in ``qtest.c`` for more details of the protocol.
+clock stepping), with a special purpose "qtest" protocol.  Refer to
+:ref:`qtest-protocol` for more details of the protocol.
 
 QTest cases can be executed with
 
@@ -56,3 +56,11 @@ from the output of
 
 which you can run manually.
 
+
+.. _qtest-protocol:
+
+QTest Protocol
+--------------
+
+.. kernel-doc:: softmmu/qtest.c
+   :doc: QTest Protocol
diff --git a/softmmu/qtest.c b/softmmu/qtest.c
index 0d43cf8883..2c6e8dc858 100644
--- a/softmmu/qtest.c
+++ b/softmmu/qtest.c
@@ -49,92 +49,139 @@ static void *qtest_server_send_opaque;
 #define FMT_timeval "%ld.%06ld"
 
 /**
- * QTest Protocol
+ * DOC: QTest Protocol
  *
  * Line based protocol, request/response based.  Server can send async messages
  * so clients should always handle many async messages before the response
  * comes in.
  *
  * Valid requests
+ * ^^^^^^^^^^^^^^
  *
  * Clock management:
+ * """""""""""""""""
  *
  * The qtest client is completely in charge of the QEMU_CLOCK_VIRTUAL.  qtest commands
  * let you adjust the value of the clock (monotonically).  All the commands
  * return the current value of the clock in nanoseconds.
  *
+ * .. code-block:: none
+ *
  *  > clock_step
  *  < OK VALUE
  *
- *     Advance the clock to the next deadline.  Useful when waiting for
- *     asynchronous events.
+ * Advance the clock to the next deadline.  Useful when waiting for
+ * asynchronous events.
+ *
+ * .. code-block:: none
  *
  *  > clock_step NS
  *  < OK VALUE
  *
- *     Advance the clock by NS nanoseconds.
+ * Advance the clock by NS nanoseconds.
+ *
+ * .. code-block:: none
  *
  *  > clock_set NS
  *  < OK VALUE
  *
- *     Advance the clock to NS nanoseconds (do nothing if it's already past).
+ * Advance the clock to NS nanoseconds (do nothing if it's already past).
  *
  * PIO and memory access:
+ * """"""""""""""""""""""
+ *
+ * .. code-block:: none
  *
  *  > outb ADDR VALUE
  *  < OK
  *
+ * .. code-block:: none
+ *
  *  > outw ADDR VALUE
  *  < OK
  *
+ * .. code-block:: none
+ *
  *  > outl ADDR VALUE
  *  < OK
  *
+ * .. code-block:: none
+ *
  *  > inb ADDR
  *  < OK VALUE
  *
+ * .. code-block:: none
+ *
  *  > inw ADDR
  *  < OK VALUE
  *
+ * .. code-block:: none
+ *
  *  > inl ADDR
  *  < OK VALUE
  *
+ * .. code-block:: none
+ *
  *  > writeb ADDR VALUE
  *  < OK
  *
+ * .. code-block:: none
+ *
  *  > writew ADDR VALUE
  *  < OK
  *
+ * .. code-block:: none
+ *
  *  > writel ADDR VALUE
  *  < OK
  *
+ * .. code-block:: none
+ *
  *  > writeq ADDR VALUE
  *  < OK
  *
+ * .. code-block:: none
+ *
  *  > readb ADDR
  *  < OK VALUE
  *
+ * .. code-block:: none
+ *
  *  > readw ADDR
  *  < OK VALUE
  *
+ * .. code-block:: none
+ *
  *  > readl ADDR
  *  < OK VALUE
  *
+ * .. code-block:: none
+ *
  *  > readq ADDR
  *  < OK VALUE
  *
+ * .. code-block:: none
+ *
  *  > read ADDR SIZE
  *  < OK DATA
  *
+ * .. code-block:: none
+ *
  *  > write ADDR SIZE DATA
  *  < OK
  *
+ * .. code-block:: none
+ *
  *  > b64read ADDR SIZE
  *  < OK B64_DATA
  *
+ * .. code-block:: none
+ *
  *  > b64write ADDR SIZE B64_DATA
  *  < OK
  *
+ * .. code-block:: none
+ *
  *  > memset ADDR SIZE VALUE
  *  < OK
  *
@@ -149,16 +196,21 @@ static void *qtest_server_send_opaque;
  * If the sizes do not match, the data will be truncated.
  *
  * IRQ management:
+ * """""""""""""""
+ *
+ * .. code-block:: none
  *
  *  > irq_intercept_in QOM-PATH
  *  < OK
  *
+ * .. code-block:: none
+ *
  *  > irq_intercept_out QOM-PATH
  *  < OK
  *
  * Attach to the gpio-in (resp. gpio-out) pins exported by the device at
  * QOM-PATH.  When the pin is triggered, one of the following async messages
- * will be printed to the qtest stream:
+ * will be printed to the qtest stream::
  *
  *  IRQ raise NUM
  *  IRQ lower NUM
@@ -168,12 +220,15 @@ static void *qtest_server_send_opaque;
  * NUM=0 even though it is remapped to GSI 2).
  *
  * Setting interrupt level:
+ * """"""""""""""""""""""""
+ *
+ * .. code-block:: none
  *
  *  > set_irq_in QOM-PATH NAME NUM LEVEL
  *  < OK
  *
- *  where NAME is the name of the irq/gpio list, NUM is an IRQ number and
- *  LEVEL is an signed integer IRQ level.
+ * where NAME is the name of the irq/gpio list, NUM is an IRQ number and
+ * LEVEL is an signed integer IRQ level.
  *
  * Forcibly set the given interrupt pin to the given level.
  *
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 17/39] docs/devel/qtest: Include libqtest API reference
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (15 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 16/39] docs/devel/qtest: Include protocol spec in document Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 18/39] qtest: unify extra_qtest_srcs and extra_qtest_deps Paolo Bonzini
                   ` (23 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Thomas Huth, Eduardo Habkost

From: Eduardo Habkost <ehabkost@redhat.com>

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Acked-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20201005205228.697463-4-ehabkost@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 docs/devel/qtest.rst          |  6 ++++++
 tests/qtest/libqos/libqtest.h | 20 ++++++++++----------
 2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/docs/devel/qtest.rst b/docs/devel/qtest.rst
index 3bf9ebee7f..075fe5f7d5 100644
--- a/docs/devel/qtest.rst
+++ b/docs/devel/qtest.rst
@@ -64,3 +64,9 @@ QTest Protocol
 
 .. kernel-doc:: softmmu/qtest.c
    :doc: QTest Protocol
+
+
+libqtest API reference
+----------------------
+
+.. kernel-doc:: tests/qtest/libqos/libqtest.h
diff --git a/tests/qtest/libqos/libqtest.h b/tests/qtest/libqos/libqtest.h
index a6ee1654f2..209fcf6973 100644
--- a/tests/qtest/libqos/libqtest.h
+++ b/tests/qtest/libqos/libqtest.h
@@ -24,7 +24,7 @@ typedef struct QTestState QTestState;
 
 /**
  * qtest_initf:
- * @fmt...: Format for creating other arguments to pass to QEMU, formatted
+ * @fmt: Format for creating other arguments to pass to QEMU, formatted
  * like sprintf().
  *
  * Convenience wrapper around qtest_init().
@@ -87,7 +87,7 @@ void qtest_quit(QTestState *s);
  * @s: #QTestState instance to operate on.
  * @fds: array of file descriptors
  * @fds_num: number of elements in @fds
- * @fmt...: QMP message to send to qemu, formatted like
+ * @fmt: QMP message to send to qemu, formatted like
  * qobject_from_jsonf_nofail().  See parse_escape() for what's
  * supported after '%'.
  *
@@ -100,7 +100,7 @@ QDict *qtest_qmp_fds(QTestState *s, int *fds, size_t fds_num,
 /**
  * qtest_qmp:
  * @s: #QTestState instance to operate on.
- * @fmt...: QMP message to send to qemu, formatted like
+ * @fmt: QMP message to send to qemu, formatted like
  * qobject_from_jsonf_nofail().  See parse_escape() for what's
  * supported after '%'.
  *
@@ -112,7 +112,7 @@ QDict *qtest_qmp(QTestState *s, const char *fmt, ...)
 /**
  * qtest_qmp_send:
  * @s: #QTestState instance to operate on.
- * @fmt...: QMP message to send to qemu, formatted like
+ * @fmt: QMP message to send to qemu, formatted like
  * qobject_from_jsonf_nofail().  See parse_escape() for what's
  * supported after '%'.
  *
@@ -124,7 +124,7 @@ void qtest_qmp_send(QTestState *s, const char *fmt, ...)
 /**
  * qtest_qmp_send_raw:
  * @s: #QTestState instance to operate on.
- * @fmt...: text to send, formatted like sprintf()
+ * @fmt: text to send, formatted like sprintf()
  *
  * Sends text to the QMP monitor verbatim.  Need not be valid JSON;
  * this is useful for negative tests.
@@ -201,7 +201,7 @@ QDict *qtest_qmp_receive(QTestState *s);
 /**
  * qtest_qmp_eventwait:
  * @s: #QTestState instance to operate on.
- * @s: #event event to wait for.
+ * @event: event to wait for.
  *
  * Continuously polls for QMP responses until it receives the desired event.
  */
@@ -210,7 +210,7 @@ void qtest_qmp_eventwait(QTestState *s, const char *event);
 /**
  * qtest_qmp_eventwait_ref:
  * @s: #QTestState instance to operate on.
- * @s: #event event to wait for.
+ * @event: event to wait for.
  *
  * Continuously polls for QMP responses until it receives the desired event.
  * Returns a copy of the event for further investigation.
@@ -237,7 +237,7 @@ QDict *qtest_qmp_receive_success(QTestState *s,
 /**
  * qtest_hmp:
  * @s: #QTestState instance to operate on.
- * @fmt...: HMP command to send to QEMU, formats arguments like sprintf().
+ * @fmt: HMP command to send to QEMU, formats arguments like sprintf().
  *
  * Send HMP command to QEMU via QMP's human-monitor-command.
  * QMP events are discarded.
@@ -629,7 +629,7 @@ void qtest_add_abrt_handler(GHookFunc fn, const void *data);
 /**
  * qtest_qmp_assert_success:
  * @qts: QTestState instance to operate on
- * @fmt...: QMP message to send to qemu, formatted like
+ * @fmt: QMP message to send to qemu, formatted like
  * qobject_from_jsonf_nofail().  See parse_escape() for what's
  * supported after '%'.
  *
@@ -676,7 +676,7 @@ void qtest_qmp_device_add_qdict(QTestState *qts, const char *drv,
  * @qts: QTestState instance to operate on
  * @driver: Name of the device that should be added
  * @id: Identification string
- * @fmt...: QMP message to send to qemu, formatted like
+ * @fmt: QMP message to send to qemu, formatted like
  * qobject_from_jsonf_nofail().  See parse_escape() for what's
  * supported after '%'.
  *
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 18/39] qtest: unify extra_qtest_srcs and extra_qtest_deps
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (16 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 17/39] docs/devel/qtest: Include libqtest API reference Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 19/39] docs/devel: update instruction on how to add new unit tests Paolo Bonzini
                   ` (22 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel

Currently the extra sources and extra dependencies of qtests are held
in two separate dictionaries.  Use the same trick as tests/meson.build
to combine them into one.  This will make it easier to update the
documentation for unit tests and qtests.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/qtest/meson.build | 55 +++++++++++++++++++----------------------
 1 file changed, 26 insertions(+), 29 deletions(-)

diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 0f32ca0895..28bafc02a2 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -193,35 +193,25 @@ qos_test_ss.add(
 qos_test_ss.add(when: 'CONFIG_VIRTFS', if_true: files('virtio-9p-test.c'))
 qos_test_ss.add(when: 'CONFIG_VHOST_USER', if_true: files('vhost-user-test.c'))
 
-extra_qtest_deps = {
-  'bios-tables-test': [io],
-  'ivshmem-test': [rt],
-  'qos-test': [chardev, io],
-  'tpm-crb-swtpm-test': [io],
-  'tpm-crb-test': [io],
-  'tpm-tis-swtpm-test': [io],
-  'tpm-tis-test': [io],
-  'tpm-tis-device-swtpm-test': [io],
-  'tpm-tis-device-test': [io],
-}
-extra_qtest_srcs = {
-  'bios-tables-test': files('boot-sector.c', 'acpi-utils.c', 'tpm-emu.c'),
-  'pxe-test': files('boot-sector.c'),
+tpmemu_files = ['tpm-emu.c', 'tpm-util.c', 'tpm-tests.c']
+
+qtests = {
+  'bios-tables-test': [io, 'boot-sector.c', 'acpi-utils.c', 'tpm-emu.c'],
   'cdrom-test': files('boot-sector.c'),
-  'migration-test': files('migration-helpers.c'),
-  'ivshmem-test': files('../../contrib/ivshmem-server/ivshmem-server.c'),
   'dbus-vmstate-test': files('migration-helpers.c') + dbus_vmstate1,
+  'ivshmem-test': [rt, '../../contrib/ivshmem-server/ivshmem-server.c'],
+  'migration-test': files('migration-helpers.c'),
+  'pxe-test': files('boot-sector.c'),
+  'qos-test': [chardev, io, qos_test_ss.apply(config_host, strict: false).sources()],
+  'tpm-crb-swtpm-test': [io, tpmemu_files],
+  'tpm-crb-test': [io, tpmemu_files],
+  'tpm-tis-swtpm-test': [io, tpmemu_files, 'tpm-tis-util.c'],
+  'tpm-tis-test': [io, tpmemu_files, 'tpm-tis-util.c'],
+  'tpm-tis-device-swtpm-test': [io, tpmemu_files, 'tpm-tis-util.c'],
+  'tpm-tis-device-test': [io, tpmemu_files, 'tpm-tis-util.c'],
   'vmgenid-test': files('boot-sector.c', 'acpi-utils.c'),
-  'tpm-crb-swtpm-test': files('tpm-emu.c', 'tpm-util.c', 'tpm-tests.c'),
-  'tpm-crb-test': files('tpm-emu.c', 'tpm-util.c', 'tpm-tests.c'),
-  'tpm-tis-device-swtpm-test': files('tpm-emu.c', 'tpm-util.c', 'tpm-tis-util.c', 'tpm-tests.c'),
-  'tpm-tis-device-test': files('tpm-emu.c', 'tpm-util.c', 'tpm-tis-util.c', 'tpm-tests.c'),
-  'tpm-tis-swtpm-test': files('tpm-emu.c', 'tpm-util.c', 'tpm-tis-util.c', 'tpm-tests.c'),
-  'tpm-tis-test': files('tpm-emu.c', 'tpm-util.c', 'tpm-tis-util.c', 'tpm-tests.c'),
-  'qos-test': qos_test_ss.apply(config_host, strict: false).sources()
 }
 
-
 qtest_executables = {}
 foreach dir : target_dirs
   if not dir.endswith('-softmmu')
@@ -230,7 +220,7 @@ foreach dir : target_dirs
 
   target_base = dir.split('-')[0]
   qtest_emulator = emulators['qemu-system-' + target_base]
-  qtests = get_variable('qtests_' + target_base, []) + qtests_generic
+  target_qtests = get_variable('qtests_' + target_base, []) + qtests_generic
 
   test_deps = []
   qtest_env = environment()
@@ -241,14 +231,21 @@ foreach dir : target_dirs
   qtest_env.set('G_TEST_DBUS_DAEMON', meson.source_root() / 'tests/dbus-vmstate-daemon.sh')
   qtest_env.set('QTEST_QEMU_BINARY', './qemu-system-' + target_base)
   
-  foreach test : qtests
+  foreach test : target_qtests
     # Executables are shared across targets, declare them only the first time we
     # encounter them
     if not qtest_executables.has_key(test)
+      src = [test + '.c']
+      deps = [qemuutil, qos]
+      if test in qtests
+        # use a sourceset to quickly separate sources and deps
+        test_ss = ss.source_set()
+        test_ss.add(qtests[test])
+        src += test_ss.all_sources()
+        deps += test_ss.all_dependencies()
+      endif
       qtest_executables += {
-        test: executable(test,
-                         files(test + '.c') + extra_qtest_srcs.get(test, []),
-                         dependencies: [qemuutil, qos] + extra_qtest_deps.get(test, []))
+        test: executable(test, src, dependencies: deps)
       }
     endif
     # FIXME: missing dependency on the emulator binary and qemu-img
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 19/39] docs/devel: update instruction on how to add new unit tests
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (17 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 18/39] qtest: unify extra_qtest_srcs and extra_qtest_deps Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 20/39] build-sys: fix git version from -version Paolo Bonzini
                   ` (21 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 docs/devel/qtest.rst   | 30 +++++++++++++++++++++---------
 docs/devel/testing.rst | 19 ++++++++++---------
 2 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/docs/devel/qtest.rst b/docs/devel/qtest.rst
index 075fe5f7d5..97c5a75626 100644
--- a/docs/devel/qtest.rst
+++ b/docs/devel/qtest.rst
@@ -33,15 +33,27 @@ Steps to add a new QTest case are:
 2. Write the test code with the glib and libqtest/libqos API. See also existing
    tests and the library headers for reference.
 
-3. Register the new test in ``tests/qtest/Makefile.include``. Add the test
-   executable name to an appropriate ``check-qtest-*-y`` variable. For example:
-
-   ``check-qtest-generic-y = tests/qtest/foo-test$(EXESUF)``
-
-4. Add object dependencies of the executable in the Makefile, including the
-   test source file(s) and other interesting objects. For example:
-
-   ``tests/qtest/foo-test$(EXESUF): tests/qtest/foo-test.o $(libqos-obj-y)``
+3. Register the new test in ``tests/qtest/meson.build``. Add the test
+   executable name to an appropriate ``qtests_*`` variable. There is
+   one variable per architecture, plus ``qtests_generic`` for tests
+   that can be run for all architectures.  For example::
+
+     qtests_generic = [
+       ...
+       'foo-test',
+       ...
+     ]
+
+4. If the test has more than one source file or needs to be linked with any
+   dependency other than ``qemuutil`` and ``qos``, list them in the ``qtests``
+   dictionary.  For example a test that needs to use the ``QIO`` library
+   will have an entry like::
+
+     {
+       ...
+       'foo-test': [io],
+       ...
+     }
 
 Debugging a QTest failure is slightly harder than the unit test because the
 tests look up QEMU program names in the environment variables, such as
diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index a171494b4e..cecee6eaa1 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -41,15 +41,16 @@ add a new unit test:
    test. The test code should be organized with the glib testing framework.
    Copying and modifying an existing test is usually a good idea.
 
-3. Add the test to ``tests/Makefile.include``. First, name the unit test
-   program and add it to ``$(check-unit-y)``; then add a rule to build the
-   executable.  For example:
-
-.. code::
-
-  check-unit-y += tests/foo-test$(EXESUF)
-  tests/foo-test$(EXESUF): tests/foo-test.o $(test-util-obj-y)
-  ...
+3. Add the test to ``tests/meson.build``. The unit tests are listed in a
+   dictionary called ``tests``.  The values are any additional sources and
+   dependencies to be linked with the test.  For a simple test whose source
+   is in ``tests/foo-test.c``, it is enough to add an entry like::
+
+     {
+       ...
+       'foo-test': [],
+       ...
+     }
 
 Since unit tests don't require environment variables, the simplest way to debug
 a unit test failure is often directly invoking it or even running it under
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 20/39] build-sys: fix git version from -version
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (18 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 19/39] docs/devel: update instruction on how to add new unit tests Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 21/39] meson.build: Re-enable KVM support for MIPS Paolo Bonzini
                   ` (20 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Marc-André Lureau, Laszlo Ersek

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Typo introduced with the script.

Fixes: 2c273f32d3 ("meson: generate qemu-version.h")
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reported-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200929143654.518157-1-marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/qemu-version.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qemu-version.sh b/scripts/qemu-version.sh
index 03128c56a2..3f6e7e6d41 100755
--- a/scripts/qemu-version.sh
+++ b/scripts/qemu-version.sh
@@ -9,7 +9,7 @@ version="$3"
 if [ -z "$pkgversion" ]; then
     cd "$dir"
     if [ -e .git ]; then
-        pkgversion=$(git describe --match 'v*' --dirty | echo "")
+        pkgversion=$(git describe --match 'v*' --dirty) || :
     fi
 fi
 
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 21/39] meson.build: Re-enable KVM support for MIPS
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (19 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 20/39] build-sys: fix git version from -version Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 22/39] qtest: rename qtest_qmp_receive to qtest_qmp_receive_dict Paolo Bonzini
                   ` (19 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Huacai Chen, Huacai Chen, aolo Bonzini

From: Huacai Chen <zltjiangshi@gmail.com>

After converting from configure to meson, KVM support is lost for MIPS,
so re-enable it in meson.build.

Fixes: fdb75aeff7c212e1afaaa3a43 ("configure: remove target configuration")
Fixes: 8a19980e3fc42239aae054bc9 ("configure: move accelerator logic to meson")
Cc: aolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Huacai Chen <chenhc@lemote.com>
Message-Id: <1602059975-10115-3-git-send-email-chenhc@lemote.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/meson.build b/meson.build
index 26230614ba..0c676af194 100644
--- a/meson.build
+++ b/meson.build
@@ -59,6 +59,8 @@ elif cpu == 's390x'
   kvm_targets = ['s390x-softmmu']
 elif cpu in ['ppc', 'ppc64']
   kvm_targets = ['ppc-softmmu', 'ppc64-softmmu']
+elif cpu in ['mips', 'mips64']
+  kvm_targets = ['mips-softmmu', 'mipsel-softmmu', 'mips64-softmmu', 'mips64el-softmmu']
 else
   kvm_targets = []
 endif
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 22/39] qtest: rename qtest_qmp_receive to qtest_qmp_receive_dict
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (20 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 21/39] meson.build: Re-enable KVM support for MIPS Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 23/39] qtest: Reintroduce qtest_qmp_receive Paolo Bonzini
                   ` (18 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Maxim Levitsky

From: Maxim Levitsky <mlevitsk@redhat.com>

In the next patch a new version of qtest_qmp_receive will be
reintroduced that will buffer received qmp events for later
consumption in qtest_qmp_eventwait_ref

No functional change intended.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/qtest/ahci-test.c        |  4 ++--
 tests/qtest/device-plug-test.c |  2 +-
 tests/qtest/drive_del-test.c   |  2 +-
 tests/qtest/libqos/libqtest.h  |  4 ++--
 tests/qtest/libqtest.c         | 16 ++++++++--------
 tests/qtest/pvpanic-test.c     |  2 +-
 tests/qtest/qmp-test.c         | 18 +++++++++---------
 7 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
index 5e1954852e..d42ebaeb4c 100644
--- a/tests/qtest/ahci-test.c
+++ b/tests/qtest/ahci-test.c
@@ -1590,7 +1590,7 @@ static void test_atapi_tray(void)
     qtest_qmp_send(ahci->parent->qts, "{'execute': 'blockdev-open-tray', "
                     "'arguments': {'id': 'cd0'}}");
     atapi_wait_tray(ahci, true);
-    rsp = qtest_qmp_receive(ahci->parent->qts);
+    rsp = qtest_qmp_receive_dict(ahci->parent->qts);
     qobject_unref(rsp);
 
     qmp_discard_response(ahci->parent->qts,
@@ -1620,7 +1620,7 @@ static void test_atapi_tray(void)
     qtest_qmp_send(ahci->parent->qts, "{'execute': 'blockdev-close-tray', "
                    "'arguments': {'id': 'cd0'}}");
     atapi_wait_tray(ahci, false);
-    rsp = qtest_qmp_receive(ahci->parent->qts);
+    rsp = qtest_qmp_receive_dict(ahci->parent->qts);
     qobject_unref(rsp);
 
     /* Now, to convince ATAPI we understand the media has changed... */
diff --git a/tests/qtest/device-plug-test.c b/tests/qtest/device-plug-test.c
index 9214892741..a2247856be 100644
--- a/tests/qtest/device-plug-test.c
+++ b/tests/qtest/device-plug-test.c
@@ -23,7 +23,7 @@ static void device_del_start(QTestState *qtest, const char *id)
 
 static void device_del_finish(QTestState *qtest)
 {
-    QDict *resp = qtest_qmp_receive(qtest);
+    QDict *resp = qtest_qmp_receive_dict(qtest);
 
     g_assert(qdict_haskey(resp, "return"));
     qobject_unref(resp);
diff --git a/tests/qtest/drive_del-test.c b/tests/qtest/drive_del-test.c
index 2d765865ce..ba0cd77445 100644
--- a/tests/qtest/drive_del-test.c
+++ b/tests/qtest/drive_del-test.c
@@ -41,7 +41,7 @@ static void device_del(QTestState *qts)
     /* Complication: ignore DEVICE_DELETED event */
     qmp_discard_response(qts, "{'execute': 'device_del',"
                          " 'arguments': { 'id': 'dev0' } }");
-    response = qtest_qmp_receive(qts);
+    response = qtest_qmp_receive_dict(qts);
     g_assert(response);
     g_assert(qdict_haskey(response, "return"));
     qobject_unref(response);
diff --git a/tests/qtest/libqos/libqtest.h b/tests/qtest/libqos/libqtest.h
index 209fcf6973..9b3f99b322 100644
--- a/tests/qtest/libqos/libqtest.h
+++ b/tests/qtest/libqos/libqtest.h
@@ -191,12 +191,12 @@ void qtest_qmp_vsend(QTestState *s, const char *fmt, va_list ap)
     GCC_FMT_ATTR(2, 0);
 
 /**
- * qtest_receive:
+ * qtest_qmp_receive_dict:
  * @s: #QTestState instance to operate on.
  *
  * Reads a QMP message from QEMU and returns the response.
  */
-QDict *qtest_qmp_receive(QTestState *s);
+QDict *qtest_qmp_receive_dict(QTestState *s);
 
 /**
  * qtest_qmp_eventwait:
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 58f58e1ece..dadc465825 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -322,7 +322,7 @@ QTestState *qtest_init(const char *extra_args)
     QDict *greeting;
 
     /* Read the QMP greeting and then do the handshake */
-    greeting = qtest_qmp_receive(s);
+    greeting = qtest_qmp_receive_dict(s);
     qobject_unref(greeting);
     qobject_unref(qtest_qmp(s, "{ 'execute': 'qmp_capabilities' }"));
 
@@ -603,7 +603,7 @@ QDict *qmp_fd_receive(int fd)
     return qmp.response;
 }
 
-QDict *qtest_qmp_receive(QTestState *s)
+QDict *qtest_qmp_receive_dict(QTestState *s)
 {
     return qmp_fd_receive(s->qmp_fd);
 }
@@ -678,7 +678,7 @@ QDict *qtest_vqmp_fds(QTestState *s, int *fds, size_t fds_num,
     qtest_qmp_vsend_fds(s, fds, fds_num, fmt, ap);
 
     /* Receive reply */
-    return qtest_qmp_receive(s);
+    return qtest_qmp_receive_dict(s);
 }
 
 QDict *qtest_vqmp(QTestState *s, const char *fmt, va_list ap)
@@ -686,7 +686,7 @@ QDict *qtest_vqmp(QTestState *s, const char *fmt, va_list ap)
     qtest_qmp_vsend(s, fmt, ap);
 
     /* Receive reply */
-    return qtest_qmp_receive(s);
+    return qtest_qmp_receive_dict(s);
 }
 
 QDict *qmp_fd(int fd, const char *fmt, ...)
@@ -776,7 +776,7 @@ QDict *qtest_qmp_eventwait_ref(QTestState *s, const char *event)
     QDict *response;
 
     for (;;) {
-        response = qtest_qmp_receive(s);
+        response = qtest_qmp_receive_dict(s);
         if ((qdict_haskey(response, "event")) &&
             (strcmp(qdict_get_str(response, "event"), event) == 0)) {
             return response;
@@ -807,7 +807,7 @@ char *qtest_vhmp(QTestState *s, const char *fmt, va_list ap)
     while (ret == NULL && qdict_get_try_str(resp, "event")) {
         /* Ignore asynchronous QMP events */
         qobject_unref(resp);
-        resp = qtest_qmp_receive(s);
+        resp = qtest_qmp_receive_dict(s);
         ret = g_strdup(qdict_get_try_str(resp, "return"));
     }
     g_assert(ret);
@@ -1255,7 +1255,7 @@ QDict *qtest_qmp_receive_success(QTestState *s,
     const char *event;
 
     for (;;) {
-        response = qtest_qmp_receive(s);
+        response = qtest_qmp_receive_dict(s);
         g_assert(!qdict_haskey(response, "error"));
         ret = qdict_get_qdict(response, "return");
         if (ret) {
@@ -1345,7 +1345,7 @@ void qtest_qmp_device_del(QTestState *qts, const char *id)
     rsp = qtest_qmp_receive_success(qts, device_deleted_cb, &got_event);
     qobject_unref(rsp);
     if (!got_event) {
-        rsp = qtest_qmp_receive(qts);
+        rsp = qtest_qmp_receive_dict(qts);
         g_assert_cmpstr(qdict_get_try_str(rsp, "event"),
                         ==, "DEVICE_DELETED");
         qobject_unref(rsp);
diff --git a/tests/qtest/pvpanic-test.c b/tests/qtest/pvpanic-test.c
index e57639481e..b0b20ad340 100644
--- a/tests/qtest/pvpanic-test.c
+++ b/tests/qtest/pvpanic-test.c
@@ -24,7 +24,7 @@ static void test_panic(void)
 
     qtest_outb(qts, 0x505, 0x1);
 
-    response = qtest_qmp_receive(qts);
+    response = qtest_qmp_receive_dict(qts);
     g_assert(qdict_haskey(response, "event"));
     g_assert_cmpstr(qdict_get_str(response, "event"), ==, "GUEST_PANICKED");
     g_assert(qdict_haskey(response, "data"));
diff --git a/tests/qtest/qmp-test.c b/tests/qtest/qmp-test.c
index e1032c5a21..eb1cd8abb8 100644
--- a/tests/qtest/qmp-test.c
+++ b/tests/qtest/qmp-test.c
@@ -47,37 +47,37 @@ static void test_malformed(QTestState *qts)
 
     /* syntax error */
     qtest_qmp_send_raw(qts, "{]\n");
-    resp = qtest_qmp_receive(qts);
+    resp = qtest_qmp_receive_dict(qts);
     qmp_expect_error_and_unref(resp, "GenericError");
     assert_recovered(qts);
 
     /* lexical error: impossible byte outside string */
     qtest_qmp_send_raw(qts, "{\xFF");
-    resp = qtest_qmp_receive(qts);
+    resp = qtest_qmp_receive_dict(qts);
     qmp_expect_error_and_unref(resp, "GenericError");
     assert_recovered(qts);
 
     /* lexical error: funny control character outside string */
     qtest_qmp_send_raw(qts, "{\x01");
-    resp = qtest_qmp_receive(qts);
+    resp = qtest_qmp_receive_dict(qts);
     qmp_expect_error_and_unref(resp, "GenericError");
     assert_recovered(qts);
 
     /* lexical error: impossible byte in string */
     qtest_qmp_send_raw(qts, "{'bad \xFF");
-    resp = qtest_qmp_receive(qts);
+    resp = qtest_qmp_receive_dict(qts);
     qmp_expect_error_and_unref(resp, "GenericError");
     assert_recovered(qts);
 
     /* lexical error: control character in string */
     qtest_qmp_send_raw(qts, "{'execute': 'nonexistent', 'id':'\n");
-    resp = qtest_qmp_receive(qts);
+    resp = qtest_qmp_receive_dict(qts);
     qmp_expect_error_and_unref(resp, "GenericError");
     assert_recovered(qts);
 
     /* lexical error: interpolation */
     qtest_qmp_send_raw(qts, "%%p");
-    resp = qtest_qmp_receive(qts);
+    resp = qtest_qmp_receive_dict(qts);
     qmp_expect_error_and_unref(resp, "GenericError");
     assert_recovered(qts);
 
@@ -111,7 +111,7 @@ static void test_qmp_protocol(void)
     qts = qtest_init_without_qmp_handshake(common_args);
 
     /* Test greeting */
-    resp = qtest_qmp_receive(qts);
+    resp = qtest_qmp_receive_dict(qts);
     q = qdict_get_qdict(resp, "QMP");
     g_assert(q);
     test_version(qdict_get(q, "version"));
@@ -205,7 +205,7 @@ static void send_oob_cmd_that_fails(QTestState *s, const char *id)
 
 static void recv_cmd_id(QTestState *s, const char *id)
 {
-    QDict *resp = qtest_qmp_receive(s);
+    QDict *resp = qtest_qmp_receive_dict(s);
 
     g_assert_cmpstr(qdict_get_try_str(resp, "id"), ==, id);
     qobject_unref(resp);
@@ -222,7 +222,7 @@ static void test_qmp_oob(void)
     qts = qtest_init_without_qmp_handshake(common_args);
 
     /* Check the greeting message. */
-    resp = qtest_qmp_receive(qts);
+    resp = qtest_qmp_receive_dict(qts);
     q = qdict_get_qdict(resp, "QMP");
     g_assert(q);
     capabilities = qdict_get_qlist(q, "capabilities");
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 23/39] qtest: Reintroduce qtest_qmp_receive
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (21 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 22/39] qtest: rename qtest_qmp_receive to qtest_qmp_receive_dict Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 24/39] qtest: remove qtest_qmp_receive_success Paolo Bonzini
                   ` (17 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Maxim Levitsky

From: Maxim Levitsky <mlevitsk@redhat.com>

The new qtest_qmp_receive buffers all the received qmp events, allowing
qtest_qmp_eventwait_ref to return them.

This is intended to solve the race in regard to ordering of qmp events
vs qmp responses, as soon as the callers start using the new interface.

In addition to that, define qtest_qmp_event_ref a function which only scans
the buffer that qtest_qmp_receive stores the events to.

This is intended for callers that are only interested in events that were
received during the last call to the qtest_qmp_receive.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Message-Id: <20201006123904.610658-3-mlevitsk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/qtest/libqos/libqtest.h | 23 ++++++++++++++++
 tests/qtest/libqtest.c        | 49 ++++++++++++++++++++++++++++++++++-
 2 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/libqos/libqtest.h b/tests/qtest/libqos/libqtest.h
index 9b3f99b322..a2e3961792 100644
--- a/tests/qtest/libqos/libqtest.h
+++ b/tests/qtest/libqos/libqtest.h
@@ -198,6 +198,16 @@ void qtest_qmp_vsend(QTestState *s, const char *fmt, va_list ap)
  */
 QDict *qtest_qmp_receive_dict(QTestState *s);
 
+/**
+ * qtest_qmp_receive:
+ * @s: #QTestState instance to operate on.
+ *
+ * Reads a QMP message from QEMU and returns the response.
+ * Buffers all the events received meanwhile, until a
+ * call to qtest_qmp_eventwait
+ */
+QDict *qtest_qmp_receive(QTestState *s);
+
 /**
  * qtest_qmp_eventwait:
  * @s: #QTestState instance to operate on.
@@ -217,6 +227,19 @@ void qtest_qmp_eventwait(QTestState *s, const char *event);
  */
 QDict *qtest_qmp_eventwait_ref(QTestState *s, const char *event);
 
+/**
+ * qtest_qmp_event_ref:
+ * @s: #QTestState instance to operate on.
+ * @s: #event event to return.
+ *
+ * Removes non-matching events from the buffer that was set by
+ * qtest_qmp_receive, until an event bearing the given name is found,
+ * and returns it.
+ * If no event matches, clears the buffer and returns NULL.
+ *
+ */
+QDict *qtest_qmp_event_ref(QTestState *s, const char *event);
+
 /**
  * qtest_qmp_receive_success:
  * @s: #QTestState instance to operate on
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index dadc465825..d4c49a52ff 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -63,6 +63,7 @@ struct QTestState
     bool irq_level[MAX_IRQ];
     GString *rx;
     QTestTransportOps ops;
+    GList *pending_events;
 };
 
 static GHookList abrt_hooks;
@@ -279,6 +280,7 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
 
     g_test_message("starting QEMU: %s", command);
 
+    s->pending_events = NULL;
     s->wstatus = 0;
     s->expected_status = 0;
     s->qemu_pid = fork();
@@ -386,6 +388,13 @@ void qtest_quit(QTestState *s)
     close(s->fd);
     close(s->qmp_fd);
     g_string_free(s->rx, true);
+
+    for (GList *it = s->pending_events; it != NULL; it = it->next) {
+        qobject_unref((QDict *)it->data);
+    }
+
+    g_list_free(s->pending_events);
+
     g_free(s);
 }
 
@@ -603,6 +612,19 @@ QDict *qmp_fd_receive(int fd)
     return qmp.response;
 }
 
+QDict *qtest_qmp_receive(QTestState *s)
+{
+    while (true) {
+        QDict *response = qtest_qmp_receive_dict(s);
+
+        if (!qdict_get_try_str(response, "event")) {
+            return response;
+        }
+        /* Stash the event for a later consumption */
+        s->pending_events = g_list_prepend(s->pending_events, response);
+    }
+}
+
 QDict *qtest_qmp_receive_dict(QTestState *s)
 {
     return qmp_fd_receive(s->qmp_fd);
@@ -771,10 +793,34 @@ void qtest_qmp_send_raw(QTestState *s, const char *fmt, ...)
     va_end(ap);
 }
 
-QDict *qtest_qmp_eventwait_ref(QTestState *s, const char *event)
+QDict *qtest_qmp_event_ref(QTestState *s, const char *event)
 {
+    GList *next = NULL;
     QDict *response;
 
+    for (GList *it = s->pending_events; it != NULL; it = next) {
+
+        next = it->next;
+        response = (QDict *)it->data;
+
+        s->pending_events = g_list_remove_link(s->pending_events, it);
+
+        if (!strcmp(qdict_get_str(response, "event"), event)) {
+            return response;
+        }
+        qobject_unref(response);
+    }
+    return NULL;
+}
+
+QDict *qtest_qmp_eventwait_ref(QTestState *s, const char *event)
+{
+    QDict *response = qtest_qmp_event_ref(s, event);
+
+    if (response) {
+        return response;
+    }
+
     for (;;) {
         response = qtest_qmp_receive_dict(s);
         if ((qdict_haskey(response, "event")) &&
@@ -1403,6 +1449,7 @@ QTestState *qtest_inproc_init(QTestState **s, bool log, const char* arch,
 {
     QTestState *qts;
     qts = g_new0(QTestState, 1);
+    qts->pending_events = NULL;
     *s = qts; /* Expose qts early on, since the query endianness relies on it */
     qts->wstatus = 0;
     for (int i = 0; i < MAX_IRQ; i++) {
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 24/39] qtest: remove qtest_qmp_receive_success
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (22 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 23/39] qtest: Reintroduce qtest_qmp_receive Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 25/39] device-plug-test: use qtest_qmp to send the device_del command Paolo Bonzini
                   ` (16 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Maxim Levitsky

From: Maxim Levitsky <mlevitsk@redhat.com>

The purpose of qtest_qmp_receive_success was mostly to process events
that arrived between the issueing of a command and the "return"
line from QMP.  This is now handled by the buffering of events
that libqtest performs automatically.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 tests/qtest/libqos/libqtest.h   | 17 -----------
 tests/qtest/libqtest.c          | 53 ++++-----------------------------
 tests/qtest/migration-helpers.c | 25 ++++++++++++----
 3 files changed, 25 insertions(+), 70 deletions(-)

diff --git a/tests/qtest/libqos/libqtest.h b/tests/qtest/libqos/libqtest.h
index a2e3961792..64bb1cd9eb 100644
--- a/tests/qtest/libqos/libqtest.h
+++ b/tests/qtest/libqos/libqtest.h
@@ -240,23 +240,6 @@ QDict *qtest_qmp_eventwait_ref(QTestState *s, const char *event);
  */
 QDict *qtest_qmp_event_ref(QTestState *s, const char *event);
 
-/**
- * qtest_qmp_receive_success:
- * @s: #QTestState instance to operate on
- * @event_cb: Event callback
- * @opaque: Argument for @event_cb
- *
- * Poll QMP messages until a command success response is received.
- * If @event_cb, call it for each event received, passing @opaque,
- * the event's name and data.
- * Return the success response's "return" member.
- */
-QDict *qtest_qmp_receive_success(QTestState *s,
-                                 void (*event_cb)(void *opaque,
-                                                  const char *name,
-                                                  QDict *data),
-                                 void *opaque);
-
 /**
  * qtest_hmp:
  * @s: #QTestState instance to operate on.
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index d4c49a52ff..baac667b8d 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -1291,35 +1291,6 @@ void qtest_cb_for_every_machine(void (*cb)(const char *machine),
     qobject_unref(response);
 }
 
-QDict *qtest_qmp_receive_success(QTestState *s,
-                                 void (*event_cb)(void *opaque,
-                                                  const char *event,
-                                                  QDict *data),
-                                 void *opaque)
-{
-    QDict *response, *ret, *data;
-    const char *event;
-
-    for (;;) {
-        response = qtest_qmp_receive_dict(s);
-        g_assert(!qdict_haskey(response, "error"));
-        ret = qdict_get_qdict(response, "return");
-        if (ret) {
-            break;
-        }
-        event = qdict_get_str(response, "event");
-        data = qdict_get_qdict(response, "data");
-        if (event_cb) {
-            event_cb(opaque, event, data);
-        }
-        qobject_unref(response);
-    }
-
-    qobject_ref(ret);
-    qobject_unref(response);
-    return ret;
-}
-
 /*
  * Generic hot-plugging test via the device_add QMP commands.
  */
@@ -1355,13 +1326,6 @@ void qtest_qmp_device_add(QTestState *qts, const char *driver, const char *id,
     qobject_unref(args);
 }
 
-static void device_deleted_cb(void *opaque, const char *name, QDict *data)
-{
-    bool *got_event = opaque;
-
-    g_assert_cmpstr(name, ==, "DEVICE_DELETED");
-    *got_event = true;
-}
 
 /*
  * Generic hot-unplugging test via the device_del QMP command.
@@ -1378,24 +1342,17 @@ static void device_deleted_cb(void *opaque, const char *name, QDict *data)
  * and this one:
  *
  * {"return": {}}
- *
- * But the order of arrival may vary - so we've got to detect both.
  */
 void qtest_qmp_device_del(QTestState *qts, const char *id)
 {
-    bool got_event = false;
     QDict *rsp;
 
-    qtest_qmp_send(qts, "{'execute': 'device_del', 'arguments': {'id': %s}}",
-                   id);
-    rsp = qtest_qmp_receive_success(qts, device_deleted_cb, &got_event);
+    rsp = qtest_qmp(qts, "{'execute': 'device_del', 'arguments': {'id': %s}}",
+                    id);
+
+    g_assert(qdict_haskey(rsp, "return"));
     qobject_unref(rsp);
-    if (!got_event) {
-        rsp = qtest_qmp_receive_dict(qts);
-        g_assert_cmpstr(qdict_get_try_str(rsp, "event"),
-                        ==, "DEVICE_DELETED");
-        qobject_unref(rsp);
-    }
+    qtest_qmp_eventwait(qts, "DEVICE_DELETED");
 }
 
 bool qmp_rsp_is_err(QDict *rsp)
diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index 516093b39a..b799dbafb7 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -17,10 +17,12 @@
 
 bool got_stop;
 
-static void stop_cb(void *opaque, const char *name, QDict *data)
+static void check_stop_event(QTestState *who)
 {
-    if (!strcmp(name, "STOP")) {
+    QDict *event = qtest_qmp_event_ref(who, "STOP");
+    if (event) {
         got_stop = true;
+        qobject_unref(event);
     }
 }
 
@@ -30,12 +32,19 @@ static void stop_cb(void *opaque, const char *name, QDict *data)
 QDict *wait_command_fd(QTestState *who, int fd, const char *command, ...)
 {
     va_list ap;
+    QDict *resp;
 
     va_start(ap, command);
     qtest_qmp_vsend_fds(who, &fd, 1, command, ap);
     va_end(ap);
 
-    return qtest_qmp_receive_success(who, stop_cb, NULL);
+    resp = qtest_qmp_receive(who);
+    check_stop_event(who);
+
+    g_assert(!qdict_haskey(resp, "error"));
+    g_assert(qdict_haskey(resp, "return"));
+
+    return qdict_get_qdict(resp, "return");
 }
 
 /*
@@ -44,12 +53,18 @@ QDict *wait_command_fd(QTestState *who, int fd, const char *command, ...)
 QDict *wait_command(QTestState *who, const char *command, ...)
 {
     va_list ap;
+    QDict *resp;
 
     va_start(ap, command);
-    qtest_qmp_vsend(who, command, ap);
+    resp = qtest_vqmp(who, command, ap);
     va_end(ap);
 
-    return qtest_qmp_receive_success(who, stop_cb, NULL);
+    check_stop_event(who);
+
+    g_assert(!qdict_haskey(resp, "error"));
+    g_assert(qdict_haskey(resp, "return"));
+
+    return qdict_get_qdict(resp, "return");
 }
 
 /*
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 25/39] device-plug-test: use qtest_qmp to send the device_del command
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (23 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 24/39] qtest: remove qtest_qmp_receive_success Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 26/39] qtest: switch users back to qtest_qmp_receive Paolo Bonzini
                   ` (15 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel

Simplify the code now that events are buffered.  There is no need
anymore to separate sending the command and retrieving the response.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/qtest/device-plug-test.c | 32 +++++++++-----------------------
 1 file changed, 9 insertions(+), 23 deletions(-)

diff --git a/tests/qtest/device-plug-test.c b/tests/qtest/device-plug-test.c
index a2247856be..559d47727a 100644
--- a/tests/qtest/device-plug-test.c
+++ b/tests/qtest/device-plug-test.c
@@ -15,26 +15,17 @@
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qstring.h"
 
-static void device_del_start(QTestState *qtest, const char *id)
+static void device_del(QTestState *qtest, const char *id)
 {
-    qtest_qmp_send(qtest,
-                   "{'execute': 'device_del', 'arguments': { 'id': %s } }", id);
-}
+    QDict *resp;
 
-static void device_del_finish(QTestState *qtest)
-{
-    QDict *resp = qtest_qmp_receive_dict(qtest);
+    resp = qtest_qmp(qtest,
+                     "{'execute': 'device_del', 'arguments': { 'id': %s } }", id);
 
     g_assert(qdict_haskey(resp, "return"));
     qobject_unref(resp);
 }
 
-static void device_del_request(QTestState *qtest, const char *id)
-{
-    device_del_start(qtest, id);
-    device_del_finish(qtest);
-}
-
 static void system_reset(QTestState *qtest)
 {
     QDict *resp;
@@ -79,7 +70,7 @@ static void test_pci_unplug_request(void)
      * be processed. However during system reset, the removal will be
      * handled, removing the device.
      */
-    device_del_request(qtest, "dev0");
+    device_del(qtest, "dev0");
     system_reset(qtest);
     wait_device_deleted_event(qtest, "dev0");
 
@@ -90,13 +81,8 @@ static void test_ccw_unplug(void)
 {
     QTestState *qtest = qtest_initf("-device virtio-balloon-ccw,id=dev0");
 
-    /*
-     * The DEVICE_DELETED events will be sent before the command
-     * completes.
-     */
-    device_del_start(qtest, "dev0");
+    device_del(qtest, "dev0");
     wait_device_deleted_event(qtest, "dev0");
-    device_del_finish(qtest);
 
     qtest_quit(qtest);
 }
@@ -109,7 +95,7 @@ static void test_spapr_cpu_unplug_request(void)
                         "-device power9_v2.0-spapr-cpu-core,core-id=1,id=dev0");
 
     /* similar to test_pci_unplug_request */
-    device_del_request(qtest, "dev0");
+    device_del(qtest, "dev0");
     system_reset(qtest);
     wait_device_deleted_event(qtest, "dev0");
 
@@ -125,7 +111,7 @@ static void test_spapr_memory_unplug_request(void)
                         "-device pc-dimm,id=dev0,memdev=mem0");
 
     /* similar to test_pci_unplug_request */
-    device_del_request(qtest, "dev0");
+    device_del(qtest, "dev0");
     system_reset(qtest);
     wait_device_deleted_event(qtest, "dev0");
 
@@ -139,7 +125,7 @@ static void test_spapr_phb_unplug_request(void)
     qtest = qtest_initf("-device spapr-pci-host-bridge,index=1,id=dev0");
 
     /* similar to test_pci_unplug_request */
-    device_del_request(qtest, "dev0");
+    device_del(qtest, "dev0");
     system_reset(qtest);
     wait_device_deleted_event(qtest, "dev0");
 
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 26/39] qtest: switch users back to qtest_qmp_receive
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (24 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 25/39] device-plug-test: use qtest_qmp to send the device_del command Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 27/39] qtest: check that drives are really appearing and disappearing Paolo Bonzini
                   ` (14 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Maxim Levitsky

From: Maxim Levitsky <mlevitsk@redhat.com>

Let test use the new functionality for buffering events.
The only remaining users of qtest_qmp_receive_dict are tests
that fuzz the QMP protocol.

Tested with 'make check-qtest'.

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Message-Id: <20201006123904.610658-4-mlevitsk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/qtest/ahci-test.c      |  4 ++--
 tests/qtest/drive_del-test.c |  9 +++------
 tests/qtest/libqtest.c       | 12 +++---------
 tests/qtest/pvpanic-test.c   |  4 +---
 tests/qtest/tpm-util.c       |  8 ++++++--
 5 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
index d42ebaeb4c..5e1954852e 100644
--- a/tests/qtest/ahci-test.c
+++ b/tests/qtest/ahci-test.c
@@ -1590,7 +1590,7 @@ static void test_atapi_tray(void)
     qtest_qmp_send(ahci->parent->qts, "{'execute': 'blockdev-open-tray', "
                     "'arguments': {'id': 'cd0'}}");
     atapi_wait_tray(ahci, true);
-    rsp = qtest_qmp_receive_dict(ahci->parent->qts);
+    rsp = qtest_qmp_receive(ahci->parent->qts);
     qobject_unref(rsp);
 
     qmp_discard_response(ahci->parent->qts,
@@ -1620,7 +1620,7 @@ static void test_atapi_tray(void)
     qtest_qmp_send(ahci->parent->qts, "{'execute': 'blockdev-close-tray', "
                    "'arguments': {'id': 'cd0'}}");
     atapi_wait_tray(ahci, false);
-    rsp = qtest_qmp_receive_dict(ahci->parent->qts);
+    rsp = qtest_qmp_receive(ahci->parent->qts);
     qobject_unref(rsp);
 
     /* Now, to convince ATAPI we understand the media has changed... */
diff --git a/tests/qtest/drive_del-test.c b/tests/qtest/drive_del-test.c
index ba0cd77445..9d20a1ed8b 100644
--- a/tests/qtest/drive_del-test.c
+++ b/tests/qtest/drive_del-test.c
@@ -15,9 +15,6 @@
 #include "libqos/virtio.h"
 #include "qapi/qmp/qdict.h"
 
-/* TODO actually test the results and get rid of this */
-#define qmp_discard_response(q, ...) qobject_unref(qtest_qmp(q, __VA_ARGS__))
-
 static void drive_add(QTestState *qts)
 {
     char *resp = qtest_hmp(qts, "drive_add 0 if=none,id=drive0");
@@ -38,13 +35,13 @@ static void device_del(QTestState *qts)
 {
     QDict *response;
 
-    /* Complication: ignore DEVICE_DELETED event */
-    qmp_discard_response(qts, "{'execute': 'device_del',"
+    response = qtest_qmp(qts, "{'execute': 'device_del',"
                          " 'arguments': { 'id': 'dev0' } }");
-    response = qtest_qmp_receive_dict(qts);
     g_assert(response);
     g_assert(qdict_haskey(response, "return"));
     qobject_unref(response);
+
+    qtest_qmp_eventwait(qts, "DEVICE_DELETED");
 }
 
 static void test_drive_without_dev(void)
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index baac667b8d..08929f5ff6 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -324,7 +324,7 @@ QTestState *qtest_init(const char *extra_args)
     QDict *greeting;
 
     /* Read the QMP greeting and then do the handshake */
-    greeting = qtest_qmp_receive_dict(s);
+    greeting = qtest_qmp_receive(s);
     qobject_unref(greeting);
     qobject_unref(qtest_qmp(s, "{ 'execute': 'qmp_capabilities' }"));
 
@@ -700,7 +700,7 @@ QDict *qtest_vqmp_fds(QTestState *s, int *fds, size_t fds_num,
     qtest_qmp_vsend_fds(s, fds, fds_num, fmt, ap);
 
     /* Receive reply */
-    return qtest_qmp_receive_dict(s);
+    return qtest_qmp_receive(s);
 }
 
 QDict *qtest_vqmp(QTestState *s, const char *fmt, va_list ap)
@@ -708,7 +708,7 @@ QDict *qtest_vqmp(QTestState *s, const char *fmt, va_list ap)
     qtest_qmp_vsend(s, fmt, ap);
 
     /* Receive reply */
-    return qtest_qmp_receive_dict(s);
+    return qtest_qmp_receive(s);
 }
 
 QDict *qmp_fd(int fd, const char *fmt, ...)
@@ -850,12 +850,6 @@ char *qtest_vhmp(QTestState *s, const char *fmt, va_list ap)
                      " 'arguments': {'command-line': %s}}",
                      cmd);
     ret = g_strdup(qdict_get_try_str(resp, "return"));
-    while (ret == NULL && qdict_get_try_str(resp, "event")) {
-        /* Ignore asynchronous QMP events */
-        qobject_unref(resp);
-        resp = qtest_qmp_receive_dict(s);
-        ret = g_strdup(qdict_get_try_str(resp, "return"));
-    }
     g_assert(ret);
     qobject_unref(resp);
     g_free(cmd);
diff --git a/tests/qtest/pvpanic-test.c b/tests/qtest/pvpanic-test.c
index b0b20ad340..0657de797f 100644
--- a/tests/qtest/pvpanic-test.c
+++ b/tests/qtest/pvpanic-test.c
@@ -24,9 +24,7 @@ static void test_panic(void)
 
     qtest_outb(qts, 0x505, 0x1);
 
-    response = qtest_qmp_receive_dict(qts);
-    g_assert(qdict_haskey(response, "event"));
-    g_assert_cmpstr(qdict_get_str(response, "event"), ==, "GUEST_PANICKED");
+    response = qtest_qmp_eventwait_ref(qts, "GUEST_PANICKED");
     g_assert(qdict_haskey(response, "data"));
     data = qdict_get_qdict(response, "data");
     g_assert(qdict_haskey(data, "action"));
diff --git a/tests/qtest/tpm-util.c b/tests/qtest/tpm-util.c
index 3ed6c8548a..5a33a6ef0f 100644
--- a/tests/qtest/tpm-util.c
+++ b/tests/qtest/tpm-util.c
@@ -237,12 +237,16 @@ void tpm_util_migrate(QTestState *who, const char *uri)
 void tpm_util_wait_for_migration_complete(QTestState *who)
 {
     while (true) {
+        QDict *rsp;
         QDict *rsp_return;
         bool completed;
         const char *status;
 
-        qtest_qmp_send(who, "{ 'execute': 'query-migrate' }");
-        rsp_return = qtest_qmp_receive_success(who, NULL, NULL);
+        rsp = qtest_qmp(who, "{ 'execute': 'query-migrate' }");
+        g_assert(qdict_haskey(rsp, "return"));
+        rsp_return = qdict_get_qdict(rsp, "return");
+
+        g_assert(!qdict_haskey(rsp_return, "error"));
         status = qdict_get_str(rsp_return, "status");
         completed = strcmp(status, "completed") == 0;
         g_assert_cmpstr(status, !=,  "failed");
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 27/39] qtest: check that drives are really appearing and disappearing
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (25 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 26/39] qtest: switch users back to qtest_qmp_receive Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 28/39] qemu-iotests, qtest: rewrite test 067 as a qtest Paolo Bonzini
                   ` (13 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf

Do not just trust the HMP commands to create and delete the drive, use
query-block to check that this is actually the case.

Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/qtest/drive_del-test.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/drive_del-test.c b/tests/qtest/drive_del-test.c
index 9d20a1ed8b..ff772b3671 100644
--- a/tests/qtest/drive_del-test.c
+++ b/tests/qtest/drive_del-test.c
@@ -14,20 +14,49 @@
 #include "libqos/libqtest.h"
 #include "libqos/virtio.h"
 #include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qlist.h"
+
+static bool has_drive(QTestState *qts)
+{
+    QDict *response;
+    QList *ret;
+    QListEntry *entry;
+    bool found;
+
+    response = qtest_qmp(qts, "{'execute': 'query-block'}");
+    g_assert(response && qdict_haskey(response, "return"));
+    ret = qdict_get_qlist(response, "return");
+
+    found = false;
+    QLIST_FOREACH_ENTRY(ret, entry) {
+        QDict *entry_dict = qobject_to(QDict, entry->value);
+        if (!strcmp(qdict_get_str(entry_dict, "device"), "drive0")) {
+            found = true;
+            break;
+        }
+    }
+
+    qobject_unref(response);
+    return found;
+}
 
 static void drive_add(QTestState *qts)
 {
     char *resp = qtest_hmp(qts, "drive_add 0 if=none,id=drive0");
 
     g_assert_cmpstr(resp, ==, "OK\r\n");
+    g_assert(has_drive(qts));
     g_free(resp);
 }
 
 static void drive_del(QTestState *qts)
 {
-    char *resp = qtest_hmp(qts, "drive_del drive0");
+    char *resp;
 
+    g_assert(has_drive(qts));
+    resp = qtest_hmp(qts, "drive_del drive0");
     g_assert_cmpstr(resp, ==, "");
+    g_assert(!has_drive(qts));
     g_free(resp);
 }
 
@@ -130,6 +159,7 @@ static void test_drive_del_device_del(void)
      */
     drive_del(qts);
     device_del(qts);
+    g_assert(!has_drive(qts));
 
     qtest_quit(qts);
 }
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 28/39] qemu-iotests, qtest: rewrite test 067 as a qtest
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (26 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 27/39] qtest: check that drives are really appearing and disappearing Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 29/39] qdev: add "check if address free" callback for buses Paolo Bonzini
                   ` (12 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf

Test 067 from qemu-iotests is executing QMP commands to hotplug
and hot-unplug disks, devices and blockdevs.  Because the power
of the text-based test harness is limited, it is actually limiting
the checks that it does, for example by skipping DEVICE_DELETED
events.

tests/qtest already has a similar test, drive_del-test.c.
We can merge them, and even reuse some of the existing code in
drive_del-test.c.  This will improve the quality of the test by
covering DEVICE_DELETED events and testing multiple architectures
(therefore covering multiple PCI hotplug mechanisms as well as s390x
virtio-ccw).

The only difference is that the new test will always use null-co:// for
the medium rather than qcow2 or raw, but this should be irrelevant for
what the test is covering.  For example there are no "qemu-img check"
runs in 067 that would check that the file is properly closed.

The new tests requires PCI hot-plug support, so drive_del-test
is moved from qemu-system-ppc to qemu-system-ppc64.

Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 .gitlab-ci.yml               |   2 +-
 tests/qemu-iotests/067       | 157 -------------
 tests/qemu-iotests/067.out   | 414 -----------------------------------
 tests/qemu-iotests/group     |   2 +-
 tests/qtest/drive_del-test.c | 211 ++++++++++++++++--
 tests/qtest/meson.build      |   4 +-
 6 files changed, 191 insertions(+), 599 deletions(-)
 delete mode 100755 tests/qemu-iotests/067
 delete mode 100644 tests/qemu-iotests/067.out

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index a51c89554f..a4cf87481e 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -232,7 +232,7 @@ build-tcg-disabled:
     - ./check -raw 001 002 003 004 005 008 009 010 011 012 021 025 032 033 048
             052 063 077 086 101 104 106 113 148 150 151 152 157 159 160 163
             170 171 183 184 192 194 197 208 215 221 222 226 227 236 253 277
-    - ./check -qcow2 028 051 056 057 058 065 067 068 082 085 091 095 096 102 122
+    - ./check -qcow2 028 051 056 057 058 065 068 082 085 091 095 096 102 122
             124 132 139 142 144 145 151 152 155 157 165 194 196 197 200 202
             208 209 215 216 218 222 227 234 246 247 248 250 254 255 257 258
             260 261 262 263 264 270 272 273 277 279
diff --git a/tests/qemu-iotests/067 b/tests/qemu-iotests/067
deleted file mode 100755
index a63be9cabf..0000000000
--- a/tests/qemu-iotests/067
+++ /dev/null
@@ -1,157 +0,0 @@
-#!/usr/bin/env bash
-#
-# Test automatic deletion of BDSes created by -drive/drive_add
-#
-# Copyright (C) 2013 Red Hat, Inc.
-#
-# This program is free software; you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation; either version 2 of the License, or
-# (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program.  If not, see <http://www.gnu.org/licenses/>.
-#
-
-# creator
-owner=kwolf@redhat.com
-
-seq=`basename $0`
-echo "QA output created by $seq"
-
-status=1	# failure is the default!
-
-# get standard environment, filters and checks
-. ./common.rc
-. ./common.filter
-
-_supported_fmt qcow2
-_supported_proto file
-# Because anything other than 16 would change the output of query-block,
-# and external data files would change the output of
-# query-named-block-nodes
-_unsupported_imgopts 'refcount_bits=\([^1]\|.\([^6]\|$\)\)' data_file
-
-do_run_qemu()
-{
-    echo Testing: "$@"
-    $QEMU -nographic -qmp-pretty stdio -serial none "$@"
-    echo
-}
-
-# Remove QMP events from (pretty-printed) output. Doesn't handle
-# nested dicts correctly, but we don't get any of those in this test.
-_filter_qmp_events()
-{
-    tr '\n' '\t' | sed -e \
-	's/{\s*"timestamp":\s*{[^}]*},\s*"event":[^,}]*\(,\s*"data":\s*{[^}]*}\)\?\s*}\s*//g' \
-	| tr '\t' '\n'
-}
-
-run_qemu()
-{
-    do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qmp | _filter_qemu \
-                          | _filter_actual_image_size \
-                          | _filter_generated_node_ids | _filter_qmp_events \
-                          | _filter_img_info
-}
-
-size=128M
-
-_make_test_img $size
-
-echo
-echo === -drive/-device and device_del ===
-echo
-
-run_qemu -drive file=$TEST_IMG,format=$IMGFMT,if=none,id=disk -device virtio-blk,drive=disk,id=virtio0 <<EOF
-{ "execute": "qmp_capabilities" }
-{ "execute": "query-block" }
-{ "execute": "device_del", "arguments": { "id": "virtio0" } }
-{ "execute": "system_reset" }
-{ "execute": "query-block" }
-{ "execute": "quit" }
-EOF
-
-echo
-echo === -drive/device_add and device_del ===
-echo
-
-run_qemu -drive file=$TEST_IMG,format=$IMGFMT,if=none,id=disk <<EOF
-{ "execute": "qmp_capabilities" }
-{ "execute": "query-block" }
-{ "execute": "device_add",
-   "arguments": { "driver": "virtio-blk", "drive": "disk",
-                  "id": "virtio0" } }
-{ "execute": "device_del", "arguments": { "id": "virtio0" } }
-{ "execute": "system_reset" }
-{ "execute": "query-block" }
-{ "execute": "quit" }
-EOF
-
-echo
-echo === drive_add/device_add and device_del ===
-echo
-
-run_qemu <<EOF
-{ "execute": "qmp_capabilities" }
-{ "execute": "human-monitor-command",
-  "arguments": { "command-line": "drive_add 0 file=$TEST_IMG,format=$IMGFMT,if=none,id=disk" } }
-{ "execute": "query-block" }
-{ "execute": "device_add",
-   "arguments": { "driver": "virtio-blk", "drive": "disk",
-                  "id": "virtio0" } }
-{ "execute": "device_del", "arguments": { "id": "virtio0" } }
-{ "execute": "system_reset" }
-{ "execute": "query-block" }
-{ "execute": "quit" }
-EOF
-
-echo
-echo === blockdev_add/device_add and device_del ===
-echo
-
-run_qemu <<EOF
-{ "execute": "qmp_capabilities" }
-{ "execute": "blockdev-add",
-  "arguments": {
-      "driver": "$IMGFMT",
-      "node-name": "disk",
-      "file": {
-          "driver": "file",
-          "filename": "$TEST_IMG"
-      }
-    }
-  }
-{ "execute": "query-named-block-nodes" }
-{ "execute": "device_add",
-   "arguments": { "driver": "virtio-blk", "drive": "disk",
-                  "id": "virtio0" } }
-{ "execute": "device_del", "arguments": { "id": "virtio0" } }
-{ "execute": "system_reset" }
-{ "execute": "query-named-block-nodes" }
-{ "execute": "quit" }
-EOF
-
-echo
-echo === Empty drive with -device and device_del ===
-echo
-
-run_qemu -device virtio-scsi -device scsi-cd,id=cd0 <<EOF
-{ "execute": "qmp_capabilities" }
-{ "execute": "query-block" }
-{ "execute": "device_del", "arguments": { "id": "cd0" } }
-{ "execute": "system_reset" }
-{ "execute": "query-block" }
-{ "execute": "quit" }
-EOF
-
-# success, all done
-echo "*** done"
-rm -f $seq.full
-status=0
diff --git a/tests/qemu-iotests/067.out b/tests/qemu-iotests/067.out
deleted file mode 100644
index b10c71db03..0000000000
--- a/tests/qemu-iotests/067.out
+++ /dev/null
@@ -1,414 +0,0 @@
-QA output created by 067
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
-
-=== -drive/-device and device_del ===
-
-Testing: -drive file=TEST_DIR/t.IMGFMT,format=IMGFMT,if=none,id=disk -device virtio-blk,drive=disk,id=virtio0
-{
-    QMP_VERSION
-}
-{
-    "return": {
-    }
-}
-{
-    "return": [
-        {
-            "io-status": "ok",
-            "device": "disk",
-            "locked": false,
-            "removable": false,
-            "inserted": {
-                "iops_rd": 0,
-                "detect_zeroes": "off",
-                "image": {
-                    "virtual-size": 134217728,
-                    "filename": "TEST_DIR/t.IMGFMT",
-                    "cluster-size": 65536,
-                    "format": "IMGFMT",
-                    "actual-size": SIZE,
-                    "dirty-flag": false
-                },
-                "iops_wr": 0,
-                "ro": false,
-                "node-name": "NODE_NAME",
-                "backing_file_depth": 0,
-                "drv": "IMGFMT",
-                "iops": 0,
-                "bps_wr": 0,
-                "write_threshold": 0,
-                "encrypted": false,
-                "bps": 0,
-                "bps_rd": 0,
-                "cache": {
-                    "no-flush": false,
-                    "direct": false,
-                    "writeback": true
-                },
-                "file": "TEST_DIR/t.IMGFMT",
-                "encryption_key_missing": false
-            },
-            "qdev": "/machine/peripheral/virtio0/virtio-backend",
-            "type": "unknown"
-        }
-    ]
-}
-{
-    "return": {
-    }
-}
-{
-    "return": {
-    }
-}
-{
-    "return": [
-    ]
-}
-{
-    "return": {
-    }
-}
-
-=== -drive/device_add and device_del ===
-
-Testing: -drive file=TEST_DIR/t.IMGFMT,format=IMGFMT,if=none,id=disk
-{
-    QMP_VERSION
-}
-{
-    "return": {
-    }
-}
-{
-    "return": [
-        {
-            "device": "disk",
-            "locked": false,
-            "removable": true,
-            "inserted": {
-                "iops_rd": 0,
-                "detect_zeroes": "off",
-                "image": {
-                    "virtual-size": 134217728,
-                    "filename": "TEST_DIR/t.IMGFMT",
-                    "cluster-size": 65536,
-                    "format": "IMGFMT",
-                    "actual-size": SIZE,
-                    "dirty-flag": false
-                },
-                "iops_wr": 0,
-                "ro": false,
-                "node-name": "NODE_NAME",
-                "backing_file_depth": 0,
-                "drv": "IMGFMT",
-                "iops": 0,
-                "bps_wr": 0,
-                "write_threshold": 0,
-                "encrypted": false,
-                "bps": 0,
-                "bps_rd": 0,
-                "cache": {
-                    "no-flush": false,
-                    "direct": false,
-                    "writeback": true
-                },
-                "file": "TEST_DIR/t.IMGFMT",
-                "encryption_key_missing": false
-            },
-            "type": "unknown"
-        }
-    ]
-}
-{
-    "return": {
-    }
-}
-{
-    "return": {
-    }
-}
-{
-    "return": {
-    }
-}
-{
-    "return": [
-    ]
-}
-{
-    "return": {
-    }
-}
-
-=== drive_add/device_add and device_del ===
-
-Testing:
-{
-    QMP_VERSION
-}
-{
-    "return": {
-    }
-}
-{
-    "return": "OK\r\n"
-}
-{
-    "return": [
-        {
-            "device": "disk",
-            "locked": false,
-            "removable": true,
-            "inserted": {
-                "iops_rd": 0,
-                "detect_zeroes": "off",
-                "image": {
-                    "virtual-size": 134217728,
-                    "filename": "TEST_DIR/t.IMGFMT",
-                    "cluster-size": 65536,
-                    "format": "IMGFMT",
-                    "actual-size": SIZE,
-                    "dirty-flag": false
-                },
-                "iops_wr": 0,
-                "ro": false,
-                "node-name": "NODE_NAME",
-                "backing_file_depth": 0,
-                "drv": "IMGFMT",
-                "iops": 0,
-                "bps_wr": 0,
-                "write_threshold": 0,
-                "encrypted": false,
-                "bps": 0,
-                "bps_rd": 0,
-                "cache": {
-                    "no-flush": false,
-                    "direct": false,
-                    "writeback": true
-                },
-                "file": "TEST_DIR/t.IMGFMT",
-                "encryption_key_missing": false
-            },
-            "type": "unknown"
-        }
-    ]
-}
-{
-    "return": {
-    }
-}
-{
-    "return": {
-    }
-}
-{
-    "return": {
-    }
-}
-{
-    "return": [
-    ]
-}
-{
-    "return": {
-    }
-}
-
-=== blockdev_add/device_add and device_del ===
-
-Testing:
-{
-    QMP_VERSION
-}
-{
-    "return": {
-    }
-}
-{
-    "return": {
-    }
-}
-{
-    "return": [
-        {
-            "iops_rd": 0,
-            "detect_zeroes": "off",
-            "image": {
-                "virtual-size": 134217728,
-                "filename": "TEST_DIR/t.IMGFMT",
-                "cluster-size": 65536,
-                "format": "IMGFMT",
-                "actual-size": SIZE,
-                "dirty-flag": false
-            },
-            "iops_wr": 0,
-            "ro": false,
-            "node-name": "disk",
-            "backing_file_depth": 0,
-            "drv": "IMGFMT",
-            "iops": 0,
-            "bps_wr": 0,
-            "write_threshold": 0,
-            "encrypted": false,
-            "bps": 0,
-            "bps_rd": 0,
-            "cache": {
-                "no-flush": false,
-                "direct": false,
-                "writeback": true
-            },
-            "file": "TEST_DIR/t.IMGFMT",
-            "encryption_key_missing": false
-        },
-        {
-            "iops_rd": 0,
-            "detect_zeroes": "off",
-            "image": {
-                "virtual-size": 197120,
-                "filename": "TEST_DIR/t.IMGFMT",
-                "format": "file",
-                "actual-size": SIZE,
-                "dirty-flag": false
-            },
-            "iops_wr": 0,
-            "ro": false,
-            "node-name": "NODE_NAME",
-            "backing_file_depth": 0,
-            "drv": "file",
-            "iops": 0,
-            "bps_wr": 0,
-            "write_threshold": 0,
-            "encrypted": false,
-            "bps": 0,
-            "bps_rd": 0,
-            "cache": {
-                "no-flush": false,
-                "direct": false,
-                "writeback": true
-            },
-            "file": "TEST_DIR/t.IMGFMT",
-            "encryption_key_missing": false
-        }
-    ]
-}
-{
-    "return": {
-    }
-}
-{
-    "return": {
-    }
-}
-{
-    "return": {
-    }
-}
-{
-    "return": [
-        {
-            "iops_rd": 0,
-            "detect_zeroes": "off",
-            "image": {
-                "virtual-size": 134217728,
-                "filename": "TEST_DIR/t.IMGFMT",
-                "cluster-size": 65536,
-                "format": "IMGFMT",
-                "actual-size": SIZE,
-                "dirty-flag": false
-            },
-            "iops_wr": 0,
-            "ro": false,
-            "node-name": "disk",
-            "backing_file_depth": 0,
-            "drv": "IMGFMT",
-            "iops": 0,
-            "bps_wr": 0,
-            "write_threshold": 0,
-            "encrypted": false,
-            "bps": 0,
-            "bps_rd": 0,
-            "cache": {
-                "no-flush": false,
-                "direct": false,
-                "writeback": true
-            },
-            "file": "TEST_DIR/t.IMGFMT",
-            "encryption_key_missing": false
-        },
-        {
-            "iops_rd": 0,
-            "detect_zeroes": "off",
-            "image": {
-                "virtual-size": 197120,
-                "filename": "TEST_DIR/t.IMGFMT",
-                "format": "file",
-                "actual-size": SIZE,
-                "dirty-flag": false
-            },
-            "iops_wr": 0,
-            "ro": false,
-            "node-name": "NODE_NAME",
-            "backing_file_depth": 0,
-            "drv": "file",
-            "iops": 0,
-            "bps_wr": 0,
-            "write_threshold": 0,
-            "encrypted": false,
-            "bps": 0,
-            "bps_rd": 0,
-            "cache": {
-                "no-flush": false,
-                "direct": false,
-                "writeback": true
-            },
-            "file": "TEST_DIR/t.IMGFMT",
-            "encryption_key_missing": false
-        }
-    ]
-}
-{
-    "return": {
-    }
-}
-
-=== Empty drive with -device and device_del ===
-
-Testing: -device virtio-scsi -device scsi-cd,id=cd0
-{
-    QMP_VERSION
-}
-{
-    "return": {
-    }
-}
-{
-    "return": [
-        {
-            "io-status": "ok",
-            "device": "",
-            "locked": false,
-            "removable": true,
-            "qdev": "cd0",
-            "tray_open": false,
-            "type": "unknown"
-        }
-    ]
-}
-{
-    "return": {
-    }
-}
-{
-    "return": {
-    }
-}
-{
-    "return": [
-    ]
-}
-{
-    "return": {
-    }
-}
-*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 9e4f7c0153..3432989283 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -88,7 +88,7 @@
 064 rw quick
 065 rw quick
 066 rw auto quick
-067 rw quick
+# 067 was removed, do not reuse
 068 rw quick
 069 rw auto quick
 070 rw quick
diff --git a/tests/qtest/drive_del-test.c b/tests/qtest/drive_del-test.c
index ff772b3671..8d08ee9995 100644
--- a/tests/qtest/drive_del-test.c
+++ b/tests/qtest/drive_del-test.c
@@ -16,21 +16,21 @@
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qlist.h"
 
-static bool has_drive(QTestState *qts)
+static bool look_for_drive0(QTestState *qts, const char *command, const char *key)
 {
     QDict *response;
     QList *ret;
     QListEntry *entry;
     bool found;
 
-    response = qtest_qmp(qts, "{'execute': 'query-block'}");
+    response = qtest_qmp(qts, "{'execute': %s}", command);
     g_assert(response && qdict_haskey(response, "return"));
     ret = qdict_get_qlist(response, "return");
 
     found = false;
     QLIST_FOREACH_ENTRY(ret, entry) {
         QDict *entry_dict = qobject_to(QDict, entry->value);
-        if (!strcmp(qdict_get_str(entry_dict, "device"), "drive0")) {
+        if (!strcmp(qdict_get_str(entry_dict, key), "drive0")) {
             found = true;
             break;
         }
@@ -40,6 +40,38 @@ static bool has_drive(QTestState *qts)
     return found;
 }
 
+static bool has_drive(QTestState *qts)
+{
+    return look_for_drive0(qts, "query-block", "device");
+}
+
+static bool has_blockdev(QTestState *qts)
+{
+    return look_for_drive0(qts, "query-named-block-nodes", "node-name");
+}
+
+static void blockdev_add_with_media(QTestState *qts)
+{
+    QDict *response;
+
+    response = qtest_qmp(qts,
+                         "{ 'execute': 'blockdev-add',"
+                         "  'arguments': {"
+                         "      'driver': 'raw',"
+                         "      'node-name': 'drive0',"
+                         "      'file': {"
+                         "          'driver': 'null-co',"
+                         "          'read-zeroes': true"
+                         "      }"
+                         "  }"
+                         "}");
+
+    g_assert(response);
+    g_assert(qdict_haskey(response, "return"));
+    qobject_unref(response);
+    g_assert(has_blockdev(qts));
+}
+
 static void drive_add(QTestState *qts)
 {
     char *resp = qtest_hmp(qts, "drive_add 0 if=none,id=drive0");
@@ -49,6 +81,17 @@ static void drive_add(QTestState *qts)
     g_free(resp);
 }
 
+static void drive_add_with_media(QTestState *qts)
+{
+    char *resp = qtest_hmp(qts,
+                           "drive_add 0 if=none,id=drive0,file=null-co://,"
+                           "file.read-zeroes=on,format=raw");
+
+    g_assert_cmpstr(resp, ==, "OK\r\n");
+    g_assert(has_drive(qts));
+    g_free(resp);
+}
+
 static void drive_del(QTestState *qts)
 {
     char *resp;
@@ -60,7 +103,43 @@ static void drive_del(QTestState *qts)
     g_free(resp);
 }
 
-static void device_del(QTestState *qts)
+/*
+ * qvirtio_get_dev_type:
+ * Returns: the preferred virtio bus/device type for the current architecture.
+ * TODO: delete this
+ */
+static const char *qvirtio_get_dev_type(void)
+{
+    const char *arch = qtest_get_arch();
+
+    if (g_str_equal(arch, "arm") || g_str_equal(arch, "aarch64")) {
+        return "device";  /* for virtio-mmio */
+    } else if (g_str_equal(arch, "s390x")) {
+        return "ccw";
+    } else {
+        return "pci";
+    }
+}
+
+static void device_add(QTestState *qts)
+{
+    QDict *response;
+    char driver[32];
+    snprintf(driver, sizeof(driver), "virtio-blk-%s",
+             qvirtio_get_dev_type());
+
+    response = qtest_qmp(qts, "{'execute': 'device_add',"
+                              " 'arguments': {"
+                              "   'driver': %s,"
+                              "   'drive': 'drive0',"
+                              "   'id': 'dev0'"
+                              "}}", driver);
+    g_assert(response);
+    g_assert(qdict_haskey(response, "return"));
+    qobject_unref(response);
+}
+
+static void device_del(QTestState *qts, bool and_reset)
 {
     QDict *response;
 
@@ -70,6 +149,13 @@ static void device_del(QTestState *qts)
     g_assert(qdict_haskey(response, "return"));
     qobject_unref(response);
 
+    if (and_reset) {
+        response = qtest_qmp(qts, "{'execute': 'system_reset' }");
+        g_assert(response);
+        g_assert(qdict_haskey(response, "return"));
+        qobject_unref(response);
+    }
+
     qtest_qmp_eventwait(qts, "DEVICE_DELETED");
 }
 
@@ -91,24 +177,6 @@ static void test_drive_without_dev(void)
     qtest_quit(qts);
 }
 
-/*
- * qvirtio_get_dev_type:
- * Returns: the preferred virtio bus/device type for the current architecture.
- * TODO: delete this
- */
-static const char *qvirtio_get_dev_type(void)
-{
-    const char *arch = qtest_get_arch();
-
-    if (g_str_equal(arch, "arm") || g_str_equal(arch, "aarch64")) {
-        return "device";  /* for virtio-mmio */
-    } else if (g_str_equal(arch, "s390x")) {
-        return "ccw";
-    } else {
-        return "pci";
-    }
-}
-
 static void test_after_failed_device_add(void)
 {
     char driver[32];
@@ -158,12 +226,97 @@ static void test_drive_del_device_del(void)
      * Doing it in this order takes notoriously tricky special paths
      */
     drive_del(qts);
-    device_del(qts);
+    device_del(qts, false);
     g_assert(!has_drive(qts));
 
     qtest_quit(qts);
 }
 
+static void test_cli_device_del(void)
+{
+    QTestState *qts;
+
+    /*
+     * -drive/-device and device_del.  Start with a drive used by a
+     * device that unplugs after reset.
+     */
+    qts = qtest_initf("-drive if=none,id=drive0,file=null-co://,"
+                      "file.read-zeroes=on,format=raw"
+                      " -device virtio-blk-%s,drive=drive0,id=dev0",
+                      qvirtio_get_dev_type());
+
+    device_del(qts, true);
+    g_assert(!has_drive(qts));
+
+    qtest_quit(qts);
+}
+
+static void test_empty_device_del(void)
+{
+    QTestState *qts;
+
+    /* device_del with no drive plugged.  */
+    qts = qtest_initf("-device virtio-scsi-%s -device scsi-cd,id=dev0",
+                      qvirtio_get_dev_type());
+
+    device_del(qts, false);
+    qtest_quit(qts);
+}
+
+static void test_device_add_and_del(void)
+{
+    QTestState *qts;
+
+    /*
+     * -drive/device_add and device_del.  Start with a drive used by a
+     * device that unplugs after reset.
+     */
+    qts = qtest_init("-drive if=none,id=drive0,file=null-co://,"
+                     "file.read-zeroes=on,format=raw");
+
+    device_add(qts);
+    device_del(qts, true);
+    g_assert(!has_drive(qts));
+
+    qtest_quit(qts);
+}
+
+static void test_drive_add_device_add_and_del(void)
+{
+    QTestState *qts;
+
+    qts = qtest_init("");
+
+    /*
+     * drive_add/device_add and device_del.  The drive is used by a
+     * device that unplugs after reset.
+     */
+    drive_add_with_media(qts);
+    device_add(qts);
+    device_del(qts, true);
+    g_assert(!has_drive(qts));
+
+    qtest_quit(qts);
+}
+
+static void test_blockdev_add_device_add_and_del(void)
+{
+    QTestState *qts;
+
+    qts = qtest_init("");
+
+    /*
+     * blockdev_add/device_add and device_del.  The it drive is used by a
+     * device that unplugs after reset, but it doesn't go away.
+     */
+    blockdev_add_with_media(qts);
+    device_add(qts);
+    device_del(qts, true);
+    g_assert(has_blockdev(qts));
+
+    qtest_quit(qts);
+}
+
 int main(int argc, char **argv)
 {
     g_test_init(&argc, &argv, NULL);
@@ -173,8 +326,18 @@ int main(int argc, char **argv)
     if (qvirtio_get_dev_type() != NULL) {
         qtest_add_func("/drive_del/after_failed_device_add",
                        test_after_failed_device_add);
-        qtest_add_func("/blockdev/drive_del_device_del",
+        qtest_add_func("/drive_del/drive_del_device_del",
                        test_drive_del_device_del);
+        qtest_add_func("/device_del/drive/cli_device",
+                       test_cli_device_del);
+        qtest_add_func("/device_del/drive/device_add",
+                       test_device_add_and_del);
+        qtest_add_func("/device_del/drive/drive_add_device_add",
+                       test_drive_add_device_add_and_del);
+        qtest_add_func("/device_del/empty",
+                       test_empty_device_del);
+        qtest_add_func("/device_del/blockdev",
+                       test_blockdev_add_device_add_and_del);
     }
 
     return g_test_run();
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 28bafc02a2..1c4b87e3e2 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -111,7 +111,7 @@ qtests_moxie = [ 'boot-serial-test' ]
 qtests_ppc = \
   (config_all_devices.has_key('CONFIG_ISA_TESTDEV') ? ['endianness-test'] : []) +            \
   (config_all_devices.has_key('CONFIG_M48T59') ? ['m48t59-test'] : []) +                     \
-  ['boot-order-test', 'prom-env-test', 'drive_del-test', 'boot-serial-test']                 \
+  ['boot-order-test', 'prom-env-test', 'boot-serial-test']                 \
 
 qtests_ppc64 = \
   (config_all_devices.has_key('CONFIG_PSERIES') ? ['device-plug-test'] : []) +               \
@@ -121,7 +121,7 @@ qtests_ppc64 = \
   (config_all_devices.has_key('CONFIG_USB_UHCI') ? ['usb-hcd-uhci-test'] : []) +             \
   (config_all_devices.has_key('CONFIG_USB_XHCI_NEC') ? ['usb-hcd-xhci-test'] : []) +         \
   (config_host.has_key('CONFIG_POSIX') ? ['test-filter-mirror'] : []) +                      \
-  qtests_pci + ['migration-test', 'numa-test', 'cpu-plug-test']
+  qtests_pci + ['migration-test', 'numa-test', 'cpu-plug-test', 'drive_del-test']
 
 qtests_sh4 = (config_all_devices.has_key('CONFIG_ISA_TESTDEV') ? ['endianness-test'] : [])
 qtests_sh4eb = (config_all_devices.has_key('CONFIG_ISA_TESTDEV') ? ['endianness-test'] : [])
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 29/39] qdev: add "check if address free" callback for buses
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (27 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 28/39] qemu-iotests, qtest: rewrite test 067 as a qtest Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 30/39] scsi/scsi_bus: switch search direction in scsi_device_find Paolo Bonzini
                   ` (11 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel

Check if an address is free on the bus before plugging in the
device.  This makes it possible to do the check without any
side effects, and to detect the problem early without having
to do it in the realize callback.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20201006123904.610658-5-mlevitsk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/qdev.c         | 17 +++++++++++++++--
 hw/net/virtio-net.c    |  2 +-
 hw/sd/core.c           |  3 ++-
 include/hw/qdev-core.h | 13 ++++++++++++-
 4 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 96772a15bd..74db78df36 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -94,13 +94,23 @@ static void bus_add_child(BusState *bus, DeviceState *child)
                              0);
 }
 
-void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
+static bool bus_check_address(BusState *bus, DeviceState *child, Error **errp)
+{
+    BusClass *bc = BUS_GET_CLASS(bus);
+    return !bc->check_address || bc->check_address(bus, child, errp);
+}
+
+bool qdev_set_parent_bus(DeviceState *dev, BusState *bus, Error **errp)
 {
     BusState *old_parent_bus = dev->parent_bus;
     DeviceClass *dc = DEVICE_GET_CLASS(dev);
 
     assert(dc->bus_type && object_dynamic_cast(OBJECT(bus), dc->bus_type));
 
+    if (!bus_check_address(bus, dev, errp)) {
+        return false;
+    }
+
     if (old_parent_bus) {
         trace_qdev_update_parent_bus(dev, object_get_typename(OBJECT(dev)),
             old_parent_bus, object_get_typename(OBJECT(old_parent_bus)),
@@ -126,6 +136,7 @@ void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
         object_unref(OBJECT(old_parent_bus));
         object_unref(OBJECT(dev));
     }
+    return true;
 }
 
 DeviceState *qdev_new(const char *name)
@@ -371,7 +382,9 @@ bool qdev_realize(DeviceState *dev, BusState *bus, Error **errp)
     assert(!dev->realized && !dev->parent_bus);
 
     if (bus) {
-        qdev_set_parent_bus(dev, bus);
+        if (!qdev_set_parent_bus(dev, bus, errp)) {
+            return false;
+        }
     } else {
         assert(!DEVICE_GET_CLASS(dev)->bus_type);
     }
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index a160a9da9c..277289d56e 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3138,7 +3138,7 @@ static bool failover_replug_primary(VirtIONet *n, Error **errp)
         error_setg(errp, "virtio_net: couldn't find primary bus");
         return false;
     }
-    qdev_set_parent_bus(n->primary_dev, n->primary_bus);
+    qdev_set_parent_bus(n->primary_dev, n->primary_bus, &error_abort);
     n->primary_should_be_hidden = false;
     if (!qemu_opt_set_bool(n->primary_device_opts,
                            "partially_hotplugged", true, errp)) {
diff --git a/hw/sd/core.c b/hw/sd/core.c
index 957d116f1a..08c93b5903 100644
--- a/hw/sd/core.c
+++ b/hw/sd/core.c
@@ -23,6 +23,7 @@
 #include "hw/qdev-core.h"
 #include "hw/sd/sd.h"
 #include "qemu/module.h"
+#include "qapi/error.h"
 #include "trace.h"
 
 static inline const char *sdbus_name(SDBus *sdbus)
@@ -240,7 +241,7 @@ void sdbus_reparent_card(SDBus *from, SDBus *to)
     readonly = sc->get_readonly(card);
 
     sdbus_set_inserted(from, false);
-    qdev_set_parent_bus(DEVICE(card), &to->qbus);
+    qdev_set_parent_bus(DEVICE(card), &to->qbus, &error_abort);
     sdbus_set_inserted(to, true);
     sdbus_set_readonly(to, readonly);
 }
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 72064f4dd4..14d476c587 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -210,13 +210,24 @@ struct BusClass {
     /* FIXME first arg should be BusState */
     void (*print_dev)(Monitor *mon, DeviceState *dev, int indent);
     char *(*get_dev_path)(DeviceState *dev);
+
     /*
      * This callback is used to create Open Firmware device path in accordance
      * with OF spec http://forthworks.com/standards/of1275.pdf. Individual bus
      * bindings can be found at http://playground.sun.com/1275/bindings/.
      */
     char *(*get_fw_dev_path)(DeviceState *dev);
+
     void (*reset)(BusState *bus);
+
+    /*
+     * Return whether the device can be added to @bus,
+     * based on the address that was set (via device properties)
+     * before realize.  If not, on return @errp contains the
+     * human-readable error message.
+     */
+    bool (*check_address)(BusState *bus, DeviceState *dev, Error **errp);
+
     BusRealize realize;
     BusUnrealize unrealize;
 
@@ -788,7 +799,7 @@ const char *qdev_fw_name(DeviceState *dev);
 Object *qdev_get_machine(void);
 
 /* FIXME: make this a link<> */
-void qdev_set_parent_bus(DeviceState *dev, BusState *bus);
+bool qdev_set_parent_bus(DeviceState *dev, BusState *bus, Error **errp);
 
 extern bool qdev_hotplug;
 extern bool qdev_hot_removed;
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 30/39] scsi/scsi_bus: switch search direction in scsi_device_find
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (28 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 29/39] qdev: add "check if address free" callback for buses Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 31/39] device_core: use drain_call_rcu in in qmp_device_add Paolo Bonzini
                   ` (10 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Stefan Hajnoczi, Maxim Levitsky

From: Maxim Levitsky <mlevitsk@redhat.com>

This change will allow us to convert the bus children list to RCU,
while not changing the logic of this function

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20200913160259.32145-2-mlevitsk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/scsi-bus.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 3284a5d1fb..6b1ed7ae9a 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -1572,7 +1572,7 @@ SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int id, int lun)
     BusChild *kid;
     SCSIDevice *target_dev = NULL;
 
-    QTAILQ_FOREACH_REVERSE(kid, &bus->qbus.children, sibling) {
+    QTAILQ_FOREACH(kid, &bus->qbus.children, sibling) {
         DeviceState *qdev = kid->child;
         SCSIDevice *dev = SCSI_DEVICE(qdev);
 
@@ -1580,7 +1580,15 @@ SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int id, int lun)
             if (dev->lun == lun) {
                 return dev;
             }
-            target_dev = dev;
+
+            /*
+             * If we don't find exact match (channel/bus/lun),
+             * we will return the first device which matches channel/bus
+             */
+
+            if (!target_dev) {
+                target_dev = dev;
+            }
         }
     }
     return target_dev;
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 31/39] device_core: use drain_call_rcu in in qmp_device_add
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (29 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 30/39] scsi/scsi_bus: switch search direction in scsi_device_find Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 32/39] device-core: use RCU for list of children of a bus Paolo Bonzini
                   ` (9 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Stefan Hajnoczi, Stefan Hajnoczi, Maxim Levitsky

From: Maxim Levitsky <mlevitsk@redhat.com>

Soon, a device removal might only happen on RCU callback execution.
This is okay for device-del which provides a DEVICE_DELETED event,
but not for the failure case of device-add.  To avoid changing
monitor semantics, just drain all pending RCU callbacks on error.

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Suggested-by: Stefan Hajnoczi <stefanha@gmail.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20200913160259.32145-4-mlevitsk@redhat.com>
[Don't use it in qmp_device_del. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 softmmu/qdev-monitor.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index e9b7228480..bcfb90a08f 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -803,6 +803,18 @@ void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp)
         return;
     }
     dev = qdev_device_add(opts, errp);
+
+    /*
+     * Drain all pending RCU callbacks. This is done because
+     * some bus related operations can delay a device removal
+     * (in this case this can happen if device is added and then
+     * removed due to a configuration error)
+     * to a RCU callback, but user might expect that this interface
+     * will finish its job completely once qmp command returns result
+     * to the user
+     */
+    drain_call_rcu();
+
     if (!dev) {
         qemu_opts_del(opts);
         return;
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 32/39] device-core: use RCU for list of children of a bus
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (30 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 31/39] device_core: use drain_call_rcu in in qmp_device_add Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 33/39] scsi: switch to bus->check_address Paolo Bonzini
                   ` (8 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Stefan Hajnoczi, Maxim Levitsky

From: Maxim Levitsky <mlevitsk@redhat.com>

This fixes the race between device emulation code that tries to find
a child device to dispatch the request to (e.g a scsi disk),
and hotplug of a new device to that bus.

Note that this doesn't convert all the readers of the list
but only these that might go over that list without BQL held.

This is a very small first step to make this code thread safe.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20200913160259.32145-5-mlevitsk@redhat.com>
[Use RCU_READ_LOCK_GUARD in more places, adjust testcase now that
 the delay in DEVICE_DELETED due to RCU is more consistent. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20201006123904.610658-9-mlevitsk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/bus.c          | 28 +++++++++++++++++-----------
 hw/core/qdev.c         | 37 +++++++++++++++++++++++--------------
 hw/scsi/scsi-bus.c     | 12 +++++++++---
 hw/scsi/virtio-scsi.c  |  6 +++++-
 include/hw/qdev-core.h |  9 +++++++++
 5 files changed, 63 insertions(+), 29 deletions(-)

diff --git a/hw/core/bus.c b/hw/core/bus.c
index 6b987b6946..a0483859ae 100644
--- a/hw/core/bus.c
+++ b/hw/core/bus.c
@@ -49,12 +49,14 @@ int qbus_walk_children(BusState *bus,
         }
     }
 
-    QTAILQ_FOREACH(kid, &bus->children, sibling) {
-        err = qdev_walk_children(kid->child,
-                                 pre_devfn, pre_busfn,
-                                 post_devfn, post_busfn, opaque);
-        if (err < 0) {
-            return err;
+    WITH_RCU_READ_LOCK_GUARD() {
+        QTAILQ_FOREACH_RCU(kid, &bus->children, sibling) {
+            err = qdev_walk_children(kid->child,
+                                     pre_devfn, pre_busfn,
+                                     post_devfn, post_busfn, opaque);
+            if (err < 0) {
+                return err;
+            }
         }
     }
 
@@ -90,8 +92,10 @@ static void bus_reset_child_foreach(Object *obj, ResettableChildCallback cb,
     BusState *bus = BUS(obj);
     BusChild *kid;
 
-    QTAILQ_FOREACH(kid, &bus->children, sibling) {
-        cb(OBJECT(kid->child), opaque, type);
+    WITH_RCU_READ_LOCK_GUARD() {
+        QTAILQ_FOREACH_RCU(kid, &bus->children, sibling) {
+            cb(OBJECT(kid->child), opaque, type);
+        }
     }
 }
 
@@ -194,9 +198,11 @@ static void bus_set_realized(Object *obj, bool value, Error **errp)
 
         /* TODO: recursive realization */
     } else if (!value && bus->realized) {
-        QTAILQ_FOREACH(kid, &bus->children, sibling) {
-            DeviceState *dev = kid->child;
-            qdev_unrealize(dev);
+        WITH_RCU_READ_LOCK_GUARD() {
+            QTAILQ_FOREACH_RCU(kid, &bus->children, sibling) {
+                DeviceState *dev = kid->child;
+                qdev_unrealize(dev);
+            }
         }
         if (bc->unrealize) {
             bc->unrealize(bus);
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 74db78df36..59e5e710b7 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -51,6 +51,12 @@ const VMStateDescription *qdev_get_vmsd(DeviceState *dev)
     return dc->vmsd;
 }
 
+static void bus_free_bus_child(BusChild *kid)
+{
+    object_unref(OBJECT(kid->child));
+    g_free(kid);
+}
+
 static void bus_remove_child(BusState *bus, DeviceState *child)
 {
     BusChild *kid;
@@ -60,15 +66,16 @@ static void bus_remove_child(BusState *bus, DeviceState *child)
             char name[32];
 
             snprintf(name, sizeof(name), "child[%d]", kid->index);
-            QTAILQ_REMOVE(&bus->children, kid, sibling);
+            QTAILQ_REMOVE_RCU(&bus->children, kid, sibling);
 
             bus->num_children--;
 
             /* This gives back ownership of kid->child back to us.  */
             object_property_del(OBJECT(bus), name);
-            object_unref(OBJECT(kid->child));
-            g_free(kid);
-            return;
+
+            /* free the bus kid, when it is safe to do so*/
+            call_rcu(kid, bus_free_bus_child, rcu);
+            break;
         }
     }
 }
@@ -83,7 +90,7 @@ static void bus_add_child(BusState *bus, DeviceState *child)
     kid->child = child;
     object_ref(OBJECT(kid->child));
 
-    QTAILQ_INSERT_HEAD(&bus->children, kid, sibling);
+    QTAILQ_INSERT_HEAD_RCU(&bus->children, kid, sibling);
 
     /* This transfers ownership of kid->child to the property.  */
     snprintf(name, sizeof(name), "child[%d]", kid->index);
@@ -672,17 +679,19 @@ DeviceState *qdev_find_recursive(BusState *bus, const char *id)
     DeviceState *ret;
     BusState *child;
 
-    QTAILQ_FOREACH(kid, &bus->children, sibling) {
-        DeviceState *dev = kid->child;
+    WITH_RCU_READ_LOCK_GUARD() {
+        QTAILQ_FOREACH_RCU(kid, &bus->children, sibling) {
+            DeviceState *dev = kid->child;
 
-        if (dev->id && strcmp(dev->id, id) == 0) {
-            return dev;
-        }
+            if (dev->id && strcmp(dev->id, id) == 0) {
+                return dev;
+            }
 
-        QLIST_FOREACH(child, &dev->child_bus, sibling) {
-            ret = qdev_find_recursive(child, id);
-            if (ret) {
-                return ret;
+            QLIST_FOREACH(child, &dev->child_bus, sibling) {
+                ret = qdev_find_recursive(child, id);
+                if (ret) {
+                    return ret;
+                }
             }
         }
     }
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 6b1ed7ae9a..4cf1f404b4 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -400,7 +400,10 @@ static bool scsi_target_emulate_report_luns(SCSITargetReq *r)
     id = r->req.dev->id;
     found_lun0 = false;
     n = 0;
-    QTAILQ_FOREACH(kid, &r->req.bus->qbus.children, sibling) {
+
+    RCU_READ_LOCK_GUARD();
+
+    QTAILQ_FOREACH_RCU(kid, &r->req.bus->qbus.children, sibling) {
         DeviceState *qdev = kid->child;
         SCSIDevice *dev = SCSI_DEVICE(qdev);
 
@@ -421,7 +424,7 @@ static bool scsi_target_emulate_report_luns(SCSITargetReq *r)
     memset(r->buf, 0, len);
     stl_be_p(&r->buf[0], n);
     i = found_lun0 ? 8 : 16;
-    QTAILQ_FOREACH(kid, &r->req.bus->qbus.children, sibling) {
+    QTAILQ_FOREACH_RCU(kid, &r->req.bus->qbus.children, sibling) {
         DeviceState *qdev = kid->child;
         SCSIDevice *dev = SCSI_DEVICE(qdev);
 
@@ -430,6 +433,7 @@ static bool scsi_target_emulate_report_luns(SCSITargetReq *r)
             i += 8;
         }
     }
+
     assert(i == n + 8);
     r->len = len;
     return true;
@@ -1572,7 +1576,8 @@ SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int id, int lun)
     BusChild *kid;
     SCSIDevice *target_dev = NULL;
 
-    QTAILQ_FOREACH(kid, &bus->qbus.children, sibling) {
+    RCU_READ_LOCK_GUARD();
+    QTAILQ_FOREACH_RCU(kid, &bus->qbus.children, sibling) {
         DeviceState *qdev = kid->child;
         SCSIDevice *dev = SCSI_DEVICE(qdev);
 
@@ -1591,6 +1596,7 @@ SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int id, int lun)
             }
         }
     }
+
     return target_dev;
 }
 
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 3a71ea7097..971afbb217 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -367,12 +367,16 @@ static int virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
     case VIRTIO_SCSI_T_TMF_I_T_NEXUS_RESET:
         target = req->req.tmf.lun[1];
         s->resetting++;
-        QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) {
+
+        rcu_read_lock();
+        QTAILQ_FOREACH_RCU(kid, &s->bus.qbus.children, sibling) {
              d = SCSI_DEVICE(kid->child);
              if (d->channel == 0 && d->id == target) {
                 qdev_reset_all(&d->qdev);
              }
         }
+        rcu_read_unlock();
+
         s->resetting--;
         break;
 
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 14d476c587..2c6307e3ed 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -3,6 +3,8 @@
 
 #include "qemu/queue.h"
 #include "qemu/bitmap.h"
+#include "qemu/rcu.h"
+#include "qemu/rcu_queue.h"
 #include "qom/object.h"
 #include "hw/hotplug.h"
 #include "hw/resettable.h"
@@ -238,6 +240,7 @@ struct BusClass {
 };
 
 typedef struct BusChild {
+    struct rcu_head rcu;
     DeviceState *child;
     int index;
     QTAILQ_ENTRY(BusChild) sibling;
@@ -258,6 +261,12 @@ struct BusState {
     int max_index;
     bool realized;
     int num_children;
+
+    /*
+     * children is a RCU QTAILQ, thus readers must use RCU to access it,
+     * and writers must hold the big qemu lock
+     */
+
     QTAILQ_HEAD(, BusChild) children;
     QLIST_ENTRY(BusState) sibling;
     ResettableState reset;
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 33/39] scsi: switch to bus->check_address
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (31 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 32/39] device-core: use RCU for list of children of a bus Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 34/39] device-core: use atomic_set on .realized property Paolo Bonzini
                   ` (7 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20201006123904.610658-6-mlevitsk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/scsi-bus.c | 122 ++++++++++++++++++++++++++++-----------------
 1 file changed, 75 insertions(+), 47 deletions(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 4cf1f404b4..4ab9811cd8 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -22,33 +22,6 @@ static void scsi_req_dequeue(SCSIRequest *req);
 static uint8_t *scsi_target_alloc_buf(SCSIRequest *req, size_t len);
 static void scsi_target_free_buf(SCSIRequest *req);
 
-static Property scsi_props[] = {
-    DEFINE_PROP_UINT32("channel", SCSIDevice, channel, 0),
-    DEFINE_PROP_UINT32("scsi-id", SCSIDevice, id, -1),
-    DEFINE_PROP_UINT32("lun", SCSIDevice, lun, -1),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
-static void scsi_bus_class_init(ObjectClass *klass, void *data)
-{
-    BusClass *k = BUS_CLASS(klass);
-    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
-
-    k->get_dev_path = scsibus_get_dev_path;
-    k->get_fw_dev_path = scsibus_get_fw_dev_path;
-    hc->unplug = qdev_simple_device_unplug_cb;
-}
-
-static const TypeInfo scsi_bus_info = {
-    .name = TYPE_SCSI_BUS,
-    .parent = TYPE_BUS,
-    .instance_size = sizeof(SCSIBus),
-    .class_init = scsi_bus_class_init,
-    .interfaces = (InterfaceInfo[]) {
-        { TYPE_HOTPLUG_HANDLER },
-        { }
-    }
-};
 static int next_scsi_bus;
 
 static void scsi_device_realize(SCSIDevice *s, Error **errp)
@@ -160,35 +133,68 @@ static void scsi_dma_restart_cb(void *opaque, int running, RunState state)
     }
 }
 
-static void scsi_qdev_realize(DeviceState *qdev, Error **errp)
+static bool scsi_bus_is_address_free(SCSIBus *bus,
+				     int channel, int target, int lun,
+				     SCSIDevice **p_dev)
+{
+    SCSIDevice *d = scsi_device_find(bus, channel, target, lun);
+    if (d && d->lun == lun) {
+        if (p_dev) {
+            *p_dev = d;
+        }
+        return false;
+    }
+    if (p_dev) {
+        *p_dev = NULL;
+    }
+    return true;
+}
+
+static bool scsi_bus_check_address(BusState *qbus, DeviceState *qdev, Error **errp)
 {
     SCSIDevice *dev = SCSI_DEVICE(qdev);
-    SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, dev->qdev.parent_bus);
-    SCSIDevice *d;
-    Error *local_err = NULL;
+    SCSIBus *bus = SCSI_BUS(qbus);
 
     if (dev->channel > bus->info->max_channel) {
         error_setg(errp, "bad scsi channel id: %d", dev->channel);
-        return;
+        return false;
     }
     if (dev->id != -1 && dev->id > bus->info->max_target) {
         error_setg(errp, "bad scsi device id: %d", dev->id);
-        return;
+        return false;
     }
     if (dev->lun != -1 && dev->lun > bus->info->max_lun) {
         error_setg(errp, "bad scsi device lun: %d", dev->lun);
-        return;
+        return false;
+    }
+
+    if (dev->id != -1 && dev->lun != -1) {
+        SCSIDevice *d;
+        if (!scsi_bus_is_address_free(bus, dev->channel, dev->id, dev->lun, &d)) {
+            error_setg(errp, "lun already used by '%s'", d->qdev.id);
+            return false;
+        }
     }
 
+    return true;
+}
+
+static void scsi_qdev_realize(DeviceState *qdev, Error **errp)
+{
+    SCSIDevice *dev = SCSI_DEVICE(qdev);
+    SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, dev->qdev.parent_bus);
+    bool is_free;
+    Error *local_err = NULL;
+
     if (dev->id == -1) {
         int id = -1;
         if (dev->lun == -1) {
             dev->lun = 0;
         }
         do {
-            d = scsi_device_find(bus, dev->channel, ++id, dev->lun);
-        } while (d && d->lun == dev->lun && id < bus->info->max_target);
-        if (d && d->lun == dev->lun) {
+            is_free = scsi_bus_is_address_free(bus, dev->channel, ++id, dev->lun, NULL);
+        } while (!is_free && id < bus->info->max_target);
+        if (!is_free) {
             error_setg(errp, "no free target");
             return;
         }
@@ -196,20 +202,13 @@ static void scsi_qdev_realize(DeviceState *qdev, Error **errp)
     } else if (dev->lun == -1) {
         int lun = -1;
         do {
-            d = scsi_device_find(bus, dev->channel, dev->id, ++lun);
-        } while (d && d->lun == lun && lun < bus->info->max_lun);
-        if (d && d->lun == lun) {
+            is_free = scsi_bus_is_address_free(bus, dev->channel, dev->id, ++lun, NULL);
+        } while (!is_free && lun < bus->info->max_lun);
+        if (!is_free) {
             error_setg(errp, "no free lun");
             return;
         }
         dev->lun = lun;
-    } else {
-        d = scsi_device_find(bus, dev->channel, dev->id, dev->lun);
-        assert(d);
-        if (d->lun == dev->lun && dev != d) {
-            error_setg(errp, "lun already used by '%s'", d->qdev.id);
-            return;
-        }
     }
 
     QTAILQ_INIT(&dev->requests);
@@ -1723,6 +1722,13 @@ const VMStateDescription vmstate_scsi_device = {
     }
 };
 
+static Property scsi_props[] = {
+    DEFINE_PROP_UINT32("channel", SCSIDevice, channel, 0),
+    DEFINE_PROP_UINT32("scsi-id", SCSIDevice, id, -1),
+    DEFINE_PROP_UINT32("lun", SCSIDevice, lun, -1),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void scsi_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
@@ -1753,6 +1759,28 @@ static const TypeInfo scsi_device_type_info = {
     .instance_init = scsi_dev_instance_init,
 };
 
+static void scsi_bus_class_init(ObjectClass *klass, void *data)
+{
+    BusClass *k = BUS_CLASS(klass);
+    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
+
+    k->get_dev_path = scsibus_get_dev_path;
+    k->get_fw_dev_path = scsibus_get_fw_dev_path;
+    k->check_address = scsi_bus_check_address;
+    hc->unplug = qdev_simple_device_unplug_cb;
+}
+
+static const TypeInfo scsi_bus_info = {
+    .name = TYPE_SCSI_BUS,
+    .parent = TYPE_BUS,
+    .instance_size = sizeof(SCSIBus),
+    .class_init = scsi_bus_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_HOTPLUG_HANDLER },
+        { }
+    }
+};
+
 static void scsi_register_types(void)
 {
     type_register_static(&scsi_bus_info);
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 34/39] device-core: use atomic_set on .realized property
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (32 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 33/39] scsi: switch to bus->check_address Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 35/39] scsi/scsi-bus: scsi_device_find: don't return unrealized devices Paolo Bonzini
                   ` (6 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Stefan Hajnoczi, Maxim Levitsky

From: Maxim Levitsky <mlevitsk@redhat.com>

Some code might race with placement of new devices on a bus.
We currently first place a (unrealized) device on the bus
and then realize it.

As a workaround, users that scan the child device list, can
check the realized property to see if it is safe to access such a device.
Use an atomic write here too to aid with this.

A separate discussion is what to do with devices that are unrealized:
It looks like for this case we only call the hotplug handler's unplug
callback and its up to it to unrealize the device.
An atomic operation doesn't cause harm for this code path though.

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20200913160259.32145-6-mlevitsk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20201006123904.610658-10-mlevitsk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/qdev.c         | 19 ++++++++++++++++++-
 include/hw/qdev-core.h |  2 ++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 59e5e710b7..fc4daa36fa 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -946,7 +946,25 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
             }
        }
 
+       qatomic_store_release(&dev->realized, value);
+
     } else if (!value && dev->realized) {
+
+        /*
+         * Change the value so that any concurrent users are aware
+         * that the device is going to be unrealized
+         *
+         * TODO: change .realized property to enum that states
+         * each phase of the device realization/unrealization
+         */
+
+        qatomic_set(&dev->realized, value);
+        /*
+         * Ensure that concurrent users see this update prior to
+         * any other changes done by unrealize.
+         */
+        smp_wmb();
+
         QLIST_FOREACH(bus, &dev->child_bus, sibling) {
             qbus_unrealize(bus);
         }
@@ -961,7 +979,6 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
     }
 
     assert(local_err == NULL);
-    dev->realized = value;
     return;
 
 child_realize_fail:
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 2c6307e3ed..868973319e 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -163,6 +163,8 @@ struct NamedClockList {
 /**
  * DeviceState:
  * @realized: Indicates whether the device has been fully constructed.
+ *            When accessed outsize big qemu lock, must be accessed with
+ *            atomic_load_acquire()
  * @reset: ResettableState for the device; handled by Resettable interface.
  *
  * This structure should not be accessed directly.  We declare it here
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 35/39] scsi/scsi-bus: scsi_device_find: don't return unrealized devices
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (33 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 34/39] device-core: use atomic_set on .realized property Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 36/39] scsi/scsi_bus: Add scsi_device_get Paolo Bonzini
                   ` (5 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Stefan Hajnoczi, Maxim Levitsky

The device core first places a device on the bus and then realizes it.
Make scsi_device_find avoid returing such devices to avoid
races in drivers that use an iothread (currently virtio-scsi)

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1812399

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20200913160259.32145-7-mlevitsk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20201006123904.610658-11-mlevitsk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/scsi-bus.c | 83 +++++++++++++++++++++++++++++-----------------
 1 file changed, 53 insertions(+), 30 deletions(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 4ab9811cd8..7599113efe 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -24,6 +24,55 @@ static void scsi_target_free_buf(SCSIRequest *req);
 
 static int next_scsi_bus;
 
+static SCSIDevice *do_scsi_device_find(SCSIBus *bus,
+                                       int channel, int id, int lun,
+                                       bool include_unrealized)
+{
+    BusChild *kid;
+    SCSIDevice *retval = NULL;
+
+    QTAILQ_FOREACH_RCU(kid, &bus->qbus.children, sibling) {
+        DeviceState *qdev = kid->child;
+        SCSIDevice *dev = SCSI_DEVICE(qdev);
+
+        if (dev->channel == channel && dev->id == id) {
+            if (dev->lun == lun) {
+                retval = dev;
+                break;
+            }
+
+            /*
+             * If we don't find exact match (channel/bus/lun),
+             * we will return the first device which matches channel/bus
+             */
+
+            if (!retval) {
+                retval = dev;
+            }
+        }
+    }
+
+    /*
+     * This function might run on the IO thread and we might race against
+     * main thread hot-plugging the device.
+     * We assume that as soon as .realized is set to true we can let
+     * the user access the device.
+     */
+
+    if (retval && !include_unrealized &&
+        !qatomic_load_acquire(&retval->qdev.realized)) {
+        retval = NULL;
+    }
+
+    return retval;
+}
+
+SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int id, int lun)
+{
+    RCU_READ_LOCK_GUARD();
+    return do_scsi_device_find(bus, channel, id, lun, false);
+}
+
 static void scsi_device_realize(SCSIDevice *s, Error **errp)
 {
     SCSIDeviceClass *sc = SCSI_DEVICE_GET_CLASS(s);
@@ -137,7 +186,10 @@ static bool scsi_bus_is_address_free(SCSIBus *bus,
 				     int channel, int target, int lun,
 				     SCSIDevice **p_dev)
 {
-    SCSIDevice *d = scsi_device_find(bus, channel, target, lun);
+    SCSIDevice *d;
+
+    RCU_READ_LOCK_GUARD();
+    d = do_scsi_device_find(bus, channel, target, lun, true);
     if (d && d->lun == lun) {
         if (p_dev) {
             *p_dev = d;
@@ -1570,35 +1622,6 @@ static char *scsibus_get_fw_dev_path(DeviceState *dev)
                            qdev_fw_name(dev), d->id, d->lun);
 }
 
-SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int id, int lun)
-{
-    BusChild *kid;
-    SCSIDevice *target_dev = NULL;
-
-    RCU_READ_LOCK_GUARD();
-    QTAILQ_FOREACH_RCU(kid, &bus->qbus.children, sibling) {
-        DeviceState *qdev = kid->child;
-        SCSIDevice *dev = SCSI_DEVICE(qdev);
-
-        if (dev->channel == channel && dev->id == id) {
-            if (dev->lun == lun) {
-                return dev;
-            }
-
-            /*
-             * If we don't find exact match (channel/bus/lun),
-             * we will return the first device which matches channel/bus
-             */
-
-            if (!target_dev) {
-                target_dev = dev;
-            }
-        }
-    }
-
-    return target_dev;
-}
-
 /* SCSI request list.  For simplicity, pv points to the whole device */
 
 static int put_scsi_requests(QEMUFile *f, void *pv, size_t size,
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 36/39] scsi/scsi_bus: Add scsi_device_get
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (34 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 35/39] scsi/scsi-bus: scsi_device_find: don't return unrealized devices Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 37/39] virtio-scsi: use scsi_device_get Paolo Bonzini
                   ` (4 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Stefan Hajnoczi, Maxim Levitsky

From: Maxim Levitsky <mlevitsk@redhat.com>

Add scsi_device_get which finds the scsi device
and takes a reference to it.

Suggested-by: Stefan Hajnoczi <stefanha@gmail.com>
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Message-Id: <20200913160259.32145-8-mlevitsk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20201006123904.610658-12-mlevitsk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/scsi-bus.c     | 11 +++++++++++
 include/hw/scsi/scsi.h |  1 +
 2 files changed, 12 insertions(+)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 7599113efe..eda8cb7e70 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -73,6 +73,17 @@ SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int id, int lun)
     return do_scsi_device_find(bus, channel, id, lun, false);
 }
 
+SCSIDevice *scsi_device_get(SCSIBus *bus, int channel, int id, int lun)
+{
+    SCSIDevice *d;
+    RCU_READ_LOCK_GUARD();
+    d = do_scsi_device_find(bus, channel, id, lun, false);
+    if (d) {
+        object_ref(d);
+    }
+    return d;
+}
+
 static void scsi_device_realize(SCSIDevice *s, Error **errp)
 {
     SCSIDeviceClass *sc = SCSI_DEVICE_GET_CLASS(s);
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 7a55cdbd74..09fa5c9d2a 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -190,6 +190,7 @@ int scsi_device_get_sense(SCSIDevice *dev, uint8_t *buf, int len, bool fixed);
 int scsi_SG_IO_FROM_DEV(BlockBackend *blk, uint8_t *cmd, uint8_t cmd_size,
                         uint8_t *buf, uint8_t buf_size);
 SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int target, int lun);
+SCSIDevice *scsi_device_get(SCSIBus *bus, int channel, int target, int lun);
 
 /* scsi-generic.c. */
 extern const SCSIReqOps scsi_generic_req_ops;
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 37/39] virtio-scsi: use scsi_device_get
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (35 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 36/39] scsi/scsi_bus: Add scsi_device_get Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 38/39] scsi/scsi_bus: fix races in REPORT LUNS Paolo Bonzini
                   ` (3 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Stefan Hajnoczi, Stefan Hajnoczi, Maxim Levitsky

From: Maxim Levitsky <mlevitsk@redhat.com>

This will help us to avoid the scsi device disappearing
after we took a reference to it.

It doesn't by itself forbid case when we try to access
an unrealized device

Suggested-by: Stefan Hajnoczi <stefanha@gmail.com>
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20200913160259.32145-9-mlevitsk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20201006123904.610658-13-mlevitsk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/virtio-scsi.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 971afbb217..3db9a8aae9 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -33,7 +33,7 @@ static inline int virtio_scsi_get_lun(uint8_t *lun)
     return ((lun[2] << 8) | lun[3]) & 0x3FFF;
 }
 
-static inline SCSIDevice *virtio_scsi_device_find(VirtIOSCSI *s, uint8_t *lun)
+static inline SCSIDevice *virtio_scsi_device_get(VirtIOSCSI *s, uint8_t *lun)
 {
     if (lun[0] != 1) {
         return NULL;
@@ -41,7 +41,7 @@ static inline SCSIDevice *virtio_scsi_device_find(VirtIOSCSI *s, uint8_t *lun)
     if (lun[2] != 0 && !(lun[2] >= 0x40 && lun[2] < 0x80)) {
         return NULL;
     }
-    return scsi_device_find(&s->bus, 0, lun[1], virtio_scsi_get_lun(lun));
+    return scsi_device_get(&s->bus, 0, lun[1], virtio_scsi_get_lun(lun));
 }
 
 void virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq, VirtIOSCSIReq *req)
@@ -256,7 +256,7 @@ static inline void virtio_scsi_ctx_check(VirtIOSCSI *s, SCSIDevice *d)
  *  case of async cancellation. */
 static int virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
 {
-    SCSIDevice *d = virtio_scsi_device_find(s, req->req.tmf.lun);
+    SCSIDevice *d = virtio_scsi_device_get(s, req->req.tmf.lun);
     SCSIRequest *r, *next;
     BusChild *kid;
     int target;
@@ -370,10 +370,10 @@ static int virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
 
         rcu_read_lock();
         QTAILQ_FOREACH_RCU(kid, &s->bus.qbus.children, sibling) {
-             d = SCSI_DEVICE(kid->child);
-             if (d->channel == 0 && d->id == target) {
-                qdev_reset_all(&d->qdev);
-             }
+            SCSIDevice *d1 = SCSI_DEVICE(kid->child);
+            if (d1->channel == 0 && d1->id == target) {
+                qdev_reset_all(&d1->qdev);
+            }
         }
         rcu_read_unlock();
 
@@ -386,14 +386,17 @@ static int virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
         break;
     }
 
+    object_unref(OBJECT(d));
     return ret;
 
 incorrect_lun:
     req->resp.tmf.response = VIRTIO_SCSI_S_INCORRECT_LUN;
+    object_unref(OBJECT(d));
     return ret;
 
 fail:
     req->resp.tmf.response = VIRTIO_SCSI_S_BAD_TARGET;
+    object_unref(OBJECT(d));
     return ret;
 }
 
@@ -564,7 +567,7 @@ static int virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI *s, VirtIOSCSIReq *req)
         }
     }
 
-    d = virtio_scsi_device_find(s, req->req.cmd.lun);
+    d = virtio_scsi_device_get(s, req->req.cmd.lun);
     if (!d) {
         req->resp.cmd.response = VIRTIO_SCSI_S_BAD_TARGET;
         virtio_scsi_complete_cmd_req(req);
@@ -580,10 +583,12 @@ static int virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI *s, VirtIOSCSIReq *req)
             req->sreq->cmd.xfer > req->qsgl.size)) {
         req->resp.cmd.response = VIRTIO_SCSI_S_OVERRUN;
         virtio_scsi_complete_cmd_req(req);
+        object_unref(OBJECT(d));
         return -ENOBUFS;
     }
     scsi_req_ref(req->sreq);
     blk_io_plug(d->conf.blk);
+    object_unref(OBJECT(d));
     return 0;
 }
 
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 38/39] scsi/scsi_bus: fix races in REPORT LUNS
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (36 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 37/39] virtio-scsi: use scsi_device_get Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  7:57 ` [PULL 39/39] meson: identify more sections of meson.build Paolo Bonzini
                   ` (2 subsequent siblings)
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Stefan Hajnoczi, Maxim Levitsky

From: Maxim Levitsky <mlevitsk@redhat.com>

Currently scsi_target_emulate_report_luns iterates over the child device list
twice, and there is no guarantee that this list is the same in both iterations.

The reason for iterating twice is that the first iteration calculates
how much memory to allocate.  However if we use a dynamic array we can
avoid iterating twice, and therefore we avoid this race.

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1866707

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20200913160259.32145-10-mlevitsk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20201006123904.610658-14-mlevitsk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/scsi-bus.c | 68 ++++++++++++++++++++++------------------------
 1 file changed, 33 insertions(+), 35 deletions(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index eda8cb7e70..b901e701f0 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -438,19 +438,23 @@ struct SCSITargetReq {
 static void store_lun(uint8_t *outbuf, int lun)
 {
     if (lun < 256) {
+        /* Simple logical unit addressing method*/
+        outbuf[0] = 0;
         outbuf[1] = lun;
-        return;
+    } else {
+        /* Flat space addressing method */
+        outbuf[0] = 0x40 | (lun >> 8);
+        outbuf[1] = (lun & 255);
     }
-    outbuf[1] = (lun & 255);
-    outbuf[0] = (lun >> 8) | 0x40;
 }
 
 static bool scsi_target_emulate_report_luns(SCSITargetReq *r)
 {
     BusChild *kid;
-    int i, len, n;
     int channel, id;
-    bool found_lun0;
+    uint8_t tmp[8] = {0};
+    int len = 0;
+    GByteArray *buf;
 
     if (r->req.cmd.xfer < 16) {
         return false;
@@ -458,46 +462,40 @@ static bool scsi_target_emulate_report_luns(SCSITargetReq *r)
     if (r->req.cmd.buf[2] > 2) {
         return false;
     }
+
+    /* reserve space for 63 LUNs*/
+    buf = g_byte_array_sized_new(512);
+
     channel = r->req.dev->channel;
     id = r->req.dev->id;
-    found_lun0 = false;
-    n = 0;
 
-    RCU_READ_LOCK_GUARD();
+    /* add size (will be updated later to correct value */
+    g_byte_array_append(buf, tmp, 8);
+    len += 8;
 
-    QTAILQ_FOREACH_RCU(kid, &r->req.bus->qbus.children, sibling) {
-        DeviceState *qdev = kid->child;
-        SCSIDevice *dev = SCSI_DEVICE(qdev);
+    /* add LUN0 */
+    g_byte_array_append(buf, tmp, 8);
+    len += 8;
 
-        if (dev->channel == channel && dev->id == id) {
-            if (dev->lun == 0) {
-                found_lun0 = true;
+    WITH_RCU_READ_LOCK_GUARD() {
+        QTAILQ_FOREACH_RCU(kid, &r->req.bus->qbus.children, sibling) {
+            DeviceState *qdev = kid->child;
+            SCSIDevice *dev = SCSI_DEVICE(qdev);
+
+            if (dev->channel == channel && dev->id == id && dev->lun != 0) {
+                store_lun(tmp, dev->lun);
+                g_byte_array_append(buf, tmp, 8);
+                len += 8;
             }
-            n += 8;
         }
     }
-    if (!found_lun0) {
-        n += 8;
-    }
-
-    scsi_target_alloc_buf(&r->req, n + 8);
-
-    len = MIN(n + 8, r->req.cmd.xfer & ~7);
-    memset(r->buf, 0, len);
-    stl_be_p(&r->buf[0], n);
-    i = found_lun0 ? 8 : 16;
-    QTAILQ_FOREACH_RCU(kid, &r->req.bus->qbus.children, sibling) {
-        DeviceState *qdev = kid->child;
-        SCSIDevice *dev = SCSI_DEVICE(qdev);
 
-        if (dev->channel == channel && dev->id == id) {
-            store_lun(&r->buf[i], dev->lun);
-            i += 8;
-        }
-    }
+    r->buf_len = len;
+    r->buf = g_byte_array_free(buf, FALSE);
+    r->len = MIN(len, r->req.cmd.xfer & ~7);
 
-    assert(i == n + 8);
-    r->len = len;
+    /* store the LUN list length */
+    stl_be_p(&r->buf[0], len - 8);
     return true;
 }
 
-- 
2.26.2




^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PULL 39/39] meson: identify more sections of meson.build
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (37 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 38/39] scsi/scsi_bus: fix races in REPORT LUNS Paolo Bonzini
@ 2020-10-10  7:57 ` Paolo Bonzini
  2020-10-10  8:37 ` [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 no-reply
  2020-10-12 10:29 ` Peter Maydell
  40 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-10  7:57 UTC (permalink / raw)
  To: qemu-devel

Add more headers that clarify code organization.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build | 36 +++++++++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/meson.build b/meson.build
index 0c676af194..652c37cceb 100644
--- a/meson.build
+++ b/meson.build
@@ -614,7 +614,9 @@ if not has_malloc_trim and get_option('malloc_trim').enabled()
   endif
 endif
 
-# Create config-host.h
+#################
+# config-host.h #
+#################
 
 config_host_data.set('CONFIG_COCOA', cocoa.found())
 config_host_data.set('CONFIG_LIBUDEV', libudev.found())
@@ -660,6 +662,10 @@ foreach k, v: config_host
   endif
 endforeach
 
+########################
+# Target configuration #
+########################
+
 minikconf = find_program('scripts/minikconf.py')
 config_all = {}
 config_all_devices = {}
@@ -866,7 +872,9 @@ config_all += {
   'CONFIG_ALL': true,
 }
 
-# Submodules
+##############
+# Submodules #
+##############
 
 capstone = not_found
 capstone_opt = get_option('capstone')
@@ -1105,9 +1113,11 @@ config_host_data.set('CONFIG_CAPSTONE', capstone.found())
 config_host_data.set('CONFIG_FDT', fdt.found())
 config_host_data.set('CONFIG_SLIRP', slirp.found())
 
-genh += configure_file(output: 'config-host.h', configuration: config_host_data)
+#####################
+# Generated sources #
+#####################
 
-# Generators
+genh += configure_file(output: 'config-host.h', configuration: config_host_data)
 
 hxtool = find_program('scripts/hxtool')
 shaderinclude = find_program('scripts/shaderinclude.pl')
@@ -1182,7 +1192,9 @@ sphinx_extn_depends = [ meson.source_root() / 'docs/sphinx/depfile.py',
                         meson.source_root() / 'docs/sphinx/qmp_lexer.py',
                         qapi_gen_depends ]
 
-# Collect sourcesets.
+###################
+# Collect sources #
+###################
 
 authz_ss = ss.source_set()
 blockdev_ss = ss.source_set()
@@ -1320,8 +1332,6 @@ if enable_modules
   modulecommon = declare_dependency(link_whole: libmodulecommon, compile_args: '-DBUILD_DSO')
 endif
 
-# Build targets from sourcesets
-
 stub_ss = stub_ss.apply(config_all, strict: false)
 
 util_ss.add_all(trace_ss)
@@ -1409,6 +1419,10 @@ specific_ss.add_all(when: 'CONFIG_LINUX_USER', if_true: linux_user_ss)
 subdir('tests/qtest/libqos')
 subdir('tests/qtest/fuzz')
 
+########################
+# Library dependencies #
+########################
+
 block_mods = []
 softmmu_mods = []
 foreach d, list : modules
@@ -1443,10 +1457,6 @@ qemu_syms = custom_target('qemu.syms', output: 'qemu.syms',
                              capture: true,
                              command: [undefsym, nm, '@INPUT@'])
 
-########################
-# Library dependencies #
-########################
-
 qom_ss = qom_ss.apply(config_host, strict: false)
 libqom = static_library('qom', qom_ss.sources() + genh,
                         dependencies: [qom_ss.dependencies()],
@@ -1797,6 +1807,10 @@ if host_machine.system() == 'windows'
   alias_target('installer', nsis)
 endif
 
+#########################
+# Configuration summary #
+#########################
+
 summary_info = {}
 summary_info += {'Install prefix':    config_host['prefix']}
 summary_info += {'BIOS directory':    config_host['qemu_datadir']}
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* Re: [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (38 preceding siblings ...)
  2020-10-10  7:57 ` [PULL 39/39] meson: identify more sections of meson.build Paolo Bonzini
@ 2020-10-10  8:37 ` no-reply
  2020-10-12 10:29 ` Peter Maydell
  40 siblings, 0 replies; 43+ messages in thread
From: no-reply @ 2020-10-10  8:37 UTC (permalink / raw)
  To: pbonzini; +Cc: qemu-devel

Patchew URL: https://patchew.org/QEMU/20201010075739.951385-1-pbonzini@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20201010075739.951385-1-pbonzini@redhat.com
Subject: [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/1602059975-10115-1-git-send-email-chenhc@lemote.com -> patchew/1602059975-10115-1-git-send-email-chenhc@lemote.com
 * [new tag]         patchew/20201010075739.951385-1-pbonzini@redhat.com -> patchew/20201010075739.951385-1-pbonzini@redhat.com
 * [new tag]         patchew/20201010080623.768-1-jiangyifei@huawei.com -> patchew/20201010080623.768-1-jiangyifei@huawei.com
Switched to a new branch 'test'
55f98d2 meson: identify more sections of meson.build
3f959b1 scsi/scsi_bus: fix races in REPORT LUNS
41c759b virtio-scsi: use scsi_device_get
17f4dca scsi/scsi_bus: Add scsi_device_get
eafe5c7 scsi/scsi-bus: scsi_device_find: don't return unrealized devices
c8ddbc5 device-core: use atomic_set on .realized property
07df212 scsi: switch to bus->check_address
a7baa24 device-core: use RCU for list of children of a bus
bbd9350 device_core: use drain_call_rcu in in qmp_device_add
7cafd84 scsi/scsi_bus: switch search direction in scsi_device_find
9ef1542 qdev: add "check if address free" callback for buses
1d6727d qemu-iotests, qtest: rewrite test 067 as a qtest
96b23e6 qtest: check that drives are really appearing and disappearing
a6d906c qtest: switch users back to qtest_qmp_receive
fcf3581 device-plug-test: use qtest_qmp to send the device_del command
3722046 qtest: remove qtest_qmp_receive_success
17077b6 qtest: Reintroduce qtest_qmp_receive
5b850fc qtest: rename qtest_qmp_receive to qtest_qmp_receive_dict
940c32f meson.build: Re-enable KVM support for MIPS
b4cd071 build-sys: fix git version from -version
e81db1e docs/devel: update instruction on how to add new unit tests
f187a57 qtest: unify extra_qtest_srcs and extra_qtest_deps
80c356b docs/devel/qtest: Include libqtest API reference
ef6b5cb docs/devel/qtest: Include protocol spec in document
7f82ce7 docs: Move QTest documentation to its own document
d5fddc0 configure: fix performance regression due to PIC objects
70cd0b9 qom: fix objects with improper parent type
a63dc48 exec: split out non-softmmu-specific parts
6a633f6 softmmu: move more files to softmmu/
32c750b hw/nvram: Always register FW_CFG_DATA_GENERATOR_INTERFACE
85a528c qom: Move the creation of the library to the main meson.build
6d611b1 authz: Move the creation of the library to the main meson.build
d597f26 crypto: Move the creation of the library to the main meson.build
d44bc2e io: Move the creation of the library to the main meson.build
6f014b0 migration: Move the creation of the library to the main meson.build
9d197f1 chardev: Move the creation of the library to the main meson.build
8b9a449 hw/core: Move the creation of the library to the main meson.build
245f27b meson.build: Sort sourcesets alphabetically
cbc9573 meson.build: Add comments to clarify code organization

=== OUTPUT BEGIN ===
1/39 Checking commit cbc957397c02 (meson.build: Add comments to clarify code organization)
2/39 Checking commit 245f27b24fa7 (meson.build: Sort sourcesets alphabetically)
3/39 Checking commit 8b9a44915aa8 (hw/core: Move the creation of the library to the main meson.build)
4/39 Checking commit 9d197f154484 (chardev: Move the creation of the library to the main meson.build)
5/39 Checking commit 6f014b047dde (migration: Move the creation of the library to the main meson.build)
6/39 Checking commit d44bc2e97f1c (io: Move the creation of the library to the main meson.build)
7/39 Checking commit d597f26a6c94 (crypto: Move the creation of the library to the main meson.build)
8/39 Checking commit 6d611b137d38 (authz: Move the creation of the library to the main meson.build)
9/39 Checking commit 85a528c294eb (qom: Move the creation of the library to the main meson.build)
10/39 Checking commit 32c750b90a9c (hw/nvram: Always register FW_CFG_DATA_GENERATOR_INTERFACE)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#52: 
new file mode 100644

total: 0 errors, 1 warnings, 55 lines checked

Patch 10/39 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
11/39 Checking commit 6a633f68ddd7 (softmmu: move more files to softmmu/)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#85: 
rename from bootdevice.c

total: 0 errors, 1 warnings, 69 lines checked

Patch 11/39 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
12/39 Checking commit a63dc48247ea (exec: split out non-softmmu-specific parts)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#52: 
new file mode 100644

WARNING: Block comments use a leading /* on a separate line
#104: FILE: cpu.c:48:
+    /* 0x01 was CPU_INTERRUPT_EXIT. This line can be removed when the

WARNING: Block comments use * on subsequent lines
#105: FILE: cpu.c:49:
+    /* 0x01 was CPU_INTERRUPT_EXIT. This line can be removed when the
+       version_id is increased. */

WARNING: Block comments use a trailing */ on a separate line
#105: FILE: cpu.c:49:
+       version_id is increased. */

WARNING: Block comments use a leading /* on a separate line
#109: FILE: cpu.c:53:
+    /* loadvm has just updated the content of RAM, bypassing the

ERROR: spaces required around that '*' (ctx:VxV)
#175: FILE: cpu.c:119:
+    .subsections = (const VMStateDescription*[]) {
                                             ^

WARNING: Block comments use a leading /* on a separate line
#205: FILE: cpu.c:149:
+    /* Create a memory property for softmmu CPU object,

WARNING: Block comments use a leading /* on a separate line
#393: FILE: cpu.c:337:
+/* enable or disable single step mode. EXCP_DEBUG is returned by the

WARNING: Block comments use * on subsequent lines
#394: FILE: cpu.c:338:
+/* enable or disable single step mode. EXCP_DEBUG is returned by the
+   CPU loop after each instruction */

WARNING: Block comments use a trailing */ on a separate line
#394: FILE: cpu.c:338:
+   CPU loop after each instruction */

ERROR: "foo * bar" should be "foo *bar"
#452: FILE: cpu.c:396:
+    void * p;

ERROR: braces {} are necessary for all arms of this statement
#458: FILE: cpu.c:402:
+        if (l > len)
[...]

ERROR: braces {} are necessary for all arms of this statement
#461: FILE: cpu.c:405:
+        if (!(flags & PAGE_VALID))
[...]

ERROR: braces {} are necessary for all arms of this statement
#464: FILE: cpu.c:408:
+            if (!(flags & PAGE_WRITE))
[...]

ERROR: do not use assignment in if condition
#467: FILE: cpu.c:411:
+            if (!(p = lock_user(VERIFY_WRITE, addr, l, 0)))

ERROR: braces {} are necessary for all arms of this statement
#467: FILE: cpu.c:411:
+            if (!(p = lock_user(VERIFY_WRITE, addr, l, 0)))
[...]

ERROR: braces {} are necessary for all arms of this statement
#472: FILE: cpu.c:416:
+            if (!(flags & PAGE_READ))
[...]

ERROR: do not use assignment in if condition
#475: FILE: cpu.c:419:
+            if (!(p = lock_user(VERIFY_READ, addr, l, 1)))

ERROR: braces {} are necessary for all arms of this statement
#475: FILE: cpu.c:419:
+            if (!(p = lock_user(VERIFY_READ, addr, l, 1)))
[...]

WARNING: Block comments use a leading /* on a separate line
#499: FILE: cpu.c:443:
+    /* NOTE: we can always suppose that qemu_host_page_size >=

WARNING: Block comments use * on subsequent lines
#500: FILE: cpu.c:444:
+    /* NOTE: we can always suppose that qemu_host_page_size >=
+       TARGET_PAGE_SIZE */

WARNING: Block comments use a trailing */ on a separate line
#500: FILE: cpu.c:444:
+       TARGET_PAGE_SIZE */

total: 10 errors, 12 warnings, 1061 lines checked

Patch 12/39 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

13/39 Checking commit 70cd0b9ec467 (qom: fix objects with improper parent type)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#69: 
new file mode 100644

total: 0 errors, 1 warnings, 57 lines checked

Patch 13/39 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
14/39 Checking commit d5fddc0cba63 (configure: fix performance regression due to PIC objects)
15/39 Checking commit 7f82ce732ca7 (docs: Move QTest documentation to its own document)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#31: 
new file mode 100644

total: 0 errors, 1 warnings, 124 lines checked

Patch 15/39 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
16/39 Checking commit ef6b5cbd233f (docs/devel/qtest: Include protocol spec in document)
17/39 Checking commit 80c356b3fbc1 (docs/devel/qtest: Include libqtest API reference)
18/39 Checking commit f187a576c59c (qtest: unify extra_qtest_srcs and extra_qtest_deps)
19/39 Checking commit e81db1ec6da4 (docs/devel: update instruction on how to add new unit tests)
20/39 Checking commit b4cd071db805 (build-sys: fix git version from -version)
21/39 Checking commit 940c32fa7664 (meson.build: Re-enable KVM support for MIPS)
22/39 Checking commit 5b850fc17469 (qtest: rename qtest_qmp_receive to qtest_qmp_receive_dict)
23/39 Checking commit 17077b686545 (qtest: Reintroduce qtest_qmp_receive)
24/39 Checking commit 372204626c2a (qtest: remove qtest_qmp_receive_success)
25/39 Checking commit fcf3581e19b2 (device-plug-test: use qtest_qmp to send the device_del command)
WARNING: line over 80 characters
#33: FILE: tests/qtest/device-plug-test.c:23:
+                     "{'execute': 'device_del', 'arguments': { 'id': %s } }", id);

total: 0 errors, 1 warnings, 76 lines checked

Patch 25/39 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
26/39 Checking commit a6d906ce8819 (qtest: switch users back to qtest_qmp_receive)
27/39 Checking commit 96b23e6b9397 (qtest: check that drives are really appearing and disappearing)
28/39 Checking commit 1d6727dc791e (qemu-iotests, qtest: rewrite test 067 as a qtest)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#46: 
deleted file mode 100755

WARNING: line over 80 characters
#650: FILE: tests/qtest/drive_del-test.c:19:
+static bool look_for_drive0(QTestState *qts, const char *command, const char *key)

total: 0 errors, 2 warnings, 309 lines checked

Patch 28/39 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
29/39 Checking commit 9ef154286ce3 (qdev: add "check if address free" callback for buses)
30/39 Checking commit 7cafd8471e6a (scsi/scsi_bus: switch search direction in scsi_device_find)
31/39 Checking commit bbd93507e2d4 (device_core: use drain_call_rcu in in qmp_device_add)
32/39 Checking commit a7baa24616a2 (device-core: use RCU for list of children of a bus)
33/39 Checking commit 07df2121d569 (scsi: switch to bus->check_address)
ERROR: code indent should never use tabs
#55: FILE: hw/scsi/scsi-bus.c:137:
+^I^I^I^I     int channel, int target, int lun,$

ERROR: code indent should never use tabs
#56: FILE: hw/scsi/scsi-bus.c:138:
+^I^I^I^I     SCSIDevice **p_dev)$

WARNING: line over 80 characters
#71: FILE: hw/scsi/scsi-bus.c:153:
+static bool scsi_bus_check_address(BusState *qbus, DeviceState *qdev, Error **errp)

WARNING: line over 80 characters
#91: FILE: hw/scsi/scsi-bus.c:173:
+        if (!scsi_bus_is_address_free(bus, dev->channel, dev->id, dev->lun, &d)) {

WARNING: line over 80 characters
#130: FILE: hw/scsi/scsi-bus.c:195:
+            is_free = scsi_bus_is_address_free(bus, dev->channel, ++id, dev->lun, NULL);

WARNING: line over 80 characters
#143: FILE: hw/scsi/scsi-bus.c:205:
+            is_free = scsi_bus_is_address_free(bus, dev->channel, dev->id, ++lun, NULL);

total: 2 errors, 4 warnings, 182 lines checked

Patch 33/39 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

34/39 Checking commit c8ddbc5cad10 (device-core: use atomic_set on .realized property)
35/39 Checking commit eafe5c7675d0 (scsi/scsi-bus: scsi_device_find: don't return unrealized devices)
36/39 Checking commit 17f4dca5c0be (scsi/scsi_bus: Add scsi_device_get)
37/39 Checking commit 41c759b9e570 (virtio-scsi: use scsi_device_get)
38/39 Checking commit 3f959b1165c9 (scsi/scsi_bus: fix races in REPORT LUNS)
39/39 Checking commit 55f98d22596c (meson: identify more sections of meson.build)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20201010075739.951385-1-pbonzini@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10
  2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
                   ` (39 preceding siblings ...)
  2020-10-10  8:37 ` [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 no-reply
@ 2020-10-12 10:29 ` Peter Maydell
  2020-10-12 13:42   ` Paolo Bonzini
  40 siblings, 1 reply; 43+ messages in thread
From: Peter Maydell @ 2020-10-12 10:29 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: QEMU Developers

On Sat, 10 Oct 2020 at 08:59, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> The following changes since commit 4a7c0bd9dcb08798c6f82e55b5a3423f7ee669f1:
>
>   Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-5.2-20201009' into staging (2020-10-09 15:48:04 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 1340ff2adb2624e61c5fcb0eb1889b932b76f669:
>
>   meson: identify more sections of meson.build (2020-10-09 13:19:50 -0400)
>
> ----------------------------------------------------------------
> * qtest documentation improvements (Eduardo, myself)
> * libqtest event buffering (Maxim)
> * use RCU for list of children of a bus (Maxim)
> * move more files to softmmu/ (myself)
> * meson.build cleanups, qemu-storage-daemon fix (Philippe)
>

This produces a new warning, I think from Sphinx:

Running Sphinx v1.6.7
loading pickled environment... done
building [mo]: targets for 0 po files that are out of date
building [html]: targets for 2 source files that are out of date
updating environment: 1 added, 2 changed, 0 removed
reading sources... [ 33%] index
reading sources... [ 66%] qtest
reading sources... [100%] testing

looking for now-outdated files... none found
pickling environment... done
checking consistency... done
preparing documents... done
writing output... [ 33%] index
writing output... [ 66%] qtest
writing output... [100%] testing

generating indices... genindex
writing additional pages... search
copying static files... done
copying extra files... done
dumping search index in English (code: en) ... done
dumping object inventory... done
build succeeded.
/home/petmay01/linaro/qemu-for-merges/docs/../tests/qtest/libqos/libqtest.h:241:
warning: Function parameter or member 'event' not described in
'qtest_qmp_event_ref'
make: Leaving directory '/home/petmay01/linaro/qemu-for-merges/build/all'
make: Entering directory '/home/petmay01/linaro/qemu-for-merges/build/all'
[build continues and succeeds]

Not sure why it isn't fatal.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10
  2020-10-12 10:29 ` Peter Maydell
@ 2020-10-12 13:42   ` Paolo Bonzini
  0 siblings, 0 replies; 43+ messages in thread
From: Paolo Bonzini @ 2020-10-12 13:42 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers

On 12/10/20 12:29, Peter Maydell wrote:
> warning: Function parameter or member 'event' not described in
> 'qtest_qmp_event_ref'
> make: Leaving directory '/home/petmay01/linaro/qemu-for-merges/build/all'
> make: Entering directory '/home/petmay01/linaro/qemu-for-merges/build/all'
> [build continues and succeeds]
> 
> Not sure why it isn't fatal.

I'll fix and resend anyway.

Paolo



^ permalink raw reply	[flat|nested] 43+ messages in thread

end of thread, other threads:[~2020-10-12 13:44 UTC | newest]

Thread overview: 43+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-10  7:57 [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 Paolo Bonzini
2020-10-10  7:57 ` [PULL 01/39] meson.build: Add comments to clarify code organization Paolo Bonzini
2020-10-10  7:57 ` [PULL 02/39] meson.build: Sort sourcesets alphabetically Paolo Bonzini
2020-10-10  7:57 ` [PULL 03/39] hw/core: Move the creation of the library to the main meson.build Paolo Bonzini
2020-10-10  7:57 ` [PULL 04/39] chardev: " Paolo Bonzini
2020-10-10  7:57 ` [PULL 05/39] migration: " Paolo Bonzini
2020-10-10  7:57 ` [PULL 06/39] io: " Paolo Bonzini
2020-10-10  7:57 ` [PULL 07/39] crypto: " Paolo Bonzini
2020-10-10  7:57 ` [PULL 08/39] authz: " Paolo Bonzini
2020-10-10  7:57 ` [PULL 09/39] qom: " Paolo Bonzini
2020-10-10  7:57 ` [PULL 10/39] hw/nvram: Always register FW_CFG_DATA_GENERATOR_INTERFACE Paolo Bonzini
2020-10-10  7:57 ` [PULL 11/39] softmmu: move more files to softmmu/ Paolo Bonzini
2020-10-10  7:57 ` [PULL 12/39] exec: split out non-softmmu-specific parts Paolo Bonzini
2020-10-10  7:57 ` [PULL 13/39] qom: fix objects with improper parent type Paolo Bonzini
2020-10-10  7:57 ` [PULL 14/39] configure: fix performance regression due to PIC objects Paolo Bonzini
2020-10-10  7:57 ` [PULL 15/39] docs: Move QTest documentation to its own document Paolo Bonzini
2020-10-10  7:57 ` [PULL 16/39] docs/devel/qtest: Include protocol spec in document Paolo Bonzini
2020-10-10  7:57 ` [PULL 17/39] docs/devel/qtest: Include libqtest API reference Paolo Bonzini
2020-10-10  7:57 ` [PULL 18/39] qtest: unify extra_qtest_srcs and extra_qtest_deps Paolo Bonzini
2020-10-10  7:57 ` [PULL 19/39] docs/devel: update instruction on how to add new unit tests Paolo Bonzini
2020-10-10  7:57 ` [PULL 20/39] build-sys: fix git version from -version Paolo Bonzini
2020-10-10  7:57 ` [PULL 21/39] meson.build: Re-enable KVM support for MIPS Paolo Bonzini
2020-10-10  7:57 ` [PULL 22/39] qtest: rename qtest_qmp_receive to qtest_qmp_receive_dict Paolo Bonzini
2020-10-10  7:57 ` [PULL 23/39] qtest: Reintroduce qtest_qmp_receive Paolo Bonzini
2020-10-10  7:57 ` [PULL 24/39] qtest: remove qtest_qmp_receive_success Paolo Bonzini
2020-10-10  7:57 ` [PULL 25/39] device-plug-test: use qtest_qmp to send the device_del command Paolo Bonzini
2020-10-10  7:57 ` [PULL 26/39] qtest: switch users back to qtest_qmp_receive Paolo Bonzini
2020-10-10  7:57 ` [PULL 27/39] qtest: check that drives are really appearing and disappearing Paolo Bonzini
2020-10-10  7:57 ` [PULL 28/39] qemu-iotests, qtest: rewrite test 067 as a qtest Paolo Bonzini
2020-10-10  7:57 ` [PULL 29/39] qdev: add "check if address free" callback for buses Paolo Bonzini
2020-10-10  7:57 ` [PULL 30/39] scsi/scsi_bus: switch search direction in scsi_device_find Paolo Bonzini
2020-10-10  7:57 ` [PULL 31/39] device_core: use drain_call_rcu in in qmp_device_add Paolo Bonzini
2020-10-10  7:57 ` [PULL 32/39] device-core: use RCU for list of children of a bus Paolo Bonzini
2020-10-10  7:57 ` [PULL 33/39] scsi: switch to bus->check_address Paolo Bonzini
2020-10-10  7:57 ` [PULL 34/39] device-core: use atomic_set on .realized property Paolo Bonzini
2020-10-10  7:57 ` [PULL 35/39] scsi/scsi-bus: scsi_device_find: don't return unrealized devices Paolo Bonzini
2020-10-10  7:57 ` [PULL 36/39] scsi/scsi_bus: Add scsi_device_get Paolo Bonzini
2020-10-10  7:57 ` [PULL 37/39] virtio-scsi: use scsi_device_get Paolo Bonzini
2020-10-10  7:57 ` [PULL 38/39] scsi/scsi_bus: fix races in REPORT LUNS Paolo Bonzini
2020-10-10  7:57 ` [PULL 39/39] meson: identify more sections of meson.build Paolo Bonzini
2020-10-10  8:37 ` [PULL 00/39] SCSI, qdev, qtest, meson patches for 2020-10-10 no-reply
2020-10-12 10:29 ` Peter Maydell
2020-10-12 13:42   ` Paolo Bonzini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.