From 090afdc5e61e80de4ac158fa4cb2a578ee69be54 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 2 Nov 2020 13:19:53 -0500 Subject: [PATCH 01/20] cutils: replace strdup with g_strdup 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 Signed-off-by: Paolo Bonzini --- 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); From 2eba427eb1e9057ee6bd2b4e6c4c78cda3534bfe Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 29 Oct 2020 13:06:09 +0100 Subject: [PATCH 02/20] docs: expand sourceset documentation Expand on the usage of sourcesets and describe the CONFIG_ALL symbol. Signed-off-by: Paolo Bonzini --- docs/devel/build-system.rst | 148 ++++++++++++++++++++++++------------ docs/devel/kconfig.rst | 2 + 2 files changed, 102 insertions(+), 48 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. + 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. +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. -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. + 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. + 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:: -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`. + # 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 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` -These files rarely need changing unless new devices / hardware need to -be enabled for a particular system/userspace emulation target +`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 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 ================ From 379218511219feddb85fde6362c0eefd94dd4828 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Fri, 30 Oct 2020 16:37:52 +0100 Subject: [PATCH 03/20] exec: Remove dead code (CID 1432876) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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é Message-Id: <20201030153752.1557776-1-philmd@redhat.com> Signed-off-by: Paolo Bonzini --- 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; From f8b8f3746327d577cdd25d6f86bd993178aece58 Mon Sep 17 00:00:00 2001 From: Alexander Bulekov Date: Sun, 1 Nov 2020 16:22:45 -0500 Subject: [PATCH 04/20] scripts/oss-fuzz: rename bin/qemu-fuzz-i386 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 Message-Id: <20201101212245.185819-1-alxndr@bu.edu> Signed-off-by: Paolo Bonzini --- 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 From 4177b062fc58dd250667415e487618ac59393d04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Fri, 17 Jul 2020 17:17:05 +0200 Subject: [PATCH 05/20] hw/isa/lpc_ich9: Ignore reserved/invalid SCI IRQ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Fixes: 8f242cb724 ("ich9: implement SCI_IRQ_SEL register") BugLink: https://bugs.launchpad.net/qemu/+bug/1878642 Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20200717151705.18611-1-f4bug@amsat.org> Signed-off-by: Paolo Bonzini --- 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) From fd250172842b3bbd4213242eb83bd5fa989f7381 Mon Sep 17 00:00:00 2001 From: Alexander Bulekov Date: Mon, 2 Nov 2020 11:33:36 -0500 Subject: [PATCH 06/20] qtest: add a reproducer for LP#1878642 https://bugs.launchpad.net/qemu/+bug/1878642 Suggested-by: Paolo Bonzini Signed-off-by: Alexander Bulekov Message-Id: <20201102163336.115444-1-alxndr@bu.edu> Signed-off-by: Paolo Bonzini --- 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(); From a5cb7c5afe717d42ae8845cd4b8805ea17b3f64d Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 19 Oct 2020 06:56:16 -0400 Subject: [PATCH 07/20] meson: use b_staticpic=false for meson >=0.56.0 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 --- 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) From cd57deabad8f60a6b0d135318810909bc0b7a93f Mon Sep 17 00:00:00 2001 From: Marc Hartmayer Date: Tue, 3 Nov 2020 12:23:33 +0100 Subject: [PATCH 08/20] meson: vhost-user-gpu/virtiofsd: use absolute path 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 Message-Id: <20201103112333.24734-1-mhartmay@linux.ibm.com> Signed-off-by: Paolo Bonzini --- 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') From 1d72d9c4874f61c38df9a473e2fd4de869ba0b11 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 3 Nov 2020 11:51:12 +0000 Subject: [PATCH 09/20] tests/qtest/libqtest.c: Check for setsockopt() failure 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 Message-Id: <20201103115112.19211-1-peter.maydell@linaro.org> Signed-off-by: Paolo Bonzini --- 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); From 0250edf1eb4ff0b164c0cdabcbf4313507f3082e Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 3 Nov 2020 11:52:57 +0000 Subject: [PATCH 10/20] tests/qtest/libqos/ahci.c: Avoid NULL dereference in ahci_exec() 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 Message-Id: <20201103115257.23623-1-peter.maydell@linaro.org> Signed-off-by: Paolo Bonzini --- 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); From af2c0e854bd2b93a61f2d7111822154ccccd86c9 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 3 Nov 2020 08:59:42 -0500 Subject: [PATCH 11/20] meson: fix warning for bad sphinx-build 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 Signed-off-by: Paolo Bonzini --- 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 From 3569a5dfc11f2c4880b7cddd9e0091b29aa0e015 Mon Sep 17 00:00:00 2001 From: Bruce Rogers Date: Tue, 3 Nov 2020 07:51:21 -0700 Subject: [PATCH 12/20] configure: fix gio_libs reference 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 Message-Id: <20201103145121.668865-1-brogers@suse.com> Signed-off-by: Paolo Bonzini --- 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 From c59c582d56ee3bbde15e6788c0d28329792b2573 Mon Sep 17 00:00:00 2001 From: AlexChen Date: Tue, 3 Nov 2020 22:53:09 +0800 Subject: [PATCH 13/20] tests/qtest: Fix potential NULL pointer dereference in qos_build_main_args() 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 Signed-off-by: Alex Chen Message-Id: <5FA16ED5.4000203@huawei.com> Signed-off-by: Paolo Bonzini --- 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; From a9f67c1d51dda405bc6a406d13c8802b98df904e Mon Sep 17 00:00:00 2001 From: Alexander Bulekov Date: Thu, 29 Oct 2020 13:28:58 -0400 Subject: [PATCH 14/20] fuzz: fix writing DMA patterns 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 Reported-by: OSS-Fuzz (Issue 26691) Reviewed-by: Darren Kenny Message-Id: <20201029172901.534442-2-alxndr@bu.edu> Signed-off-by: Paolo Bonzini --- 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; From cc3d99c7418925b9f252482d67055e7c3f2c4814 Mon Sep 17 00:00:00 2001 From: Alexander Bulekov Date: Thu, 29 Oct 2020 13:28:59 -0400 Subject: [PATCH 15/20] fuzz: check the MR in the DMA callback 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 Reviewed-by: Darren Kenny Message-Id: <20201029172901.534442-3-alxndr@bu.edu> Signed-off-by: Paolo Bonzini --- 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; From 953e6d7c0e94126dbfdb63ba1546e6b74ed9ccee Mon Sep 17 00:00:00 2001 From: Alexander Bulekov Date: Thu, 29 Oct 2020 13:29:00 -0400 Subject: [PATCH 16/20] fuzz: fuzz offsets within pio/mmio regions The code did not add offsets to FlatRange bases, so we did not fuzz offsets within device MemoryRegions. Signed-off-by: Alexander Bulekov Reviewed-by: Darren Kenny Message-Id: <20201029172901.534442-4-alxndr@bu.edu> Signed-off-by: Paolo Bonzini --- 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; } From e19c520fe6545230bf37c2458c6d97ed18cd0c4f Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 21 Oct 2020 16:08:53 -0400 Subject: [PATCH 17/20] semihosting: fix order of initialization functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Fixes: 619985e937 ("semihosting: defer connect_chardevs a little more to use serialx", 2020-07-27) Signed-off-by: Paolo Bonzini --- 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) { From 794b95608f8d92189baba697a7d6072e94ed2e0e Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 3 Nov 2020 07:37:46 -0500 Subject: [PATCH 18/20] ivshmem-test: do not use short-form boolean option This QemuOpts idiom will be deprecated, so get rid of it in the tests. Signed-off-by: Paolo Bonzini --- 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); From e27bd4987699df5f49a03e93cf57941abeb82938 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 3 Nov 2020 06:57:46 -0500 Subject: [PATCH 19/20] qtest: escape device name in device-introspect-test 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 --- 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) From 372bcb2585cd2f06a01f13b1a208370ccf7479cf Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 3 Nov 2020 11:13:39 -0500 Subject: [PATCH 20/20] qapi, qemu-options: make all parsing visitors parse boolean options the same MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Daniel P. Berrangé Reviewed-by: Markus Armbruster Message-Id: <20201103161339.447118-1-pbonzini@redhat.com> Signed-off-by: Paolo Bonzini --- 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 +---------------- util/qemu-option.c | 20 ++------------------ 6 files changed, 30 insertions(+), 52 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..3c24bb3d45 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, + "'on' or '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/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);