[PULL 00/20] Misc patches for QEMU 5.2-rc1

[PULL 00/20] Misc patches for QEMU 5.2-rc1

[Paolo Bonzini]

The following changes since commit 83851c7c60c90e9fb6a23ff48076387a77bc33cd:

  Merge remote-tracking branch 'remotes/mdroth/tags/qga-pull-2020-10-27-v3-tag' into staging (2020-11-03 12:47:58 +0000)

are available in the Git repository at:

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

for you to fetch changes up to 46fe962264f91946ca75d23667015d9ba081d6ef:

  qtest: escape device name in device-introspect-test (2020-11-04 07:29:56 -0500)

----------------------------------------------------------------
Doc and bug fixes

----------------------------------------------------------------
AlexChen (1):
      tests/qtest: Fix potential NULL pointer dereference in qos_build_main_args()

Alexander Bulekov (5):
      scripts/oss-fuzz: rename bin/qemu-fuzz-i386
      qtest: add a reproducer for LP#1878642
      fuzz: fix writing DMA patterns
      fuzz: check the MR in the DMA callback
      fuzz: fuzz offsets within pio/mmio regions

Bruce Rogers (1):
      configure: fix gio_libs reference

Marc Hartmayer (1):
      meson: vhost-user-gpu/virtiofsd: use absolute path

Paolo Bonzini (8):
      cutils: replace strdup with g_strdup
      docs: expand sourceset documentation
      meson: use b_staticpic=false for meson >=0.56.0
      meson: fix warning for bad sphinx-build
      semihosting: fix order of initialization functions
      qapi, qemu-options: make all parsing visitors parse boolean options the same
      ivshmem-test: do not use short-form boolean option
      qtest: escape device name in device-introspect-test

Peter Maydell (2):
      tests/qtest/libqtest.c: Check for setsockopt() failure
      tests/qtest/libqos/ahci.c: Avoid NULL dereference in ahci_exec()

Philippe Mathieu-Daudé (2):
      exec: Remove dead code (CID 1432876)
      hw/isa/lpc_ich9: Ignore reserved/invalid SCI IRQ

 configure                            |   8 +-
 contrib/vhost-user-gpu/meson.build   |   2 +-
 docs/devel/build-system.rst          | 158 +++++++++++++++++++++++------------
 docs/devel/kconfig.rst               |   2 +
 docs/meson.build                     |   3 +-
 hw/isa/lpc_ich9.c                    |  14 +++-
 include/hw/i386/ich9.h               |   1 +
 include/qapi/util.h                  |   2 +
 include/qemu/cutils.h                |   1 +
 meson.build                          |   4 +-
 qapi/opts-visitor.c                  |  14 +---
 qapi/qapi-util.c                     |  23 +++++
 qapi/qobject-input-visitor.c         |   6 +-
 qapi/string-input-visitor.c          |  17 +---
 scripts/oss-fuzz/build.sh            |   4 +-
 softmmu/physmem.c                    |  10 +--
 softmmu/vl.c                         |   4 +-
 tests/qemu-iotests/051.out           |   6 +-
 tests/qemu-iotests/051.pc.out        |   6 +-
 tests/qemu-iotests/133.out           |   8 +-
 tests/qemu-iotests/137.out           |   4 +-
 tests/qtest/device-introspect-test.c |  10 ++-
 tests/qtest/fuzz-test.c              |  15 ++++
 tests/qtest/fuzz/generic_fuzz.c      |  44 +++++-----
 tests/qtest/fuzz/qos_fuzz.c          |   3 +-
 tests/qtest/ivshmem-test.c           |   2 +-
 tests/qtest/libqos/ahci.c            |   5 +-
 tests/qtest/libqtest.c               |   9 +-
 tools/virtiofsd/meson.build          |   2 +-
 util/cutils.c                        |   2 +-
 util/qemu-option.c                   |  20 +----
 31 files changed, 239 insertions(+), 170 deletions(-)
-- 
2.26.2

[PULL 01/20] cutils: replace strdup with g_strdup

[Paolo Bonzini]

Memory returned by get_relocated_path must be freed with
free or g_free depending on the path that the function
took; Coverity takes exception to this practice.  The
fix lets caller use g_free as is standard in QEMU.

While at it, mention the requirements on the caller in
the doc comment.

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/qemu/cutils.h | 1 +
 util/cutils.c         | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
index 4bbf4834ea..986ed8e15f 100644
--- a/include/qemu/cutils.h
+++ b/include/qemu/cutils.h
@@ -205,6 +205,7 @@ int qemu_pstrcmp0(const char **str1, const char **str2);
  * as the prefix.  For example, if `bindir` is `/usr/bin` and @dir is
  * `/usr/share/qemu`, the function will append `../share/qemu` to the
  * directory that contains the running executable and return the result.
+ * The returned string should be freed by the caller.
  */
 char *get_relocated_path(const char *dir);
 
diff --git a/util/cutils.c b/util/cutils.c
index c395974fab..9498e28e1a 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -937,7 +937,7 @@ char *get_relocated_path(const char *dir)
     /* Fail if qemu_init_exec_dir was not called.  */
     assert(exec_dir[0]);
     if (!starts_with_prefix(dir) || !starts_with_prefix(bindir)) {
-        return strdup(dir);
+        return g_strdup(dir);
     }
 
     result = g_string_new(exec_dir);
-- 
2.26.2

[PULL 02/20] docs: expand sourceset documentation

[Paolo Bonzini]

Expand on the usage of sourcesets and describe the CONFIG_ALL
symbol.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 docs/devel/build-system.rst | 158 ++++++++++++++++++++++++------------
 docs/devel/kconfig.rst      |   2 +
 2 files changed, 107 insertions(+), 53 deletions(-)

diff --git a/docs/devel/build-system.rst b/docs/devel/build-system.rst
index 6fcf8854b7..31f4dced2a 100644
--- a/docs/devel/build-system.rst
+++ b/docs/devel/build-system.rst
@@ -187,21 +187,23 @@ process for:
 
 4) other data files, such as icons or desktop files
 
-The source code is highly modularized, split across many files to
-facilitate building of all of these components with as little duplicated
-compilation as possible. The Meson "sourceset" functionality is used
-to list the files and their dependency on various configuration  
-symbols.
-
 All executables are built by default, except for some `contrib/`
 binaries that are known to fail to build on some platforms (for example
 32-bit or big-endian platforms).  Tests are also built by default,
 though that might change in the future.
 
-Various subsystems that are common to both tools and emulators have
-their own sourceset, for example `block_ss` for the block device subsystem,
-`chardev_ss` for the character device subsystem, etc.  These sourcesets
-are then turned into static libraries as follows::
+The source code is highly modularized, split across many files to
+facilitate building of all of these components with as little duplicated
+compilation as possible. Using the Meson "sourceset" functionality,
+`meson.build` files group the source files in rules that are
+enabled according to the available system libraries and to various
+configuration symbols.  Sourcesets belong to one of four groups:
+
+Subsystem sourcesets:
+  Various subsystems that are common to both tools and emulators have
+  their own sourceset, for example `block_ss` for the block device subsystem,
+  `chardev_ss` for the character device subsystem, etc.  These sourcesets
+  are then turned into static libraries as follows::
 
     libchardev = static_library('chardev', chardev_ss.sources(),
                                 name_suffix: 'fa',
@@ -209,61 +211,111 @@ are then turned into static libraries as follows::
 
     chardev = declare_dependency(link_whole: libchardev)
 
-As of Meson 0.55.1, the special `.fa` suffix should be used for everything
-that is used with `link_whole`, to ensure that the link flags are placed
-correctly in the command line.
-
-Files linked into emulator targets there can be split into two distinct groups
-of files, those which are independent of the QEMU emulation target and
-those which are dependent on the QEMU emulation target.
-
-In the target-independent set lives various general purpose helper code,
-such as error handling infrastructure, standard data structures,
-platform portability wrapper functions, etc. This code can be compiled
-once only and the .o files linked into all output binaries.
-Target-independent code lives in the `common_ss`, `softmmu_ss` and
-`user_ss` sourcesets.  `common_ss` is linked into all emulators, `softmmu_ss`
-only in system emulators, `user_ss` only in user-mode emulators.
-
-In the target-dependent set lives CPU emulation, device emulation and
-much glue code. This sometimes also has to be compiled multiple times,
-once for each target being built.  Target-dependent files are included
-in the `specific_ss` sourceset.
-
-All binaries link with a static library `libqemuutil.a`, which is then
-linked to all the binaries.  `libqemuutil.a` is built from several
-sourcesets; most of them however host generated code, and the only two
-of general interest are `util_ss` and `stub_ss`.
-
-The separation between these two is purely for documentation purposes.
-`util_ss` contains generic utility files.  Even though this code is only
-linked in some binaries, sometimes it requires hooks only in some of
-these and depend on other functions that are not fully implemented by
-all QEMU binaries.  `stub_ss` links dummy stubs that will only be linked
-into the binary if the real implementation is not present.  In a way,
-the stubs can be thought of as a portable implementation of the weak
-symbols concept.
+  As of Meson 0.55.1, the special `.fa` suffix should be used for everything
+  that is used with `link_whole`, to ensure that the link flags are placed
+  correctly in the command line.
+
+Target-independent emulator sourcesets:
+  Various general purpose helper code is compiled only once and
+  the .o files are linked into all output binaries that need it.
+  This includes error handling infrastructure, standard data structures,
+  platform portability wrapper functions, etc.
+
+  Target-independent code lives in the `common_ss`, `softmmu_ss` and
+  `user_ss` sourcesets.  `common_ss` is linked into all emulators,
+  `softmmu_ss` only in system emulators, `user_ss` only in user-mode
+  emulators.
+
+  Target-independent sourcesets must exercise particular care when using
+  `if_false` rules.  The `if_false` rule will be used correctly when linking
+  emulator binaries; however, when *compiling* target-independent files
+  into .o files, Meson may need to pick *both* the `if_true` and
+  `if_false` sides to cater for targets that want either side.  To
+  achieve that, you can add a special rule using the ``CONFIG_ALL``
+  symbol::
+
+    # Some targets have CONFIG_ACPI, some don't, so this is not enough
+    softmmu_ss.add(when: 'CONFIG_ACPI`, if_true: files('acpi.c'),
+                                        if_false: files('acpi-stub.c'))
+
+    # This is required as well:
+    softmmu_ss.add(when: 'CONFIG_ALL`, if_true: files('acpi-stub.c'))
+
+Target-dependent emulator sourcesets:
+  In the target-dependent set lives CPU emulation, some device emulation and
+  much glue code. This sometimes also has to be compiled multiple times,
+  once for each target being built.  Target-dependent files are included
+  in the `specific_ss` sourceset.
+
+  Each emulator also includes sources for files in the `hw/` and `target/`
+  subdirectories.  The subdirectory used for each emulator comes
+  from the target's definition of ``TARGET_BASE_ARCH`` or (if missing)
+  ``TARGET_ARCH``, as found in `default-configs/targets/*.mak`.
+
+  Each subdirectory in `hw/` adds one sourceset to the `hw_arch` dictionary,
+  for example::
+
+    arm_ss = ss.source_set()
+    arm_ss.add(files('boot.c'), fdt)
+    ...
+    hw_arch += {'arm': arm_ss}
+
+  The sourceset is only used for system emulators.
+
+  Each subdirectory in `target/` instead should add one sourceset to each
+  of the `target_arch` and `target_softmmu_arch`, which are used respectively
+  for all emulators and for system emulators only.  For example::
+
+    arm_ss = ss.source_set()
+    arm_softmmu_ss = ss.source_set()
+    ...
+    target_arch += {'arm': arm_ss}
+    target_softmmu_arch += {'arm': arm_softmmu_ss}
+
+Utility sourcesets:
+  All binaries link with a static library `libqemuutil.a`.  This library
+  is built from several sourcesets; most of them however host generated
+  code, and the only two of general interest are `util_ss` and `stub_ss`.
+
+  The separation between these two is purely for documentation purposes.
+  `util_ss` contains generic utility files.  Even though this code is only
+  linked in some binaries, sometimes it requires hooks only in some of
+  these and depend on other functions that are not fully implemented by
+  all QEMU binaries.  `stub_ss` links dummy stubs that will only be linked
+  into the binary if the real implementation is not present.  In a way,
+  the stubs can be thought of as a portable implementation of the weak
+  symbols concept.
+
 
 The following files concur in the definition of which files are linked
 into each emulator:
 
-`default-configs/*.mak`
-  The files under default-configs/ control what emulated hardware is built
-  into each QEMU system and userspace emulator targets. They merely contain
-  a list of config variable definitions like the machines that should be
-  included. For example, default-configs/aarch64-softmmu.mak has::
+`default-configs/devices/*.mak`
+  The files under `default-configs/devices/` control the boards and devices
+  that are built into each QEMU system emulation targets. They merely contain
+  a list of config variable definitions such as::
 
     include arm-softmmu.mak
     CONFIG_XLNX_ZYNQMP_ARM=y
     CONFIG_XLNX_VERSAL=y
 
 `*/Kconfig`
-  These files are processed together with `default-configs/*.mak` and
+  These files are processed together with `default-configs/devices/*.mak` and
   describe the dependencies between various features, subsystems and
-  device models.  They are described in kconfig.rst.
+  device models.  They are described in :ref:`kconfig`
+
+`default-configs/targets/*.mak`
+  These files mostly define symbols that appear in the `*-config-target.h`
+  file for each emulator [#cfgtarget]_.  However, the ``TARGET_ARCH``
+  and ``TARGET_BASE_ARCH`` will also be used to select the `hw/` and
+  `target/` subdirectories that are compiled into each target.
+
+.. [#cfgtarget] This header is included by `qemu/osdep.h` when
+                compiling files from the target-specific sourcesets.
 
-These files rarely need changing unless new devices / hardware need to
-be enabled for a particular system/userspace emulation target
+These files rarely need changing unless you are adding a completely
+new target, or enabling new devices or hardware for a particular
+system/userspace emulation target
 
 
 Support scripts
diff --git a/docs/devel/kconfig.rst b/docs/devel/kconfig.rst
index e5df72b342..336ba0e8e5 100644
--- a/docs/devel/kconfig.rst
+++ b/docs/devel/kconfig.rst
@@ -1,3 +1,5 @@
+.. _kconfig:
+
 ================
 QEMU and Kconfig
 ================
-- 
2.26.2

[PULL 03/20] exec: Remove dead code (CID 1432876)

[Paolo Bonzini]

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

We removed the global_locking field in commit 4174495408a,
leaving dead code around the 'unlocked' variable. Remove it
to fix the DEADCODE issue reported by Coverity (CID 1432876).

Fixes: 4174495408a ("exec: Remove MemoryRegion::global_locking field")
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20201030153752.1557776-1-philmd@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 softmmu/physmem.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index a9adedb9f8..0b31be2928 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -2723,22 +2723,14 @@ static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr)
 
 static bool prepare_mmio_access(MemoryRegion *mr)
 {
-    bool unlocked = !qemu_mutex_iothread_locked();
     bool release_lock = false;
 
-    if (unlocked) {
+    if (!qemu_mutex_iothread_locked()) {
         qemu_mutex_lock_iothread();
-        unlocked = false;
         release_lock = true;
     }
     if (mr->flush_coalesced_mmio) {
-        if (unlocked) {
-            qemu_mutex_lock_iothread();
-        }
         qemu_flush_coalesced_mmio_buffer();
-        if (unlocked) {
-            qemu_mutex_unlock_iothread();
-        }
     }
 
     return release_lock;
-- 
2.26.2

[PULL 04/20] scripts/oss-fuzz: rename bin/qemu-fuzz-i386

[Paolo Bonzini]

From: Alexander Bulekov <alxndr@bu.edu>

OSS-Fuzz changed the way it scans for fuzzers in $DEST_DIR. The new code
also scans subdirectories for fuzzers. This means that OSS-Fuzz is
considering bin/qemu-fuzz-i386 as an independent fuzzer (it is not - it
requires a --fuzz-target argument). This has led to coverage-build
failures and false crash reports. To work around this, we take advantage
of OSS-Fuzz' filename extension check - OSS-Fuzz will not run anything
that has an extension that is not ".exe":
https://github.com/google/oss-fuzz/blob/master/infra/utils.py#L115

Reported-by: OSS-Fuzz (Issue 26725)
Reported-by: OSS-Fuzz (Issue 26679)
Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Message-Id: <20201101212245.185819-1-alxndr@bu.edu>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/oss-fuzz/build.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh
index fcae4a0c26..3b1c82b63d 100755
--- a/scripts/oss-fuzz/build.sh
+++ b/scripts/oss-fuzz/build.sh
@@ -91,7 +91,7 @@ make "-j$(nproc)" qemu-fuzz-i386 V=1
 # Copy over the datadir
 cp  -r ../pc-bios/ "$DEST_DIR/pc-bios"
 
-cp "./qemu-fuzz-i386" "$DEST_DIR/bin/"
+cp "./qemu-fuzz-i386" "$DEST_DIR/bin/qemu-fuzz-i386.base"
 
 # Run the fuzzer with no arguments, to print the help-string and get the list
 # of available fuzz-targets. Copy over the qemu-fuzz-i386, naming it according
@@ -104,7 +104,7 @@ do
     # that are thin wrappers around this target that set the required
     # environment variables according to predefined configs.
     if [ "$target" != "generic-fuzz" ]; then
-        ln  "$DEST_DIR/bin/qemu-fuzz-i386" \
+        ln  "$DEST_DIR/bin/qemu-fuzz-i386.base" \
             "$DEST_DIR/qemu-fuzz-i386-target-$target"
     fi
 done
-- 
2.26.2

[PULL 05/20] hw/isa/lpc_ich9: Ignore reserved/invalid SCI IRQ

[Paolo Bonzini]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

libFuzzer triggered the following assertion:

  cat << EOF | qemu-system-i386 -M pc-q35-5.0 \
    -nographic -monitor none -serial none \
    -qtest stdio -d guest_errors -trace pci\*
  outl 0xcf8 0x8400f841
  outl 0xcfc 0xebed205d
  outl 0x5d02 0xedf82049
  EOF
  pci_cfg_write ICH9-LPC 31:0 @0x41 <- 0xebed205d
  hw/pci/pci.c:268: int pci_bus_get_irq_level(PCIBus *, int): Assertion `irq_num < bus->nirq' failed.

This is because ich9_lpc_sci_irq() returns -1 for reserved
(illegal) values, but ich9_lpc_pmbase_sci_update() considers
it valid and store it in a 8-bit unsigned type. Then the 255
value is used as GSI IRQ, resulting in a PIRQ value of 247,
more than ICH9_LPC_NB_PIRQS (8).

Fix by simply ignoring the invalid access (and reporting it):

  pci_cfg_write ICH9-LPC 31:0 @0x41 <- 0xebed205d
  ICH9 LPC: SCI IRQ SEL #3 is reserved
  pci_cfg_read mch 00:0 @0x0 -> 0x8086
  pci_cfg_read mch 00:0 @0x0 -> 0x29c08086
  ...

Cc: qemu-stable@nongnu.org
Reported-by: Alexander Bulekov <alxndr@bu.edu>
Fixes: 8f242cb724 ("ich9: implement SCI_IRQ_SEL register")
BugLink: https://bugs.launchpad.net/qemu/+bug/1878642
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20200717151705.18611-1-f4bug@amsat.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/isa/lpc_ich9.c      | 14 +++++++++++---
 include/hw/i386/ich9.h |  1 +
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 04e5323140..087a18d04d 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -29,6 +29,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/log.h"
 #include "cpu.h"
 #include "qapi/visitor.h"
 #include "qemu/range.h"
@@ -312,10 +313,12 @@ void ich9_generate_smi(void)
     cpu_interrupt(first_cpu, CPU_INTERRUPT_SMI);
 }
 
+/* Returns -1 on error, IRQ number on success */
 static int ich9_lpc_sci_irq(ICH9LPCState *lpc)
 {
-    switch (lpc->d.config[ICH9_LPC_ACPI_CTRL] &
-            ICH9_LPC_ACPI_CTRL_SCI_IRQ_SEL_MASK) {
+    uint8_t sel = lpc->d.config[ICH9_LPC_ACPI_CTRL] &
+                  ICH9_LPC_ACPI_CTRL_SCI_IRQ_SEL_MASK;
+    switch (sel) {
     case ICH9_LPC_ACPI_CTRL_9:
         return 9;
     case ICH9_LPC_ACPI_CTRL_10:
@@ -328,6 +331,8 @@ static int ich9_lpc_sci_irq(ICH9LPCState *lpc)
         return 21;
     default:
         /* reserved */
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "ICH9 LPC: SCI IRQ SEL #%u is reserved\n", sel);
         break;
     }
     return -1;
@@ -459,7 +464,7 @@ ich9_lpc_pmbase_sci_update(ICH9LPCState *lpc)
 {
     uint32_t pm_io_base = pci_get_long(lpc->d.config + ICH9_LPC_PMBASE);
     uint8_t acpi_cntl = pci_get_long(lpc->d.config + ICH9_LPC_ACPI_CTRL);
-    uint8_t new_gsi;
+    int new_gsi;
 
     if (acpi_cntl & ICH9_LPC_ACPI_CTRL_ACPI_EN) {
         pm_io_base &= ICH9_LPC_PMBASE_BASE_ADDRESS_MASK;
@@ -470,6 +475,9 @@ ich9_lpc_pmbase_sci_update(ICH9LPCState *lpc)
     ich9_pm_iospace_update(&lpc->pm, pm_io_base);
 
     new_gsi = ich9_lpc_sci_irq(lpc);
+    if (new_gsi == -1) {
+        return;
+    }
     if (lpc->sci_level && new_gsi != lpc->sci_gsi) {
         qemu_set_irq(lpc->pm.irq, 0);
         lpc->sci_gsi = new_gsi;
diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index 294024be5f..d1ea000d3d 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -144,6 +144,7 @@ struct ICH9LPCState {
 #define ICH9_LPC_PMBASE_BASE_ADDRESS_MASK       Q35_MASK(32, 15, 7)
 #define ICH9_LPC_PMBASE_RTE                     0x1
 #define ICH9_LPC_PMBASE_DEFAULT                 0x1
+
 #define ICH9_LPC_ACPI_CTRL                      0x44
 #define ICH9_LPC_ACPI_CTRL_ACPI_EN              0x80
 #define ICH9_LPC_ACPI_CTRL_SCI_IRQ_SEL_MASK     Q35_MASK(8, 2, 0)
-- 
2.26.2

[PULL 06/20] qtest: add a reproducer for LP#1878642

[Paolo Bonzini]

From: Alexander Bulekov <alxndr@bu.edu>

https://bugs.launchpad.net/qemu/+bug/1878642

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Message-Id: <20201102163336.115444-1-alxndr@bu.edu>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/qtest/fuzz-test.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/tests/qtest/fuzz-test.c b/tests/qtest/fuzz-test.c
index 2f38bb1ec2..9cb4c42bde 100644
--- a/tests/qtest/fuzz-test.c
+++ b/tests/qtest/fuzz-test.c
@@ -34,6 +34,19 @@ static void test_lp1878263_megasas_zero_iov_cnt(void)
     qtest_quit(s);
 }
 
+static void test_lp1878642_pci_bus_get_irq_level_assert(void)
+{
+    QTestState *s;
+
+    s = qtest_init("-M pc-q35-5.0 "
+                   "-nographic -monitor none -serial none "
+                   "-d guest_errors -trace pci*");
+
+    qtest_outl(s, 0xcf8, 0x8400f841);
+    qtest_outl(s, 0xcfc, 0xebed205d);
+    qtest_outl(s, 0x5d02, 0xebed205d);
+}
+
 int main(int argc, char **argv)
 {
     const char *arch = qtest_get_arch();
@@ -43,6 +56,8 @@ int main(int argc, char **argv)
     if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
         qtest_add_func("fuzz/test_lp1878263_megasas_zero_iov_cnt",
                        test_lp1878263_megasas_zero_iov_cnt);
+        qtest_add_func("fuzz/test_lp1878642_pci_bus_get_irq_level_assert",
+                       test_lp1878642_pci_bus_get_irq_level_assert);
     }
 
     return g_test_run();
-- 
2.26.2

[PULL 07/20] meson: use b_staticpic=false for meson >=0.56.0

[Paolo Bonzini]

Meson 0.56.0 correctly builds non-PIC static libraries with -fPIE if
b_pie=true.  We do not have to pass b_staticpic=true if PIE is requested
if Meson is new-enough, which improves performance.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 configure   | 6 +++++-
 meson.build | 4 ++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index 2c3c69f118..c87c1dfbf3 100755
--- a/configure
+++ b/configure
@@ -6961,6 +6961,10 @@ fi
 mv $cross config-meson.cross
 
 rm -rf meson-private meson-info meson-logs
+unset staticpic
+if ! version_ge "$($meson --version)" 0.56.0; then
+  staticpic=$(if test "$pie" = yes; then echo true; else echo false; fi)
+fi
 NINJA=$ninja $meson setup \
         --prefix "$prefix" \
         --libdir "$libdir" \
@@ -6980,7 +6984,7 @@ NINJA=$ninja $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) \
+        ${staticpic:+-Db_staticpic=$staticpic} \
         -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 \
diff --git a/meson.build b/meson.build
index 39ac5cf6d8..f5175010df 100644
--- a/meson.build
+++ b/meson.build
@@ -1,6 +1,6 @@
 project('qemu', ['c'], meson_version: '>=0.55.0',
-        default_options: ['warning_level=1', 'c_std=gnu99', 'cpp_std=gnu++11',
-                          'b_colorout=auto'],
+        default_options: ['warning_level=1', 'c_std=gnu99', 'cpp_std=gnu++11', 'b_colorout=auto'] +
+                         (meson.version().version_compare('>=0.56.0') ? [ 'b_staticpic=false' ] : []),
         version: run_command('head', meson.source_root() / 'VERSION').stdout().strip())
 
 not_found = dependency('', required: false)
-- 
2.26.2

[PULL 08/20] meson: vhost-user-gpu/virtiofsd: use absolute path

[Paolo Bonzini]

From: Marc Hartmayer <mhartmay@linux.ibm.com>

The option `libexecdir` is relative to `prefix` (see
https://mesonbuild.com/Builtin-options.html), so we have to be aware
of this when creating 50-qemu-gpu.json and
50-qemu-virtiofsd.json. Otherwise, tools like libvirt will not be able
to find the executable.

Fixes: 16bf7a3326d8 ("configure: move directory options from config-host.mak to meson")
Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Message-Id: <20201103112333.24734-1-mhartmay@linux.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 contrib/vhost-user-gpu/meson.build | 2 +-
 tools/virtiofsd/meson.build        | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/vhost-user-gpu/meson.build b/contrib/vhost-user-gpu/meson.build
index 37ecca13ca..c487ca72c1 100644
--- a/contrib/vhost-user-gpu/meson.build
+++ b/contrib/vhost-user-gpu/meson.build
@@ -9,6 +9,6 @@ if 'CONFIG_TOOLS' in config_host and 'CONFIG_VIRGL' in config_host \
 
   configure_file(input: '50-qemu-gpu.json.in',
                  output: '50-qemu-gpu.json',
-                 configuration: { 'libexecdir' : get_option('libexecdir') },
+                 configuration: { 'libexecdir' : get_option('prefix') / get_option('libexecdir') },
                  install_dir: qemu_datadir / 'vhost-user')
 endif
diff --git a/tools/virtiofsd/meson.build b/tools/virtiofsd/meson.build
index e1a4dc98d9..17edecf55c 100644
--- a/tools/virtiofsd/meson.build
+++ b/tools/virtiofsd/meson.build
@@ -15,5 +15,5 @@ executable('virtiofsd', files(
 
 configure_file(input: '50-qemu-virtiofsd.json.in',
                output: '50-qemu-virtiofsd.json',
-               configuration: { 'libexecdir' : get_option('libexecdir') },
+               configuration: { 'libexecdir' : get_option('prefix') / get_option('libexecdir') },
                install_dir: qemu_datadir / 'vhost-user')
-- 
2.26.2

[PULL 09/20] tests/qtest/libqtest.c: Check for setsockopt() failure

[Paolo Bonzini]

From: Peter Maydell <peter.maydell@linaro.org>

In socket_accept() we use setsockopt() to set SO_RCVTIMEO,
but we don't check the return value for failure. Do so.

Fixes: Coverity CID 1432321
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20201103115112.19211-1-peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/qtest/libqtest.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 99deff47ef..be0fb430dd 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -110,8 +110,13 @@ static int socket_accept(int sock)
     struct timeval timeout = { .tv_sec = SOCKET_TIMEOUT,
                                .tv_usec = 0 };
 
-    setsockopt(sock, SOL_SOCKET, SO_RCVTIMEO, (void *)&timeout,
-               sizeof(timeout));
+    if (qemu_setsockopt(sock, SOL_SOCKET, SO_RCVTIMEO,
+                        (void *)&timeout, sizeof(timeout))) {
+        fprintf(stderr, "%s failed to set SO_RCVTIMEO: %s\n",
+                __func__, strerror(errno));
+        close(sock);
+        return -1;
+    }
 
     do {
         addrlen = sizeof(addr);
-- 
2.26.2

[PULL 10/20] tests/qtest/libqos/ahci.c: Avoid NULL dereference in ahci_exec()

[Paolo Bonzini]

From: Peter Maydell <peter.maydell@linaro.org>

In ahci_exec() we attempt to permit the caller to pass a NULL pointer
for opts_in (in which case we use a default set of options).  However
although we check for NULL when setting up the opts variable at the
top of the function, we unconditionally dereference opts_in at the
end of the function as part of freeing the opts->buffer.

Switch to checking whether the final buffer is the same as the
buffer we started with, instead of assuming the value we started
with is always opts_in->buffer.

At the moment all the callers pass a non-NULL opts argument, so
we never saw any crashes in practice.

Fixes: Coverity CID 1432302
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20201103115257.23623-1-peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/qtest/libqos/ahci.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/libqos/ahci.c b/tests/qtest/libqos/ahci.c
index 2946abc15a..fba3e7a954 100644
--- a/tests/qtest/libqos/ahci.c
+++ b/tests/qtest/libqos/ahci.c
@@ -637,10 +637,13 @@ void ahci_exec(AHCIQState *ahci, uint8_t port,
     AHCICommand *cmd;
     int rc;
     AHCIOpts *opts;
+    uint64_t buffer_in;
 
     opts = g_memdup((opts_in == NULL ? &default_opts : opts_in),
                     sizeof(AHCIOpts));
 
+    buffer_in = opts->buffer;
+
     /* No guest buffer provided, create one. */
     if (opts->size && !opts->buffer) {
         opts->buffer = ahci_alloc(ahci, opts->size);
@@ -686,7 +689,7 @@ void ahci_exec(AHCIQState *ahci, uint8_t port,
         g_assert_cmpint(rc, ==, 0);
     }
     ahci_command_free(cmd);
-    if (opts->buffer != opts_in->buffer) {
+    if (opts->buffer != buffer_in) {
         ahci_free(ahci, opts->buffer);
     }
     g_free(opts);
-- 
2.26.2

[PULL 11/20] meson: fix warning for bad sphinx-build

[Paolo Bonzini]

The warning was printing an empty string if the bad sphinx-build
was not passed on the command line.  Instead, always use the
path that was returned by find_program.

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 docs/meson.build | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/docs/meson.build b/docs/meson.build
index 8c222f96bb..bf8204a08f 100644
--- a/docs/meson.build
+++ b/docs/meson.build
@@ -27,7 +27,8 @@ if sphinx_build.found()
   build_docs = (sphinx_build_test_out.returncode() == 0)
 
   if not build_docs
-    warning('@0@ exists but it is either too old or uses too old a Python version'.format(get_option('sphinx_build')))
+    warning('@0@ is either too old or uses too old a Python version'
+            .format(sphinx_build.full_path()))
     if get_option('docs').enabled()
       error('Install a Python 3 version of python-sphinx')
     endif
-- 
2.26.2

[PULL 12/20] configure: fix gio_libs reference

[Paolo Bonzini]

From: Bruce Rogers <brogers@suse.com>

The gio library detection code no longer works, due to a missing $ in
front of the gio_libs reference. Make the string be $gio_libs.

Fixes: 76346b6264a ("configure: Test that gio libs from pkg-config
work")

Signed-off-by: Bruce Rogers <brogers@suse.com>
Message-Id: <20201103145121.668865-1-brogers@suse.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index c87c1dfbf3..8c4b5d6d9e 100755
--- a/configure
+++ b/configure
@@ -3499,7 +3499,7 @@ if $pkg_config --atleast-version=$glib_req_ver gio-2.0; then
     # with pkg-config --static --libs data for gio-2.0 that is missing
     # -lblkid and will give a link error.
     write_c_skeleton
-    if compile_prog "" "gio_libs" ; then
+    if compile_prog "" "$gio_libs" ; then
         gio=yes
     else
         gio=no
-- 
2.26.2

[PULL 13/20] tests/qtest: Fix potential NULL pointer dereference in qos_build_main_args()

[Paolo Bonzini]

From: AlexChen <alex.chen@huawei.com>

In qos_build_main_args(), the pointer 'path' is dereferenced before
checking it is valid, which may lead to NULL pointer dereference.
So move the assignment to 'cmd_line' after checking 'path' is valid.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Alex Chen <alex.chen@huawei.com>
Message-Id: <5FA16ED5.4000203@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/qtest/fuzz/qos_fuzz.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/fuzz/qos_fuzz.c b/tests/qtest/fuzz/qos_fuzz.c
index b943577b8c..cee1a2a60f 100644
--- a/tests/qtest/fuzz/qos_fuzz.c
+++ b/tests/qtest/fuzz/qos_fuzz.c
@@ -70,7 +70,7 @@ static GString *qos_build_main_args(void)
 {
     char **path = fuzz_path_vec;
     QOSGraphNode *test_node;
-    GString *cmd_line = g_string_new(path[0]);
+    GString *cmd_line;
     void *test_arg;
 
     if (!path) {
@@ -79,6 +79,7 @@ static GString *qos_build_main_args(void)
     }
 
     /* Before test */
+    cmd_line = g_string_new(path[0]);
     current_path = path;
     test_node = qos_graph_get_node(path[(g_strv_length(path) - 1)]);
     test_arg = test_node->u.test.arg;
-- 
2.26.2

[PULL 14/20] fuzz: fix writing DMA patterns

[Paolo Bonzini]

From: Alexander Bulekov <alxndr@bu.edu>

This code had all sorts of issues. We used a loop similar to
address_space_write_rom, but I did not remove a "break" that only made
sense in the context of the switch statement in the original code. Then,
after the loop, we did a separate qtest_memwrite over the entire DMA
access range, defeating the purpose of the loop. Additionally, we
increment the buf pointer, and then try to g_free() it. Fix these
problems.

Reported-by: OSS-Fuzz (Issue 26725)
Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reported-by: OSS-Fuzz (Issue 26691)
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Message-Id: <20201029172901.534442-2-alxndr@bu.edu>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/qtest/fuzz/generic_fuzz.c | 37 +++++++++++++++------------------
 1 file changed, 17 insertions(+), 20 deletions(-)

diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
index a8f5864883..3e2d50feaa 100644
--- a/tests/qtest/fuzz/generic_fuzz.c
+++ b/tests/qtest/fuzz/generic_fuzz.c
@@ -229,10 +229,10 @@ void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr, bool is_write)
     address_range ar = {addr, len};
     g_array_append_val(dma_regions, ar);
     pattern p = g_array_index(dma_patterns, pattern, dma_pattern_index);
-    void *buf = pattern_alloc(p, ar.size);
+    void *buf_base = pattern_alloc(p, ar.size);
+    void *buf = buf_base;
     hwaddr l, addr1;
     MemoryRegion *mr1;
-    uint8_t *ram_ptr;
     while (len > 0) {
         l = len;
         mr1 = address_space_translate(first_cpu->as,
@@ -244,30 +244,27 @@ void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr, bool is_write)
             l = memory_access_size(mr1, l, addr1);
         } else {
             /* ROM/RAM case */
-            ram_ptr = qemu_map_ram_ptr(mr1->ram_block, addr1);
-            memcpy(ram_ptr, buf, l);
-            break;
+            if (qtest_log_enabled) {
+                /*
+                * With QTEST_LOG, use a normal, slow QTest memwrite. Prefix the log
+                * that will be written by qtest.c with a DMA tag, so we can reorder
+                * the resulting QTest trace so the DMA fills precede the last PIO/MMIO
+                * command.
+                */
+                fprintf(stderr, "[DMA] ");
+                if (double_fetch) {
+                    fprintf(stderr, "[DOUBLE-FETCH] ");
+                }
+                fflush(stderr);
+            }
+            qtest_memwrite(qts_global, addr, buf, l);
         }
         len -= l;
         buf += l;
         addr += l;
 
     }
-    if (qtest_log_enabled) {
-        /*
-         * With QTEST_LOG, use a normal, slow QTest memwrite. Prefix the log
-         * that will be written by qtest.c with a DMA tag, so we can reorder
-         * the resulting QTest trace so the DMA fills precede the last PIO/MMIO
-         * command.
-         */
-        fprintf(stderr, "[DMA] ");
-        if (double_fetch) {
-            fprintf(stderr, "[DOUBLE-FETCH] ");
-        }
-        fflush(stderr);
-    }
-    qtest_memwrite(qts_global, ar.addr, buf, ar.size);
-    g_free(buf);
+    g_free(buf_base);
 
     /* Increment the index of the pattern for the next DMA access */
     dma_pattern_index = (dma_pattern_index + 1) % dma_patterns->len;
-- 
2.26.2

[PULL 15/20] fuzz: check the MR in the DMA callback

[Paolo Bonzini]

From: Alexander Bulekov <alxndr@bu.edu>

We should be checking that the device is trying to read from RAM, before
filling the region with data. Otherwise, we will try to populate
nonsensical addresses in RAM for callbacks on PIO/MMIO reads. We did
this originally, however the final version I sent had the line commented
out..

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Message-Id: <20201029172901.534442-3-alxndr@bu.edu>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/qtest/fuzz/generic_fuzz.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
index 3e2d50feaa..3a5dbc3ce2 100644
--- a/tests/qtest/fuzz/generic_fuzz.c
+++ b/tests/qtest/fuzz/generic_fuzz.c
@@ -192,7 +192,7 @@ void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr, bool is_write)
      */
     if (dma_patterns->len == 0
         || len == 0
-        /* || mr != MACHINE(qdev_get_machine())->ram */
+        || mr != current_machine->ram
         || is_write
         || addr > current_machine->ram_size) {
         return;
-- 
2.26.2

[PULL 16/20] fuzz: fuzz offsets within pio/mmio regions

[Paolo Bonzini]

From: Alexander Bulekov <alxndr@bu.edu>

The code did not add offsets to FlatRange bases, so we did not fuzz
offsets within device MemoryRegions.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Message-Id: <20201029172901.534442-4-alxndr@bu.edu>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/qtest/fuzz/generic_fuzz.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
index 3a5dbc3ce2..262a963d2e 100644
--- a/tests/qtest/fuzz/generic_fuzz.c
+++ b/tests/qtest/fuzz/generic_fuzz.c
@@ -298,6 +298,11 @@ static bool get_io_address(address_range *result, AddressSpace *as,
     } while (cb_info.index != index && !cb_info.found);
 
     *result = cb_info.result;
+    if (result->size) {
+        offset = offset % result->size;
+        result->addr += offset;
+        result->size -= offset;
+    }
     return cb_info.found;
 }
 
-- 
2.26.2

[PULL 17/20] semihosting: fix order of initialization functions

[Paolo Bonzini]

qemu_semihosting_console_init uses semihosting.chardev which is set
by qemu_semihosting_connect_chardevs.  Thus qemu_semihosting_connect_chardevs
has to be called first.

Both have to be called after processing -serial and friends though, so
that the semihosting console can connect to a multiplexer as in
"-serial mon:stdio -semihosting-config chardev=serial0"

Suggested-by: Alex Bennée <alex.bennee@linaro.org>
Fixes: 619985e937 ("semihosting: defer connect_chardevs a little more to use serialx", 2020-07-27)
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 softmmu/vl.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/softmmu/vl.c b/softmmu/vl.c
index a537a0377f..a71164494e 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -4284,9 +4284,6 @@ void qemu_init(int argc, char **argv, char **envp)
     qemu_opts_foreach(qemu_find_opts("mon"),
                       mon_init_func, NULL, &error_fatal);
 
-    /* connect semihosting console input if requested */
-    qemu_semihosting_console_init();
-
     if (foreach_device_config(DEV_SERIAL, serial_parse) < 0)
         exit(1);
     if (foreach_device_config(DEV_PARALLEL, parallel_parse) < 0)
@@ -4296,6 +4293,7 @@ void qemu_init(int argc, char **argv, char **envp)
 
     /* now chardevs have been created we may have semihosting to connect */
     qemu_semihosting_connect_chardevs();
+    qemu_semihosting_console_init();
 
     /* If no default VGA is requested, the default is "none".  */
     if (default_vga) {
-- 
2.26.2

[PULL 18/20] qapi, qemu-options: make all parsing visitors parse boolean options the same

[Paolo Bonzini]

OptsVisitor, StringInputVisitor and the keyval visitor have
three different ideas of how a human could write the value of
a boolean option.  Pay homage to the backwards-compatibility
gods and make the new common helper accept all four sets (on/off,
true/false, y/n and yes/no), but remove case-insensitivity.

Since OptsVisitor is supposed to match qemu-options, adjust
it as well.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20201103161339.447118-1-pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/qapi/util.h           |  2 ++
 qapi/opts-visitor.c           | 14 +-------------
 qapi/qapi-util.c              | 23 +++++++++++++++++++++++
 qapi/qobject-input-visitor.c  |  6 +-----
 qapi/string-input-visitor.c   | 17 +----------------
 tests/qemu-iotests/051.out    |  6 +++---
 tests/qemu-iotests/051.pc.out |  6 +++---
 tests/qemu-iotests/133.out    |  8 ++++----
 tests/qemu-iotests/137.out    |  4 ++--
 util/qemu-option.c            | 20 ++------------------
 10 files changed, 42 insertions(+), 64 deletions(-)

diff --git a/include/qapi/util.h b/include/qapi/util.h
index bc312e90aa..6178e98e97 100644
--- a/include/qapi/util.h
+++ b/include/qapi/util.h
@@ -19,6 +19,8 @@ typedef struct QEnumLookup {
 const char *qapi_enum_lookup(const QEnumLookup *lookup, int val);
 int qapi_enum_parse(const QEnumLookup *lookup, const char *buf,
                     int def, Error **errp);
+bool qapi_bool_parse(const char *name, const char *value, bool *obj,
+                     Error **errp);
 
 int parse_qapi_name(const char *name, bool complete);
 
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 7781c23a42..587f31baf6 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -368,7 +368,6 @@ opts_type_str(Visitor *v, const char *name, char **obj, Error **errp)
 }
 
 
-/* mimics qemu-option.c::parse_option_bool() */
 static bool
 opts_type_bool(Visitor *v, const char *name, bool *obj, Error **errp)
 {
@@ -379,19 +378,8 @@ opts_type_bool(Visitor *v, const char *name, bool *obj, Error **errp)
     if (!opt) {
         return false;
     }
-
     if (opt->str) {
-        if (strcmp(opt->str, "on") == 0 ||
-            strcmp(opt->str, "yes") == 0 ||
-            strcmp(opt->str, "y") == 0) {
-            *obj = true;
-        } else if (strcmp(opt->str, "off") == 0 ||
-            strcmp(opt->str, "no") == 0 ||
-            strcmp(opt->str, "n") == 0) {
-            *obj = false;
-        } else {
-            error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
-                       "on|yes|y|off|no|n");
+        if (!qapi_bool_parse(opt->name, opt->str, obj, errp)) {
             return false;
         }
     } else {
diff --git a/qapi/qapi-util.c b/qapi/qapi-util.c
index 29a6c98b53..dfc0bde497 100644
--- a/qapi/qapi-util.c
+++ b/qapi/qapi-util.c
@@ -13,6 +13,7 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu/ctype.h"
+#include "qapi/qmp/qerror.h"
 
 const char *qapi_enum_lookup(const QEnumLookup *lookup, int val)
 {
@@ -40,6 +41,28 @@ int qapi_enum_parse(const QEnumLookup *lookup, const char *buf,
     return def;
 }
 
+bool qapi_bool_parse(const char *name, const char *value, bool *obj, Error **errp)
+{
+    if (g_str_equal(value, "on") ||
+        g_str_equal(value, "yes") ||
+        g_str_equal(value, "true") ||
+        g_str_equal(value, "y")) {
+        *obj = true;
+        return true;
+    }
+    if (g_str_equal(value, "off") ||
+        g_str_equal(value, "no") ||
+        g_str_equal(value, "false") ||
+        g_str_equal(value, "n")) {
+        *obj = false;
+        return true;
+    }
+
+    error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name,
+               "boolean (on/off)");
+    return false;
+}
+
 /*
  * Parse a valid QAPI name from @str.
  * A valid name consists of letters, digits, hyphen and underscore.
diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index 7b184b50a7..23843b242e 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -512,11 +512,7 @@ static bool qobject_input_type_bool_keyval(Visitor *v, const char *name,
         return false;
     }
 
-    if (!strcmp(str, "on")) {
-        *obj = true;
-    } else if (!strcmp(str, "off")) {
-        *obj = false;
-    } else {
+    if (!qapi_bool_parse(name, str, obj, NULL)) {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
                    full_name(qiv, name), "'on' or 'off'");
         return false;
diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index 6e53396ea3..197139c1c0 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -332,22 +332,7 @@ static bool parse_type_bool(Visitor *v, const char *name, bool *obj,
     StringInputVisitor *siv = to_siv(v);
 
     assert(siv->lm == LM_NONE);
-    if (!strcasecmp(siv->string, "on") ||
-        !strcasecmp(siv->string, "yes") ||
-        !strcasecmp(siv->string, "true")) {
-        *obj = true;
-        return true;
-    }
-    if (!strcasecmp(siv->string, "off") ||
-        !strcasecmp(siv->string, "no") ||
-        !strcasecmp(siv->string, "false")) {
-        *obj = false;
-        return true;
-    }
-
-    error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
-               "boolean");
-    return false;
+    return qapi_bool_parse(name ? name : "null", siv->string, obj, errp);
 }
 
 static bool parse_type_str(Visitor *v, const char *name, char **obj,
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index de4771bcb3..21d2dbec4c 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -110,13 +110,13 @@ QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) quit
 
 Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=: Parameter 'lazy-refcounts' expects 'on' or 'off'
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=: Parameter 'lazy-refcounts' expects boolean (on/off)
 
 Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42: Parameter 'lazy-refcounts' expects 'on' or 'off'
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42: Parameter 'lazy-refcounts' expects boolean (on/off)
 
 Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo: Parameter 'lazy-refcounts' expects 'on' or 'off'
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo: Parameter 'lazy-refcounts' expects boolean (on/off)
 
 
 === With version 2 images enabling lazy refcounts must fail ===
diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out
index f707471fb0..d814b83871 100644
--- a/tests/qemu-iotests/051.pc.out
+++ b/tests/qemu-iotests/051.pc.out
@@ -110,13 +110,13 @@ QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) quit
 
 Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=: Parameter 'lazy-refcounts' expects 'on' or 'off'
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=: Parameter 'lazy-refcounts' expects boolean (on/off)
 
 Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42: Parameter 'lazy-refcounts' expects 'on' or 'off'
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42: Parameter 'lazy-refcounts' expects boolean (on/off)
 
 Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo: Parameter 'lazy-refcounts' expects 'on' or 'off'
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo: Parameter 'lazy-refcounts' expects boolean (on/off)
 
 
 === With version 2 images enabling lazy refcounts must fail ===
diff --git a/tests/qemu-iotests/133.out b/tests/qemu-iotests/133.out
index d70c2e8041..75431c2594 100644
--- a/tests/qemu-iotests/133.out
+++ b/tests/qemu-iotests/133.out
@@ -35,8 +35,8 @@ qemu-io: Cannot set both -c and the cache options
 
 === Check that invalid options are handled correctly ===
 
-qemu-io: Parameter 'read-only' expects 'on' or 'off'
-qemu-io: Parameter 'cache.no-flush' expects 'on' or 'off'
-qemu-io: Parameter 'cache.direct' expects 'on' or 'off'
-qemu-io: Parameter 'auto-read-only' expects 'on' or 'off'
+qemu-io: Parameter 'read-only' expects boolean (on/off)
+qemu-io: Parameter 'cache.no-flush' expects boolean (on/off)
+qemu-io: Parameter 'cache.direct' expects boolean (on/off)
+qemu-io: Parameter 'auto-read-only' expects boolean (on/off)
 *** done
diff --git a/tests/qemu-iotests/137.out b/tests/qemu-iotests/137.out
index 86377c80cd..a420db61d5 100644
--- a/tests/qemu-iotests/137.out
+++ b/tests/qemu-iotests/137.out
@@ -15,7 +15,7 @@ read 33554432/33554432 bytes at offset 0
 
 === Try setting some invalid values ===
 
-qemu-io: Parameter 'lazy-refcounts' expects 'on' or 'off'
+qemu-io: Parameter 'lazy-refcounts' expects boolean (on/off)
 qemu-io: cache-size, l2-cache-size and refcount-cache-size may not be set at the same time
 qemu-io: l2-cache-size may not exceed cache-size
 qemu-io: refcount-cache-size may not exceed cache-size
@@ -38,7 +38,7 @@ wrote 512/512 bytes at offset 0
 ./common.rc: Killed                  ( VALGRIND_QEMU="${VALGRIND_QEMU_IO}" _qemu_proc_exec "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
 OK: Dirty bit not set
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
-qemu-io: Parameter 'lazy-refcounts' expects 'on' or 'off'
+qemu-io: Parameter 'lazy-refcounts' expects boolean (on/off)
 qcow2: Marking image as corrupt: Preventing invalid allocation of L2 table at offset 0; further corruption events will be suppressed
 write failed: Input/output error
 *** done
diff --git a/util/qemu-option.c b/util/qemu-option.c
index b9f93a7f8b..acefbc23fa 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -96,21 +96,6 @@ const char *get_opt_value(const char *p, char **value)
     return offset;
 }
 
-static bool parse_option_bool(const char *name, const char *value, bool *ret,
-                              Error **errp)
-{
-    if (!strcmp(value, "on")) {
-        *ret = 1;
-    } else if (!strcmp(value, "off")) {
-        *ret = 0;
-    } else {
-        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
-                   name, "'on' or 'off'");
-        return false;
-    }
-    return true;
-}
-
 static bool parse_option_number(const char *name, const char *value,
                                 uint64_t *ret, Error **errp)
 {
@@ -363,7 +348,7 @@ static bool qemu_opt_get_bool_helper(QemuOpts *opts, const char *name,
     if (opt == NULL) {
         def_val = find_default_by_name(opts, name);
         if (def_val) {
-            parse_option_bool(name, def_val, &ret, &error_abort);
+            qapi_bool_parse(name, def_val, &ret, &error_abort);
         }
         return ret;
     }
@@ -471,8 +456,7 @@ static bool qemu_opt_parse(QemuOpt *opt, Error **errp)
         /* nothing */
         return true;
     case QEMU_OPT_BOOL:
-        return parse_option_bool(opt->name, opt->str, &opt->value.boolean,
-                                 errp);
+        return qapi_bool_parse(opt->name, opt->str, &opt->value.boolean, errp);
     case QEMU_OPT_NUMBER:
         return parse_option_number(opt->name, opt->str, &opt->value.uint,
                                    errp);
-- 
2.26.2

[PULL 19/20] ivshmem-test: do not use short-form boolean option

[Paolo Bonzini]

This QemuOpts idiom will be deprecated, so get rid of it in the tests.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/qtest/ivshmem-test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qtest/ivshmem-test.c b/tests/qtest/ivshmem-test.c
index d5c8b9f128..dfa69424ed 100644
--- a/tests/qtest/ivshmem-test.c
+++ b/tests/qtest/ivshmem-test.c
@@ -135,7 +135,7 @@ static void setup_vm_cmd(IVState *s, const char *cmd, bool msix)
 static void setup_vm(IVState *s)
 {
     char *cmd = g_strdup_printf("-object memory-backend-file"
-                                ",id=mb1,size=1M,share,mem-path=/dev/shm%s"
+                                ",id=mb1,size=1M,share=on,mem-path=/dev/shm%s"
                                 " -device ivshmem-plain,memdev=mb1", tmpshm);
 
     setup_vm_cmd(s, cmd, false);
-- 
2.26.2

[PULL 20/20] qtest: escape device name in device-introspect-test

[Paolo Bonzini]

device-introspect-test uses HMP, so it should escape the device name
properly.  Because of this, a few devices that had commas in their
names were escaping testing.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/qtest/device-introspect-test.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/device-introspect-test.c b/tests/qtest/device-introspect-test.c
index 9f22340ee5..bbec166dbc 100644
--- a/tests/qtest/device-introspect-test.c
+++ b/tests/qtest/device-introspect-test.c
@@ -104,7 +104,8 @@ static QList *device_type_list(QTestState *qts, bool abstract)
 static void test_one_device(QTestState *qts, const char *type)
 {
     QDict *resp;
-    char *help;
+    char *help, *escaped;
+    GRegex *comma;
 
     g_test_message("Testing device '%s'", type);
 
@@ -113,8 +114,13 @@ static void test_one_device(QTestState *qts, const char *type)
                type);
     qobject_unref(resp);
 
-    help = qtest_hmp(qts, "device_add \"%s,help\"", type);
+    comma = g_regex_new(",", 0, 0, NULL);
+    escaped = g_regex_replace_literal(comma, type, -1, 0, ",,", 0, NULL);
+    g_regex_unref(comma);
+
+    help = qtest_hmp(qts, "device_add \"%s,help\"", escaped);
     g_free(help);
+    g_free(escaped);
 }
 
 static void test_device_intro_list(void)
-- 
2.26.2