Re: [Qemu-devel] [PATCH V9 2/4] tests/migration: Support cross compilation in generating boot header file
On Thu, Sep 06, 2018 at 12:23:45PM -0400, Wei Huang wrote:
>
>
> - Original Message -
> > From: "Andrew Jones"
> > To: "Wei Huang"
> > Cc: lviv...@redhat.com, "peter maydell" ,
> > quint...@redhat.com, qemu-devel@nongnu.org,
> > dgilb...@redhat.com, "alex bennee"
> > Sent: Thursday, September 6, 2018 9:00:33 AM
> > Subject: Re: [Qemu-devel] [PATCH V9 2/4] tests/migration: Support cross
> > compilation in generating boot header file
> >
> > On Thu, Sep 06, 2018 at 09:37:04AM -0400, Wei Huang wrote:
> > >
> > >
> > > - Original Message -
> > > > From: "Andrew Jones"
> > > > To: "Wei Huang"
> > > > Cc: qemu-devel@nongnu.org, lviv...@redhat.com, "peter maydell"
> > > > , quint...@redhat.com,
> > > > dgilb...@redhat.com, "alex bennee"
> > > > Sent: Thursday, September 6, 2018 7:03:32 AM
> > > > Subject: Re: [Qemu-devel] [PATCH V9 2/4] tests/migration: Support cross
> > > > compilation in generating boot header file
> > > >
> > > > On Wed, Sep 05, 2018 at 03:15:32PM -0400, Wei Huang wrote:
> > > > > Recently a new configure option, CROSS_CC_GUEST, was added to
> > > > > $(TARGET)-softmmu/config-target.mak to support TCG-related tests. This
> > > > > patch tries to leverage this option to support cross compilation when
> > > > > the
> > > > > migration boot block file is being re-generated:
> > > > >
> > > > > * The x86 related files are moved to a new sub-dir (named ./i386).
> > > > > * A new top-layer Makefile is created in tests/migration/ directory.
> > > > >This Makefile searches and parses CROSS_CC_GUEST to generate
> > > > >CROSS_PREFIX.
> > > > >The CROSS_PREFIX, if available, is then passed to
> > > > >migration/$ARCH/Makefile.
> > > > >
> > > > > Reviewed-by: Juan Quintela
> > > > > Signed-off-by: Wei Huang
> > > > > ---
> > > > > tests/migration-test.c | 2 +-
> > > > > tests/migration/Makefile | 44
> > > > > --
> > > > > tests/migration/i386/Makefile | 22 +++
> > > > > .../{x86-a-b-bootblock.S => i386/a-b-bootblock.S} | 4 --
> > > > > .../{x86-a-b-bootblock.h => i386/a-b-bootblock.h} | 8 ++--
> > > > > 5 files changed, 51 insertions(+), 29 deletions(-)
> > > > > create mode 100644 tests/migration/i386/Makefile
> > > > > rename tests/migration/{x86-a-b-bootblock.S => i386/a-b-bootblock.S}
> > > > > (93%)
> > > > > rename tests/migration/{x86-a-b-bootblock.h => i386/a-b-bootblock.h}
> > > > > (92%)
> > > > >
> > > > > diff --git a/tests/migration-test.c b/tests/migration-test.c
> > > > > index 0e687b7..fe6b41a 100644
> > > > > --- a/tests/migration-test.c
> > > > > +++ b/tests/migration-test.c
> > > > > @@ -83,7 +83,7 @@ static const char *tmpfs;
> > > > > /* A simple PC boot sector that modifies memory (1-100MB) quickly
> > > > > * outputting a 'B' every so often if it's still running.
> > > > > */
> > > > > -#include "tests/migration/x86-a-b-bootblock.h"
> > > > > +#include "tests/migration/i386/a-b-bootblock.h"
> > > > >
> > > > > static void init_bootfile_x86(const char *bootpath)
> > > > > {
> > > > > diff --git a/tests/migration/Makefile b/tests/migration/Makefile
> > > > > index c0824b4..a9ed875 100644
> > > > > --- a/tests/migration/Makefile
> > > > > +++ b/tests/migration/Makefile
> > > > > @@ -1,31 +1,35 @@
> > > > > -# To specify cross compiler prefix, use CROSS_PREFIX=
> > > > > -# $ make CROSS_PREFIX=x86_64-linux-gnu-
> > > > > +#
> > > > > +# Copyright (c) 2018 Red Hat, Inc. and/or its affiliates
> > > > > +#
> > > > > +# This work is licensed under the terms of the GNU GPL, version 2 or
> > > > > later.
> > > > > +# See the COPYING file in the top-level directory.
> > > > > +#
> > > > > +
> > > > > +TARGET_LIST = i386
> > > > > +
> > > > > +SRC_PATH = ../..
> > > > >
> > > > > override define __note
> > > > > -/* This file is automatically generated from
> > > > > - * tests/migration/x86-a-b-bootblock.S, edit that and then run
> > > > > - * tests/migration/rebuild-x86-bootblock.sh to update,
> > > > > - * and then remember to send both in your patch submission.
> > > > > +/* This file is automatically generated from the assembly file in
> > > > > + * tests/migration/$@. Edit that file and then run "make all"
> > > > > + * inside tests/migration to update, and then remember to send both
> > > > > + * the header and the assembler differences in your patch submission.
> > > > > */
> > > > > endef
> > > > > export __note
> > > > >
> > > > > -.PHONY: all clean
> > > > > -all: x86-a-b-bootblock.h
> > > > > -
> > > > > -x86-a-b-bootblock.h: x86.bootsect
> > > > > - echo "$$__note" > header.tmp
> > > > > - xxd -i $< | sed -e 's/.*int.*//' >> header.tmp
> > > > > - mv header.tmp $@
> > > > > +find-arch-cross-cc = $(lastword $(shell grep -h "CROSS_CC_GUEST="
> > > > > $(wildcard $(SRC_PATH)/$(patsubst
> > > > > i386,*86*,$(1))-softmmu/config-target.mak)))
> > > >
> > > > The above function hangs u
[Qemu-devel] [PULL 00/14] ppc-for-3.1 queue 20180907
The following changes since commit 19b599f7664b2ebfd0f405fb79c14dd241557452: Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2018-08-27-v2' into staging (2018-08-27 16:44:20 +0100) are available in the Git repository at: git://github.com/dgibson/qemu.git tags/ppc-for-3.1-20180907 for you to fetch changes up to be0c46d464c7c6b601adcd21fe9d2dd054a6a2cf: target-ppc: Extend HWCAP2 bits for ISA 3.0 (2018-09-07 11:29:50 +1000) ppc patch queue 2018-09-07 Here's another pull request for qemu-3.1. No real theme here, just an assortment of various fixes. Probably the most notable thing is the removal of the ppcemb target which has been deprecated for some time now. Emilio G. Cota (1): spapr: fix leak of rev array Greg Kurz (1): spapr_pci: fix potential NULL pointer dereference Jose Ricardo Ziviani (1): Fix a deadlock case in the CPU hotplug flow Mark Cave-Ayland (7): macio: move MACIOIDEState type declarations to macio.h macio: add macio bus to help with fw path generation macio: add addr property to macio IDE object grackle: set device fw_name and address for correct fw path generation mac_oldworld: implement custom FWPathProvider uninorth: add ofw-addr property to allow correct fw path generation mac_newworld: implement custom FWPathProvider Nikunj A Dadhania (1): target/ppc/kvm: set vcpu as online/offline Sam Bobroff (1): spapr: Correct reference count on spapr-cpu-core Sandipan Das (1): target-ppc: Extend HWCAP2 bits for ISA 3.0 Thomas Huth (1): ppc: Remove deprecated ppcemb target configure | 13 ++--- cpus.c | 1 - default-configs/ppcemb-softmmu.mak | 23 --- hw/ide/macio.c | 2 ++ hw/misc/macio/macio.c | 42 --- hw/pci-host/grackle.c | 17 +++ hw/pci-host/uninorth.c | 16 +++ hw/ppc/mac.h | 26 - hw/ppc/mac_newworld.c | 59 +- hw/ppc/mac_oldworld.c | 59 +- hw/ppc/ppc405_boards.c | 14 - hw/ppc/ppc440_bamboo.c | 7 - hw/ppc/sam460ex.c | 7 - hw/ppc/spapr.c | 3 ++ hw/ppc/spapr_cpu_core.c| 11 +++ hw/ppc/spapr_pci.c | 2 +- hw/ppc/spapr_rtas.c| 2 ++ hw/ppc/virtex_ml507.c | 7 - include/exec/poison.h | 1 - include/hw/misc/macio/macio.h | 37 include/hw/pci-host/uninorth.h | 1 + linux-user/elfload.c | 2 ++ qapi/common.json | 2 +- qemu-deprecated.texi | 6 target/ppc/cpu-qom.h | 2 -- target/ppc/cpu.h | 16 --- target/ppc/kvm.c | 13 +++-- target/ppc/kvm_ppc.h | 7 + target/ppc/mmu_helper.c| 6 ++-- target/ppc/translate_init.inc.c| 35 +- tests/machine-none-test.c | 1 - 31 files changed, 259 insertions(+), 181 deletions(-) delete mode 100644 default-configs/ppcemb-softmmu.mak
[Qemu-devel] [PULL 02/14] spapr: fix leak of rev array
From: "Emilio G. Cota"
Introduced in 04d595b300 ("spapr: do not use CPU_FOREACH_REVERSE",
2018-08-23)
Fixes: CID1395181
Reported-by: Peter Maydell
Signed-off-by: Emilio G. Cota
Reviewed-by: Richard Henderson
Signed-off-by: David Gibson
---
hw/ppc/spapr.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 4edb6c7d16..505d4c84e5 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -607,6 +607,7 @@ static void spapr_populate_cpus_dt_node(void *fdt,
sPAPRMachineState *spapr)
spapr_populate_cpu_dt(cs, fdt, offset, spapr);
}
+g_free(rev);
}
static uint32_t spapr_pc_dimm_node(MemoryDeviceInfoList *list, ram_addr_t addr)
--
2.17.1
[Qemu-devel] [PULL 01/14] ppc: Remove deprecated ppcemb target
From: Thomas Huth
There is no known available OS for ppc around anymore that uses page
sizes below 4k, so it does not make much sense that we keep wasting
our time on building and testing the ppcemb-softmmu target. It has
been deprecated since two releases, and nobody complained, so let's
remove this now.
Signed-off-by: Thomas Huth
Signed-off-by: David Gibson
---
configure | 13 +++
cpus.c | 1 -
default-configs/ppcemb-softmmu.mak | 23
hw/ppc/ppc405_boards.c | 14
hw/ppc/ppc440_bamboo.c | 7 --
hw/ppc/sam460ex.c | 7 --
hw/ppc/virtex_ml507.c | 7 --
include/exec/poison.h | 1 -
qapi/common.json | 2 +-
qemu-deprecated.texi | 6 -
target/ppc/cpu-qom.h | 2 --
target/ppc/cpu.h | 16 --
target/ppc/kvm.c | 4 +---
target/ppc/mmu_helper.c| 6 ++---
target/ppc/translate_init.inc.c| 35 +-
tests/machine-none-test.c | 1 -
16 files changed, 9 insertions(+), 136 deletions(-)
delete mode 100644 default-configs/ppcemb-softmmu.mak
diff --git a/configure b/configure
index 58862d2ae8..7fd989aee1 100755
--- a/configure
+++ b/configure
@@ -195,8 +195,7 @@ supported_kvm_target() {
i386:i386 | i386:x86_64 | i386:x32 | \
x86_64:i386 | x86_64:x86_64 | x86_64:x32 | \
mips:mips | mipsel:mips | \
-ppc:ppc | ppcemb:ppc | ppc64:ppc | \
-ppc:ppc64 | ppcemb:ppc64 | ppc64:ppc64 | \
+ppc:ppc | ppc64:ppc | ppc:ppc64 | ppc64:ppc64 | \
s390x:s390x)
return 0
;;
@@ -6951,7 +6950,7 @@ if test "$linux" = "yes" ; then
i386|x86_64|x32)
linux_arch=x86
;;
- ppcemb|ppc|ppc64)
+ ppc|ppc64)
linux_arch=powerpc
;;
s390x)
@@ -6981,7 +6980,7 @@ target_name=$(echo $target | cut -d '-' -f 1)
target_bigendian="no"
case "$target_name" in
-
armeb|aarch64_be|hppa|lm32|m68k|microblaze|mips|mipsn32|mips64|moxie|or1k|ppc|ppcemb|ppc64|ppc64abi32|s390x|sh4eb|sparc|sparc64|sparc32plus|xtensaeb)
+
armeb|aarch64_be|hppa|lm32|m68k|microblaze|mips|mipsn32|mips64|moxie|or1k|ppc|ppc64|ppc64abi32|s390x|sh4eb|sparc|sparc64|sparc32plus|xtensaeb)
target_bigendian=yes
;;
esac
@@ -7109,12 +7108,6 @@ case "$target_name" in
gdb_xml_files="power-core.xml power-fpu.xml power-altivec.xml
power-spe.xml"
target_compiler=$cross_cc_powerpc
;;
- ppcemb)
-TARGET_BASE_ARCH=ppc
-TARGET_ABI_DIR=ppc
-gdb_xml_files="power-core.xml power-fpu.xml power-altivec.xml
power-spe.xml"
-target_compiler=$cross_cc_ppcemb
- ;;
ppc64)
TARGET_BASE_ARCH=ppc
TARGET_ABI_DIR=ppc
diff --git a/cpus.c b/cpus.c
index 8ee6e5db93..f66cb67066 100644
--- a/cpus.c
+++ b/cpus.c
@@ -2251,7 +2251,6 @@ static CpuInfoArch
sysemu_target_to_cpuinfo_arch(SysEmuTarget target)
return CPU_INFO_ARCH_X86;
case SYS_EMU_TARGET_PPC:
-case SYS_EMU_TARGET_PPCEMB:
case SYS_EMU_TARGET_PPC64:
return CPU_INFO_ARCH_PPC;
diff --git a/default-configs/ppcemb-softmmu.mak
b/default-configs/ppcemb-softmmu.mak
deleted file mode 100644
index ac44f150c6..00
--- a/default-configs/ppcemb-softmmu.mak
+++ /dev/null
@@ -1,23 +0,0 @@
-# Default configuration for ppcemb-softmmu
-
-include pci.mak
-include sound.mak
-include usb.mak
-CONFIG_PPC4XX=y
-CONFIG_M48T59=y
-CONFIG_SERIAL=y
-CONFIG_SERIAL_ISA=y
-CONFIG_I8257=y
-CONFIG_OPENPIC=y
-CONFIG_PFLASH_CFI01=y
-CONFIG_PFLASH_CFI02=y
-CONFIG_PTIMER=y
-CONFIG_I8259=y
-CONFIG_XILINX=y
-CONFIG_XILINX_ETHLITE=y
-CONFIG_USB_EHCI_SYSBUS=y
-CONFIG_SM501=y
-CONFIG_DDC=y
-CONFIG_IDE_SII3112=y
-CONFIG_I2C=y
-CONFIG_BITBANG_I2C=y
diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index f5a9c24b6c..3be3fe4432 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -162,13 +162,6 @@ static void ref405ep_init(MachineState *machine)
DriveInfo *dinfo;
MemoryRegion *sysmem = get_system_memory();
-#ifdef TARGET_PPCEMB
-if (!qtest_enabled()) {
-warn_report("qemu-system-ppcemb is deprecated, "
-"please use qemu-system-ppc instead.");
-}
-#endif
-
/* XXX: fix this */
memory_region_allocate_system_memory(&ram_memories[0], NULL, "ef405ep.ram",
0x0800);
@@ -463,13 +456,6 @@ static void taihu_405ep_init(MachineState *machine)
int fl_idx, fl_sectors;
DriveInfo *dinfo;
-#ifdef TARGET_PPCEMB
-if (!qtest_enabled()) {
-warn_report("qemu-system-ppcemb is deprecated, "
-"please use qemu-system-ppc instead.");
-}
-#endif
-
/* RAM is soldered to the board so the size cannot be changed */
ram_size = 0x0800;
memory_region_allocate_system_memory(ram, NULL, "taihu_405ep.ram",
diff --git a/hw/ppc/ppc440_bam
[Qemu-devel] [PULL 10/14] mac_newworld: implement custom FWPathProvider
From: Mark Cave-Ayland
This enables the correct generation of bootdevice fw paths for in-built IDE
and virtio-pci-blk devices suitable for OpenBIOS.
Note we also set the MachineClass ignore_boot_device_suffixes property to true
since an additional disk node should not be added except for virtio devices.
Signed-off-by: Mark Cave-Ayland
Signed-off-by: David Gibson
---
hw/ppc/mac_newworld.c | 58 ++-
1 file changed, 57 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 325013f563..a630cb81cd 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -64,6 +64,7 @@
#include "hw/ppc/openpic.h"
#include "hw/ide.h"
#include "hw/loader.h"
+#include "hw/fw-path-provider.h"
#include "elf.h"
#include "qemu/error-report.h"
#include "sysemu/kvm.h"
@@ -521,6 +522,54 @@ static void ppc_core99_init(MachineState *machine)
qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
}
+/*
+ * Implementation of an interface to adjust firmware path
+ * for the bootindex property handling.
+ */
+static char *core99_fw_dev_path(FWPathProvider *p, BusState *bus,
+DeviceState *dev)
+{
+PCIDevice *pci;
+IDEBus *ide_bus;
+IDEState *ide_s;
+MACIOIDEState *macio_ide;
+
+if (!strcmp(object_get_typename(OBJECT(dev)), "macio-newworld")) {
+pci = PCI_DEVICE(dev);
+return g_strdup_printf("mac-io@%x", PCI_SLOT(pci->devfn));
+}
+
+if (!strcmp(object_get_typename(OBJECT(dev)), "macio-ide")) {
+macio_ide = MACIO_IDE(dev);
+return g_strdup_printf("ata-3@%x", macio_ide->addr);
+}
+
+if (!strcmp(object_get_typename(OBJECT(dev)), "ide-drive")) {
+ide_bus = IDE_BUS(qdev_get_parent_bus(dev));
+ide_s = idebus_active_if(ide_bus);
+
+if (ide_s->drive_kind == IDE_CD) {
+return g_strdup("cdrom");
+}
+
+return g_strdup("hd");
+}
+
+if (!strcmp(object_get_typename(OBJECT(dev)), "ide-hd")) {
+return g_strdup("hd");
+}
+
+if (!strcmp(object_get_typename(OBJECT(dev)), "ide-cd")) {
+return g_strdup("cdrom");
+}
+
+if (!strcmp(object_get_typename(OBJECT(dev)), "virtio-blk-device")) {
+return g_strdup("disk");
+}
+
+return NULL;
+}
+
static int core99_kvm_type(const char *arg)
{
/* Always force PR KVM */
@@ -530,6 +579,7 @@ static int core99_kvm_type(const char *arg)
static void core99_machine_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
+FWPathProviderClass *fwc = FW_PATH_PROVIDER_CLASS(oc);
mc->desc = "Mac99 based PowerMAC";
mc->init = ppc_core99_init;
@@ -543,6 +593,8 @@ static void core99_machine_class_init(ObjectClass *oc, void
*data)
#else
mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("7400_v2.9");
#endif
+mc->ignore_boot_device_suffixes = true;
+fwc->get_dev_path = core99_fw_dev_path;
}
static char *core99_get_via_config(Object *obj, Error **errp)
@@ -599,7 +651,11 @@ static const TypeInfo core99_machine_info = {
.parent= TYPE_MACHINE,
.class_init= core99_machine_class_init,
.instance_init = core99_instance_init,
-.instance_size = sizeof(Core99MachineState)
+.instance_size = sizeof(Core99MachineState),
+.interfaces = (InterfaceInfo[]) {
+{ TYPE_FW_PATH_PROVIDER },
+{ }
+},
};
static void mac_machine_register_types(void)
--
2.17.1
[Qemu-devel] [PULL 14/14] target-ppc: Extend HWCAP2 bits for ISA 3.0
From: Sandipan Das
This adds the HWCAP2 bit to detect if a linux user process is
running on an ISA 3.0 compliant cpu like POWER9. This can be
verified using a simple test program that prints the value in
the auxiliary vector for AT_HWCAP2 as shown below.
Before:
$ qemu-ppc64le -cpu power8 test
0x8c00
$ qemu-ppc64le -cpu power9 test
0x8c00
After:
$ qemu-ppc64le -cpu power8 test
0x8c00
$ qemu-ppc64le -cpu power9 test
0x8c80
Signed-off-by: Sandipan Das
Reviewed-by: Laurent Vivier
Signed-off-by: David Gibson
---
linux-user/elfload.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 8638612aec..e97c4cde49 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -710,6 +710,7 @@ enum {
QEMU_PPC_FEATURE2_HAS_EBB = 0x1000, /* Event Base Branching */
QEMU_PPC_FEATURE2_HAS_ISEL = 0x0800, /* Integer Select */
QEMU_PPC_FEATURE2_HAS_TAR = 0x0400, /* Target Address Register */
+QEMU_PPC_FEATURE2_ARCH_3_00 = 0x0080, /* ISA 3.00 */
};
#define ELF_HWCAP get_elf_hwcap()
@@ -764,6 +765,7 @@ static uint32_t get_elf_hwcap2(void)
GET_FEATURE2(PPC2_BCTAR_ISA207, QEMU_PPC_FEATURE2_HAS_TAR);
GET_FEATURE2((PPC2_BCTAR_ISA207 | PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
PPC2_ISA207S), QEMU_PPC_FEATURE2_ARCH_2_07);
+GET_FEATURE2(PPC2_ISA300, QEMU_PPC_FEATURE2_ARCH_3_00);
#undef GET_FEATURE
#undef GET_FEATURE2
--
2.17.1
[Qemu-devel] [PULL 04/14] macio: move MACIOIDEState type declarations to macio.h
From: Mark Cave-Ayland
Signed-off-by: Mark Cave-Ayland
Signed-off-by: David Gibson
---
hw/ide/macio.c| 1 +
hw/ppc/mac.h | 26 --
include/hw/misc/macio/macio.h | 26 ++
3 files changed, 27 insertions(+), 26 deletions(-)
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index d3a85cba3b..f23961e241 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -26,6 +26,7 @@
#include "hw/hw.h"
#include "hw/ppc/mac.h"
#include "hw/ppc/mac_dbdma.h"
+#include "hw/misc/macio/macio.h"
#include "sysemu/block-backend.h"
#include "sysemu/dma.h"
diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
index 41fd289e81..a741300ac9 100644
--- a/hw/ppc/mac.h
+++ b/hw/ppc/mac.h
@@ -86,32 +86,6 @@ typedef struct Core99MachineState {
uint8_t via_config;
} Core99MachineState;
-/* MacIO */
-#define TYPE_MACIO_IDE "macio-ide"
-#define MACIO_IDE(obj) OBJECT_CHECK(MACIOIDEState, (obj), TYPE_MACIO_IDE)
-
-typedef struct MACIOIDEState {
-/*< private >*/
-SysBusDevice parent_obj;
-/*< public >*/
-uint32_t channel;
-qemu_irq real_ide_irq;
-qemu_irq real_dma_irq;
-qemu_irq ide_irq;
-qemu_irq dma_irq;
-
-MemoryRegion mem;
-IDEBus bus;
-IDEDMA dma;
-void *dbdma;
-bool dma_active;
-uint32_t timing_reg;
-uint32_t irq_reg;
-} MACIOIDEState;
-
-void macio_ide_init_drives(MACIOIDEState *ide, DriveInfo **hd_table);
-void macio_ide_register_dma(MACIOIDEState *ide);
-
/* Grackle PCI */
#define TYPE_GRACKLE_PCI_HOST_BRIDGE "grackle-pcihost"
diff --git a/include/hw/misc/macio/macio.h b/include/hw/misc/macio/macio.h
index cfaa145500..0c3964ec12 100644
--- a/include/hw/misc/macio/macio.h
+++ b/include/hw/misc/macio/macio.h
@@ -34,6 +34,32 @@
#include "hw/ppc/mac_dbdma.h"
#include "hw/ppc/openpic.h"
+/* MacIO IDE */
+#define TYPE_MACIO_IDE "macio-ide"
+#define MACIO_IDE(obj) OBJECT_CHECK(MACIOIDEState, (obj), TYPE_MACIO_IDE)
+
+typedef struct MACIOIDEState {
+/*< private >*/
+SysBusDevice parent_obj;
+/*< public >*/
+uint32_t channel;
+qemu_irq real_ide_irq;
+qemu_irq real_dma_irq;
+qemu_irq ide_irq;
+qemu_irq dma_irq;
+
+MemoryRegion mem;
+IDEBus bus;
+IDEDMA dma;
+void *dbdma;
+bool dma_active;
+uint32_t timing_reg;
+uint32_t irq_reg;
+} MACIOIDEState;
+
+void macio_ide_init_drives(MACIOIDEState *ide, DriveInfo **hd_table);
+void macio_ide_register_dma(MACIOIDEState *ide);
+
#define TYPE_MACIO "macio"
#define MACIO(obj) OBJECT_CHECK(MacIOState, (obj), TYPE_MACIO)
--
2.17.1
[Qemu-devel] [PULL 08/14] mac_oldworld: implement custom FWPathProvider
From: Mark Cave-Ayland
This enables the correct generation of bootdevice fw paths for in-built IDE
and virtio-pci-blk devices suitable for OpenBIOS.
Note we also set the MachineClass ignore_boot_device_suffixes property to true
since an additional disk node should not be added except for virtio devices.
Signed-off-by: Mark Cave-Ayland
Signed-off-by: David Gibson
---
hw/ppc/mac_oldworld.c | 58 ++-
1 file changed, 57 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index c7b73e274f..9891c325a9 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -42,6 +42,7 @@
#include "hw/misc/macio/macio.h"
#include "hw/ide.h"
#include "hw/loader.h"
+#include "hw/fw-path-provider.h"
#include "elf.h"
#include "qemu/error-report.h"
#include "sysemu/kvm.h"
@@ -373,6 +374,54 @@ static void ppc_heathrow_init(MachineState *machine)
qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
}
+/*
+ * Implementation of an interface to adjust firmware path
+ * for the bootindex property handling.
+ */
+static char *heathrow_fw_dev_path(FWPathProvider *p, BusState *bus,
+ DeviceState *dev)
+{
+PCIDevice *pci;
+IDEBus *ide_bus;
+IDEState *ide_s;
+MACIOIDEState *macio_ide;
+
+if (!strcmp(object_get_typename(OBJECT(dev)), "macio-oldworld")) {
+pci = PCI_DEVICE(dev);
+return g_strdup_printf("mac-io@%x", PCI_SLOT(pci->devfn));
+}
+
+if (!strcmp(object_get_typename(OBJECT(dev)), "macio-ide")) {
+macio_ide = MACIO_IDE(dev);
+return g_strdup_printf("ata-3@%x", macio_ide->addr);
+}
+
+if (!strcmp(object_get_typename(OBJECT(dev)), "ide-drive")) {
+ide_bus = IDE_BUS(qdev_get_parent_bus(dev));
+ide_s = idebus_active_if(ide_bus);
+
+if (ide_s->drive_kind == IDE_CD) {
+return g_strdup("cdrom");
+}
+
+return g_strdup("hd");
+}
+
+if (!strcmp(object_get_typename(OBJECT(dev)), "ide-hd")) {
+return g_strdup("hd");
+}
+
+if (!strcmp(object_get_typename(OBJECT(dev)), "ide-cd")) {
+return g_strdup("cdrom");
+}
+
+if (!strcmp(object_get_typename(OBJECT(dev)), "virtio-blk-device")) {
+return g_strdup("disk");
+}
+
+return NULL;
+}
+
static int heathrow_kvm_type(const char *arg)
{
/* Always force PR KVM */
@@ -382,6 +431,7 @@ static int heathrow_kvm_type(const char *arg)
static void heathrow_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
+FWPathProviderClass *fwc = FW_PATH_PROVIDER_CLASS(oc);
mc->desc = "Heathrow based PowerMAC";
mc->init = ppc_heathrow_init;
@@ -395,12 +445,18 @@ static void heathrow_class_init(ObjectClass *oc, void
*data)
mc->kvm_type = heathrow_kvm_type;
mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("750_v3.1");
mc->default_display = "std";
+mc->ignore_boot_device_suffixes = true;
+fwc->get_dev_path = heathrow_fw_dev_path;
}
static const TypeInfo ppc_heathrow_machine_info = {
.name = MACHINE_TYPE_NAME("g3beige"),
.parent= TYPE_MACHINE,
-.class_init= heathrow_class_init
+.class_init= heathrow_class_init,
+.interfaces = (InterfaceInfo[]) {
+{ TYPE_FW_PATH_PROVIDER },
+{ }
+},
};
static void ppc_heathrow_register_types(void)
--
2.17.1
[Qemu-devel] [PULL 11/14] spapr: Correct reference count on spapr-cpu-core
From: Sam Bobroff spapr_init_cpus() currently creates spapr-cpu-core objects via object_new() and setting their realized property to true. This leaves their reference count at two, because object_new() adds an initial reference and the realization attaches them to a default parent object which also increments the reference count. This causes a problem if one of these cores is hot unplugged: no delete event is generated for it because it's reference count doesn't reach zero when it is detached from it's parent. Correct this by adding a call to object_unref() in spapr_init_cpus(). Signed-off-by: Sam Bobroff Signed-off-by: David Gibson --- hw/ppc/spapr.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 505d4c84e5..4a9dd4d9bc 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -2480,6 +2480,8 @@ static void spapr_init_cpus(sPAPRMachineState *spapr) object_property_set_int(core, core_id, CPU_CORE_PROP_CORE_ID, &error_fatal); object_property_set_bool(core, true, "realized", &error_fatal); + +object_unref(core); } } } -- 2.17.1
[Qemu-devel] [PULL 06/14] macio: add addr property to macio IDE object
From: Mark Cave-Ayland
This contains the offset of the IDE controller within the macio address space
and is required to allow the address to be included within the fw path.
Signed-off-by: Mark Cave-Ayland
Signed-off-by: David Gibson
---
hw/ide/macio.c| 1 +
hw/misc/macio/macio.c | 5 +++--
include/hw/misc/macio/macio.h | 1 +
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index f23961e241..bab8c45a43 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -461,6 +461,7 @@ static void macio_ide_initfn(Object *obj)
static Property macio_ide_properties[] = {
DEFINE_PROP_UINT32("channel", MACIOIDEState, channel, 0),
+DEFINE_PROP_UINT32("addr", MACIOIDEState, addr, -1),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index 229bfddb90..94da85c8d7 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -219,10 +219,11 @@ static void macio_init_ide(MacIOState *s, MACIOIDEState
*ide, size_t ide_size,
int index)
{
gchar *name = g_strdup_printf("ide[%i]", index);
+uint32_t addr = 0x1f000 + ((index + 1) * 0x1000);
macio_init_child_obj(s, name, ide, ide_size, TYPE_MACIO_IDE);
-memory_region_add_subregion(&s->bar, 0x1f000 + ((index + 1) * 0x1000),
-&ide->mem);
+qdev_prop_set_uint32(DEVICE(ide), "addr", addr);
+memory_region_add_subregion(&s->bar, addr, &ide->mem);
g_free(name);
}
diff --git a/include/hw/misc/macio/macio.h b/include/hw/misc/macio/macio.h
index 3189973ee6..970058b6ed 100644
--- a/include/hw/misc/macio/macio.h
+++ b/include/hw/misc/macio/macio.h
@@ -51,6 +51,7 @@ typedef struct MACIOIDEState {
/*< private >*/
SysBusDevice parent_obj;
/*< public >*/
+uint32_t addr;
uint32_t channel;
qemu_irq real_ide_irq;
qemu_irq real_dma_irq;
--
2.17.1
[Qemu-devel] [PULL 09/14] uninorth: add ofw-addr property to allow correct fw path generation
From: Mark Cave-Ayland
Signed-off-by: Mark Cave-Ayland
Signed-off-by: David Gibson
---
hw/pci-host/uninorth.c | 16
hw/ppc/mac_newworld.c | 1 +
include/hw/pci-host/uninorth.h | 1 +
3 files changed, 18 insertions(+)
diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c
index a843aa7b36..1378c5c7fb 100644
--- a/hw/pci-host/uninorth.c
+++ b/hw/pci-host/uninorth.c
@@ -118,6 +118,13 @@ static void pci_unin_init_irqs(UNINHostState *s)
}
}
+static char *pci_unin_main_ofw_unit_address(const SysBusDevice *dev)
+{
+UNINHostState *s = UNI_NORTH_PCI_HOST_BRIDGE(dev);
+
+return g_strdup_printf("%x", s->ofw_addr);
+}
+
static void pci_unin_main_realize(DeviceState *dev, Error **errp)
{
UNINHostState *s = UNI_NORTH_PCI_HOST_BRIDGE(dev);
@@ -455,12 +462,21 @@ static const TypeInfo unin_internal_pci_host_info = {
},
};
+static Property pci_unin_main_pci_host_props[] = {
+DEFINE_PROP_UINT32("ofw-addr", UNINHostState, ofw_addr, -1),
+DEFINE_PROP_END_OF_LIST()
+};
+
static void pci_unin_main_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
+SysBusDeviceClass *sbc = SYS_BUS_DEVICE_CLASS(klass);
dc->realize = pci_unin_main_realize;
+dc->props = pci_unin_main_pci_host_props;
set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
+dc->fw_name = "pci";
+sbc->explicit_ofw_unit_address = pci_unin_main_ofw_unit_address;
}
static const TypeInfo pci_unin_main_info = {
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index a6b95f024c..325013f563 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -344,6 +344,7 @@ static void ppc_core99_init(MachineState *machine)
/* Uninorth main bus */
dev = qdev_create(NULL, TYPE_UNI_NORTH_PCI_HOST_BRIDGE);
+qdev_prop_set_uint32(dev, "ofw-addr", 0xf200);
object_property_set_link(OBJECT(dev), OBJECT(pic_dev), "pic",
&error_abort);
qdev_init_nofail(dev);
diff --git a/include/hw/pci-host/uninorth.h b/include/hw/pci-host/uninorth.h
index 2a1cf9f284..060324536a 100644
--- a/include/hw/pci-host/uninorth.h
+++ b/include/hw/pci-host/uninorth.h
@@ -49,6 +49,7 @@
typedef struct UNINHostState {
PCIHostState parent_obj;
+uint32_t ofw_addr;
OpenPICState *pic;
qemu_irq irqs[4];
MemoryRegion pci_mmio;
--
2.17.1
[Qemu-devel] [PULL 12/14] Fix a deadlock case in the CPU hotplug flow
From: Jose Ricardo Ziviani
We need to set cs->halted to 1 before calling ppc_set_compat. The reason
is that ppc_set_compat kicks up the new thread created to manage the
hotplugged KVM virtual CPU and the code drives directly to KVM_RUN
ioctl. When cs->halted is 1, the code:
int kvm_cpu_exec(CPUState *cpu)
...
if (kvm_arch_process_async_events(cpu)) {
atomic_set(&cpu->exit_request, 0);
return EXCP_HLT;
}
...
returns before it reaches KVM_RUN, giving time to the main thread to
finish its job. Otherwise we can fall in a deadlock because the KVM
thread will issue the KVM_RUN ioctl while the main thread is setting up
KVM registers. Depending on how these jobs are scheduled we'll end up
freezing QEMU.
The following output shows kvm_vcpu_ioctl sleeping because it cannot get
the mutex and never will.
PS: kvm_vcpu_ioctl was triggered kvm_set_one_reg - compat_pvr.
STATE: TASK_UNINTERRUPTIBLE|TASK_WAKEKILL
PID: 61564 TASK: c03e981e0780 CPU: 48 COMMAND: "qemu-system-ppc"
#0 [c03e982679a0] __schedule at c0b10a44
#1 [c03e98267a60] schedule at c0b113a8
#2 [c03e98267a90] schedule_preempt_disabled at c0b11910
#3 [c03e98267ab0] __mutex_lock at c0b132ec
#4 [c03e98267bc0] kvm_vcpu_ioctl at c0080ea03140 [kvm]
#5 [c03e98267d20] do_vfs_ioctl at c0407d30
#6 [c03e98267dc0] ksys_ioctl at c0408674
#7 [c03e98267e10] sys_ioctl at c04086f8
#8 [c03e98267e30] system_call at c000b488
crash> struct -x kvm.vcpus 0xc03da000
vcpus = {0xc03db488, 0xc03d52b8, 0xc039e9c8,
0xc03d0e20, 0xc03d5828, 0x0, 0x0, ...}
crash> struct -x kvm_vcpu.mutex.owner 0xc03d5828
mutex.owner = {
counter = 0xc03a23a5c881 <- flag 1: waiters
},
crash> bt 0xc03a23a5c880
PID: 61579 TASK: c03a23a5c880 CPU: 9 COMMAND: "CPU 4/KVM"
(active)
crash> struct -x kvm_vcpu.mutex.wait_list 0xc03d5828
mutex.wait_list = {
next = 0xc03e98267b10,
prev = 0xc03e98267b10
},
crash> struct -x mutex_waiter.task 0xc03e98267b10
task = 0xc03e981e0780
The following command-line was used to reproduce the problem (note: gdb
and trace can change the results).
$ qemu-ppc/build/ppc64-softmmu/qemu-system-ppc64 -cpu host \
-enable-kvm -m 4096 \
-smp 4,maxcpus=8,sockets=1,cores=2,threads=4 \
-display none -nographic \
-drive file=disk1.qcow2,format=qcow2
...
(qemu) device_add host-spapr-cpu-core,core-id=4
[no interaction is possible after it, only SIGKILL to take the terminal
back]
Signed-off-by: Jose Ricardo Ziviani
Reviewed-by: Greg Kurz
Signed-off-by: David Gibson
---
hw/ppc/spapr_cpu_core.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 876f0b3d9d..a73b244a3f 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -34,16 +34,16 @@ static void spapr_cpu_reset(void *opaque)
cpu_reset(cs);
-/* Set compatibility mode to match the boot CPU, which was either set
- * by the machine reset code or by CAS. This should never fail.
- */
-ppc_set_compat(cpu, POWERPC_CPU(first_cpu)->compat_pvr, &error_abort);
-
/* All CPUs start halted. CPU0 is unhalted from the machine level
* reset code and the rest are explicitly started up by the guest
* using an RTAS call */
cs->halted = 1;
+/* Set compatibility mode to match the boot CPU, which was either set
+ * by the machine reset code or by CAS. This should never fail.
+ */
+ppc_set_compat(cpu, POWERPC_CPU(first_cpu)->compat_pvr, &error_abort);
+
env->spr[SPR_HIOR] = 0;
lpcr = env->spr[SPR_LPCR];
--
2.17.1
[Qemu-devel] [PULL 07/14] grackle: set device fw_name and address for correct fw path generation
From: Mark Cave-Ayland
Signed-off-by: Mark Cave-Ayland
Signed-off-by: David Gibson
---
hw/pci-host/grackle.c | 17 +
hw/ppc/mac_oldworld.c | 1 +
2 files changed, 18 insertions(+)
diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
index 4810a4de79..5a151e93e9 100644
--- a/hw/pci-host/grackle.c
+++ b/hw/pci-host/grackle.c
@@ -37,6 +37,7 @@
typedef struct GrackleState {
PCIHostState parent_obj;
+uint32_t ofw_addr;
HeathrowState *pic;
qemu_irq irqs[4];
MemoryRegion pci_mmio;
@@ -146,12 +147,28 @@ static const TypeInfo grackle_pci_info = {
},
};
+static char *grackle_ofw_unit_address(const SysBusDevice *dev)
+{
+GrackleState *s = GRACKLE_PCI_HOST_BRIDGE(dev);
+
+return g_strdup_printf("%x", s->ofw_addr);
+}
+
+static Property grackle_properties[] = {
+DEFINE_PROP_UINT32("ofw-addr", GrackleState, ofw_addr, -1),
+DEFINE_PROP_END_OF_LIST()
+};
+
static void grackle_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
+SysBusDeviceClass *sbc = SYS_BUS_DEVICE_CLASS(klass);
dc->realize = grackle_realize;
+dc->props = grackle_properties;
set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
+dc->fw_name = "pci";
+sbc->explicit_ofw_unit_address = grackle_ofw_unit_address;
}
static const TypeInfo grackle_host_info = {
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 80b5525775..c7b73e274f 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -254,6 +254,7 @@ static void ppc_heathrow_init(MachineState *machine)
/* Grackle PCI host bridge */
dev = qdev_create(NULL, TYPE_GRACKLE_PCI_HOST_BRIDGE);
+qdev_prop_set_uint32(dev, "ofw-addr", 0x8000);
object_property_set_link(OBJECT(dev), OBJECT(pic_dev), "pic",
&error_abort);
qdev_init_nofail(dev);
--
2.17.1
[Qemu-devel] [PULL 03/14] spapr_pci: fix potential NULL pointer dereference
From: Greg Kurz Commit 2c88b098e76fd added a call to SPAPR_MACHINE_GET_CLASS(spapr) in spapr_phb_realize() before we check spapr isn't NULL. This causes QEMU to crash when starting a non-pseries machine with a sPAPR PHB. This could be fixed by setting the smc variable after the null check, but it seems more explicit to use a ternary operator to skip the call to SPAPR_MACHINE_GET_CLASS() if spapr is NULL, since spapr_phb_realize() will return immediately in this case. This was reported by Coverity (CID 1395170 and 1395183). Fixes: 2c88b098e76fde0c7fcc0476dd3f80ce58409505 Signed-off-by: Greg Kurz Reviewed-by: Cédric Le Goater Signed-off-by: David Gibson --- hw/ppc/spapr_pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index 5cd676e443..6bcb4f419b 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -1559,7 +1559,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) sPAPRMachineState *spapr = (sPAPRMachineState *) object_dynamic_cast(qdev_get_machine(), TYPE_SPAPR_MACHINE); -sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr); +sPAPRMachineClass *smc = spapr ? SPAPR_MACHINE_GET_CLASS(spapr) : NULL; SysBusDevice *s = SYS_BUS_DEVICE(dev); sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(s); PCIHostState *phb = PCI_HOST_BRIDGE(s); -- 2.17.1
[Qemu-devel] [PULL 05/14] macio: add macio bus to help with fw path generation
From: Mark Cave-Ayland
As the in-built IDE controller is attached to the macio bus then we should also
model this the same in QEMU to aid fw path generation.
Note that all existing macio devices are moved onto the new macio bus so that
the qdev tree accurately reflects the real hardware.
Signed-off-by: Mark Cave-Ayland
Signed-off-by: David Gibson
---
hw/misc/macio/macio.c | 37 ++-
include/hw/misc/macio/macio.h | 10 ++
2 files changed, 38 insertions(+), 9 deletions(-)
diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index 52aa3775f4..229bfddb90 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -90,6 +90,15 @@ static void macio_bar_setup(MacIOState *s)
macio_escc_legacy_setup(s);
}
+static void macio_init_child_obj(MacIOState *s, const char *childname,
+ void *child, size_t childsize,
+ const char *childtype)
+{
+object_initialize_child(OBJECT(s), childname, child, childsize, childtype,
+&error_abort, NULL);
+qdev_set_parent_bus(DEVICE(child), BUS(&s->macio_bus));
+}
+
static void macio_common_realize(PCIDevice *d, Error **errp)
{
MacIOState *s = MACIO(d);
@@ -211,7 +220,7 @@ static void macio_init_ide(MacIOState *s, MACIOIDEState
*ide, size_t ide_size,
{
gchar *name = g_strdup_printf("ide[%i]", index);
-sysbus_init_child_obj(OBJECT(s), name, ide, ide_size, TYPE_MACIO_IDE);
+macio_init_child_obj(s, name, ide, ide_size, TYPE_MACIO_IDE);
memory_region_add_subregion(&s->bar, 0x1f000 + ((index + 1) * 0x1000),
&ide->mem);
g_free(name);
@@ -229,7 +238,7 @@ static void macio_oldworld_init(Object *obj)
qdev_prop_allow_set_link_before_realize,
0, NULL);
-sysbus_init_child_obj(obj, "cuda", &s->cuda, sizeof(s->cuda), TYPE_CUDA);
+macio_init_child_obj(s, "cuda", &s->cuda, sizeof(s->cuda), TYPE_CUDA);
object_initialize(&os->nvram, sizeof(os->nvram), TYPE_MACIO_NVRAM);
dev = DEVICE(&os->nvram);
@@ -340,7 +349,7 @@ static void macio_newworld_realize(PCIDevice *d, Error
**errp)
object_property_set_link(OBJECT(&s->pmu), OBJECT(sysbus_dev), "gpio",
&error_abort);
qdev_prop_set_bit(DEVICE(&s->pmu), "has-adb", ns->has_adb);
-qdev_set_parent_bus(DEVICE(&s->pmu), sysbus_get_default());
+qdev_set_parent_bus(DEVICE(&s->pmu), BUS(&s->macio_bus));
object_property_add_child(OBJECT(s), "pmu", OBJECT(&s->pmu), NULL);
object_property_set_bool(OBJECT(&s->pmu), true, "realized", &err);
@@ -356,7 +365,7 @@ static void macio_newworld_realize(PCIDevice *d, Error
**errp)
} else {
/* CUDA */
object_initialize(&s->cuda, sizeof(s->cuda), TYPE_CUDA);
-qdev_set_parent_bus(DEVICE(&s->cuda), sysbus_get_default());
+qdev_set_parent_bus(DEVICE(&s->cuda), BUS(&s->macio_bus));
object_property_add_child(OBJECT(s), "cuda", OBJECT(&s->cuda), NULL);
qdev_prop_set_uint64(DEVICE(&s->cuda), "timebase-frequency",
s->frequency);
@@ -385,8 +394,8 @@ static void macio_newworld_init(Object *obj)
qdev_prop_allow_set_link_before_realize,
0, NULL);
-sysbus_init_child_obj(obj, "gpio", &ns->gpio, sizeof(ns->gpio),
- TYPE_MACIO_GPIO);
+macio_init_child_obj(s, "gpio", &ns->gpio, sizeof(ns->gpio),
+ TYPE_MACIO_GPIO);
for (i = 0; i < 2; i++) {
macio_init_ide(s, &ns->ide[i], sizeof(ns->ide[i]), i);
@@ -399,10 +408,13 @@ static void macio_instance_init(Object *obj)
memory_region_init(&s->bar, obj, "macio", 0x8);
-sysbus_init_child_obj(obj, "dbdma", &s->dbdma, sizeof(s->dbdma),
- TYPE_MAC_DBDMA);
+qbus_create_inplace(&s->macio_bus, sizeof(s->macio_bus), TYPE_MACIO_BUS,
+DEVICE(obj), "macio.0");
-sysbus_init_child_obj(obj, "escc", &s->escc, sizeof(s->escc), TYPE_ESCC);
+macio_init_child_obj(s, "dbdma", &s->dbdma, sizeof(s->dbdma),
+ TYPE_MAC_DBDMA);
+
+macio_init_child_obj(s, "escc", &s->escc, sizeof(s->escc), TYPE_ESCC);
}
static const VMStateDescription vmstate_macio_oldworld = {
@@ -470,6 +482,12 @@ static void macio_class_init(ObjectClass *klass, void
*data)
dc->user_creatable = false;
}
+static const TypeInfo macio_bus_info = {
+.name = TYPE_MACIO_BUS,
+.parent = TYPE_BUS,
+.instance_size = sizeof(MacIOBusState),
+};
+
static const TypeInfo macio_oldworld_type_info = {
.name = TYPE_OLDWORLD_MACIO,
.parent= TYPE_MACIO,
@@ -501,6 +519,7 @@ static const TypeInfo macio_type_info = {
static void macio_register_types(void)
{
+type_register_static(&macio_bus_info);
type
[Qemu-devel] [PULL 13/14] target/ppc/kvm: set vcpu as online/offline
From: Nikunj A Dadhania
Set the newly added register(KVM_REG_PPC_ONLINE) to indicate if the vcpu is
online(1) or offline(0)
KVM will use this information to set the RWMR register, which controls the PURR
and SPURR accumulation.
CC: pau...@samba.org
Signed-off-by: Nikunj A Dadhania
Signed-off-by: David Gibson
---
hw/ppc/spapr_cpu_core.c | 1 +
hw/ppc/spapr_rtas.c | 2 ++
target/ppc/kvm.c| 9 +
target/ppc/kvm_ppc.h| 7 +++
4 files changed, 19 insertions(+)
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index a73b244a3f..2398ce62c0 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -90,6 +90,7 @@ void spapr_cpu_set_entry_state(PowerPCCPU *cpu, target_ulong
nip, target_ulong r
env->nip = nip;
env->gpr[3] = r3;
+kvmppc_set_reg_ppc_online(cpu, 1);
CPU(cpu)->halted = 0;
/* Enable Power-saving mode Exit Cause exceptions */
ppc_store_lpcr(cpu, env->spr[SPR_LPCR] | pcc->lpcr_pm);
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 4ac96bc94b..d6a0952154 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -33,6 +33,7 @@
#include "sysemu/device_tree.h"
#include "sysemu/cpus.h"
#include "sysemu/hw_accel.h"
+#include "kvm_ppc.h"
#include "hw/ppc/spapr.h"
#include "hw/ppc/spapr_vio.h"
@@ -207,6 +208,7 @@ static void rtas_stop_self(PowerPCCPU *cpu,
sPAPRMachineState *spapr,
* guest */
ppc_store_lpcr(cpu, env->spr[SPR_LPCR] & ~pcc->lpcr_pm);
cs->halted = 1;
+kvmppc_set_reg_ppc_online(cpu, 0);
qemu_cpu_kick(cs);
}
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index ef63842217..30aeafa7de 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -2783,3 +2783,12 @@ bool kvmppc_pvr_workaround_required(PowerPCCPU *cpu)
return !kvmppc_is_pr(cs->kvm_state);
}
+
+void kvmppc_set_reg_ppc_online(PowerPCCPU *cpu, unsigned int online)
+{
+CPUState *cs = CPU(cpu);
+
+if (kvm_enabled()) {
+kvm_set_one_reg(cs, KVM_REG_PPC_ONLINE, &online);
+}
+}
diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
index 657582bb32..f696c6e498 100644
--- a/target/ppc/kvm_ppc.h
+++ b/target/ppc/kvm_ppc.h
@@ -72,6 +72,7 @@ bool kvmppc_pvr_workaround_required(PowerPCCPU *cpu);
bool kvmppc_hpt_needs_host_contiguous_pages(void);
void kvm_check_mmu(PowerPCCPU *cpu, Error **errp);
+void kvmppc_set_reg_ppc_online(PowerPCCPU *cpu, unsigned int online);
#else
@@ -187,6 +188,12 @@ static inline target_ulong
kvmppc_configure_v3_mmu(PowerPCCPU *cpu,
return 0;
}
+static inline void kvmppc_set_reg_ppc_online(PowerPCCPU *cpu,
+ unsigned int online)
+{
+return;
+}
+
#ifndef CONFIG_USER_ONLY
static inline bool kvmppc_spapr_use_multitce(void)
{
--
2.17.1
Re: [Qemu-devel] [PATCH] hw/intc/arm_gic: Drop GIC_BASE_IRQ macro
On 8/24/18 6:18 PM, Peter Maydell wrote:
> The GIC_BASE_IRQ macro is a leftover from when we shared code
> between the GICv2 and the v7M NVIC. Since the NVIC is now
> split off, GIC_BASE_IRQ is always 0, and we can just delete it.
>
> Signed-off-by: Peter Maydell
Reviewed-by: Luc Michel
> ---
> hw/intc/gic_internal.h | 2 --
> hw/intc/arm_gic.c| 31 ++-
> hw/intc/arm_gic_common.c | 1 -
> 3 files changed, 14 insertions(+), 20 deletions(-)
>
> diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h
> index 45c2af0bf59..8d29b40ca10 100644
> --- a/hw/intc/gic_internal.h
> +++ b/hw/intc/gic_internal.h
> @@ -26,8 +26,6 @@
>
> #define ALL_CPU_MASK ((unsigned)(((1 << GIC_NCPU) - 1)))
>
> -#define GIC_BASE_IRQ 0
> -
> #define GIC_DIST_SET_ENABLED(irq, cm) (s->irq_state[irq].enabled |= (cm))
> #define GIC_DIST_CLEAR_ENABLED(irq, cm) (s->irq_state[irq].enabled &= ~(cm))
> #define GIC_DIST_TEST_ENABLED(irq, cm) ((s->irq_state[irq].enabled & (cm))
> != 0)
> diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
> index 542b4b93eab..b3ac2d11fc5 100644
> --- a/hw/intc/arm_gic.c
> +++ b/hw/intc/arm_gic.c
> @@ -955,7 +955,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr
> offset, MemTxAttrs attrs)
> res = 0;
> if (!(s->security_extn && !attrs.secure) && gic_has_groups(s)) {
> /* Every byte offset holds 8 group status bits */
> -irq = (offset - 0x080) * 8 + GIC_BASE_IRQ;
> +irq = (offset - 0x080) * 8;
> if (irq >= s->num_irq) {
> goto bad_reg;
> }
> @@ -974,7 +974,6 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr
> offset, MemTxAttrs attrs)
> irq = (offset - 0x100) * 8;
> else
> irq = (offset - 0x180) * 8;
> -irq += GIC_BASE_IRQ;
> if (irq >= s->num_irq)
> goto bad_reg;
> res = 0;
> @@ -994,7 +993,6 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr
> offset, MemTxAttrs attrs)
> irq = (offset - 0x200) * 8;
> else
> irq = (offset - 0x280) * 8;
> -irq += GIC_BASE_IRQ;
> if (irq >= s->num_irq)
> goto bad_reg;
> res = 0;
> @@ -1019,7 +1017,6 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr
> offset, MemTxAttrs attrs)
> goto bad_reg;
> }
>
> -irq += GIC_BASE_IRQ;
> if (irq >= s->num_irq)
> goto bad_reg;
> res = 0;
> @@ -1036,7 +1033,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr
> offset, MemTxAttrs attrs)
> }
> } else if (offset < 0x800) {
> /* Interrupt Priority. */
> -irq = (offset - 0x400) + GIC_BASE_IRQ;
> +irq = (offset - 0x400);
> if (irq >= s->num_irq)
> goto bad_reg;
> res = gic_dist_get_priority(s, cpu, irq, attrs);
> @@ -1046,7 +1043,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr
> offset, MemTxAttrs attrs)
> /* For uniprocessor GICs these RAZ/WI */
> res = 0;
> } else {
> -irq = (offset - 0x800) + GIC_BASE_IRQ;
> +irq = (offset - 0x800);
> if (irq >= s->num_irq) {
> goto bad_reg;
> }
> @@ -1060,7 +1057,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr
> offset, MemTxAttrs attrs)
> }
> } else if (offset < 0xf00) {
> /* Interrupt Configuration. */
> -irq = (offset - 0xc00) * 4 + GIC_BASE_IRQ;
> +irq = (offset - 0xc00) * 4;
> if (irq >= s->num_irq)
> goto bad_reg;
> res = 0;
> @@ -1183,7 +1180,7 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
> */
> if (!(s->security_extn && !attrs.secure) && gic_has_groups(s)) {
> /* Every byte offset holds 8 group status bits */
> -irq = (offset - 0x80) * 8 + GIC_BASE_IRQ;
> +irq = (offset - 0x80) * 8;
> if (irq >= s->num_irq) {
> goto bad_reg;
> }
> @@ -1204,7 +1201,7 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
> }
> } else if (offset < 0x180) {
> /* Interrupt Set Enable. */
> -irq = (offset - 0x100) * 8 + GIC_BASE_IRQ;
> +irq = (offset - 0x100) * 8;
> if (irq >= s->num_irq)
> goto bad_reg;
> if (irq < GIC_NR_SGIS) {
> @@ -1239,7 +1236,7 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
> }
> } else if (offset < 0x200) {
> /* Interrupt Clear Enable. */
> -irq = (offset - 0x180) * 8 + GIC_BASE_IRQ;
> +irq = (offset - 0x180) * 8;
> if (irq >= s->num_irq)
> goto bad_reg;
> if (irq < GIC_NR_SGIS) {
> @@ -1264,7 +1261,7 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
> }
>
[Qemu-devel] [Bug 1790975] Re: Default arm virt machine broken
Hi Jonathan, I sent an email yesterday on the qemu ML. " Please can you try using qemu-system-arm -machine virt,highmem=off -m 1024M -kernel zImage -serial stdio Does your guest support LPAE? This may be the cause. Thanks Eric " -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1790975 Title: Default arm virt machine broken Status in QEMU: New Bug description: This occurs on qemu_v3.0.0 but not on qemu_v2.12.2 (built from qemu_v3.0.0 tag on github) Symptom: You'll see something like this in the kernel output: [1.285210] OF: PCI: host bridge /pcie@1000 ranges: [1.286246] OF: PCI:IO 0x3eff..0x3eff -> 0x [1.287061] OF: PCI: MEM 0x1000..0x3efe -> 0x1000 [1.287820] OF: PCI: MEM 0x80..0xff -> 0x80 [1.289312] pci-host-generic 401000.pcie: can't claim ECAM area [mem 0x1000-0x1fff]: address conflict with /pcie@1000 [mem 0x1000-0x3efe] [1.290984] pci-host-generic: probe of 401000.pcie failed with error -16 Qemu Command Line: qemu-system-arm -machine virt -m 1024M -kernel zImage -serial stdio I can post my zImage if anyone has problems reproducing with their own zImage. Note that this problem breaks pci making the machine unusable. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1790975/+subscriptions
[Qemu-devel] [PATCH v2 01/12] qdev-monitor: print help to stdout
qdev_device_help() is used from command line "-device help", or from
HMP "device_add". If used from command line, print help to stdout
(it is only printed on explicit demand).
Signed-off-by: Marc-André Lureau
---
include/monitor/monitor.h | 2 ++
monitor.c | 16 +---
qdev-monitor.c| 32 +++-
3 files changed, 34 insertions(+), 16 deletions(-)
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 2ef5e04b37..e7667fda35 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -47,4 +47,6 @@ int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd);
void monitor_fdset_dup_fd_remove(int dup_fd);
int monitor_fdset_dup_fd_find(int dup_fd);
+void out_vprintf(FILE *stream, const char *fmt, va_list ap) GCC_FMT_ATTR(2, 0);
+
#endif /* MONITOR_H */
diff --git a/monitor.c b/monitor.c
index 021c11b1bf..6ea4082ab5 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4597,19 +4597,29 @@ static void monitor_readline_flush(void *opaque)
}
/*
- * Print to current monitor if we have one, else to stderr.
+ * Print to current monitor if we have one, else to stream.
* TODO should return int, so callers can calculate width, but that
* requires surgery to monitor_vprintf(). Left for another day.
*/
-void error_vprintf(const char *fmt, va_list ap)
+void out_vprintf(FILE *stream, const char *fmt, va_list ap)
{
if (cur_mon && !monitor_cur_is_qmp()) {
monitor_vprintf(cur_mon, fmt, ap);
} else {
-vfprintf(stderr, fmt, ap);
+vfprintf(stream, fmt, ap);
}
}
+/*
+ * Print to current monitor if we have one, else to stderr.
+ * TODO should return int, so callers can calculate width, but that
+ * requires surgery to monitor_vprintf(). Left for another day.
+ */
+void error_vprintf(const char *fmt, va_list ap)
+{
+out_vprintf(stderr, fmt, ap);
+}
+
void error_vprintf_unless_qmp(const char *fmt, va_list ap)
{
if (cur_mon && !monitor_cur_is_qmp()) {
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 61e0300991..1da324aba6 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -104,22 +104,31 @@ static bool qdev_class_has_alias(DeviceClass *dc)
return (qdev_class_get_alias(dc) != NULL);
}
+static void out_printf(const char *fmt, ...)
+{
+va_list ap;
+
+va_start(ap, fmt);
+out_vprintf(stdout, fmt, ap);
+va_end(ap);
+}
+
static void qdev_print_devinfo(DeviceClass *dc)
{
-error_printf("name \"%s\"", object_class_get_name(OBJECT_CLASS(dc)));
+out_printf("name \"%s\"", object_class_get_name(OBJECT_CLASS(dc)));
if (dc->bus_type) {
-error_printf(", bus %s", dc->bus_type);
+out_printf(", bus %s", dc->bus_type);
}
if (qdev_class_has_alias(dc)) {
-error_printf(", alias \"%s\"", qdev_class_get_alias(dc));
+out_printf(", alias \"%s\"", qdev_class_get_alias(dc));
}
if (dc->desc) {
-error_printf(", desc \"%s\"", dc->desc);
+out_printf(", desc \"%s\"", dc->desc);
}
if (!dc->user_creatable) {
-error_printf(", no-user");
+out_printf(", no-user");
}
-error_printf("\n");
+out_printf("\n");
}
static void qdev_print_devinfos(bool show_no_user)
@@ -155,8 +164,7 @@ static void qdev_print_devinfos(bool show_no_user)
continue;
}
if (!cat_printed) {
-error_printf("%s%s devices:\n", i ? "\n" : "",
- cat_name[i]);
+out_printf("%s%s devices:\n", i ? "\n" : "", cat_name[i]);
cat_printed = true;
}
qdev_print_devinfo(dc);
@@ -278,13 +286,11 @@ int qdev_device_help(QemuOpts *opts)
}
for (prop = prop_list; prop; prop = prop->next) {
-error_printf("%s.%s=%s", driver,
- prop->value->name,
- prop->value->type);
+out_printf("%s.%s=%s", driver, prop->value->name, prop->value->type);
if (prop->value->has_description) {
-error_printf(" (%s)\n", prop->value->description);
+out_printf(" (%s)\n", prop->value->description);
} else {
-error_printf("\n");
+out_printf("\n");
}
}
--
2.19.0.rc1
[Qemu-devel] [PATCH v2 00/12] Various qemu command line options help improvements
Hi, This is a compilation of patches I have to improve command line help support. The "qemu-option" patches have already been sent earlier, I modified the first to fix an issue reported by Markus. The other patches add support for -object help. A few related patches for QOM, to fix/improve some minor issues. v2: after Eric Blake review - add "qdev-monitor: print help to stdout" - add "cutils: add qemu_pstrcmp" - use consistently "arg=type - desc" help format Marc-André Lureau (12): qdev-monitor: print help to stdout cutils: add qemu_pstrcmp qemu-option: add help fallback to print the list of options qemu-option: improve qemu_opts_print_help() output qom/object: fix iterating properties over a class qom/object: register 'type' property as class property tests/qom-proplist: check duplicate "bv" property registration failed tests/qom-proplist: check properties are not listed multiple times tests/qom-proplist: check class properties iterator vl: handle -object help hostmem: add some properties description vl: list user creatable properties when 'help' is argument include/monitor/monitor.h | 2 ++ include/qemu/cutils.h | 12 +++ backends/hostmem-memfd.c | 9 + backends/hostmem.c | 14 monitor.c | 16 +++-- qdev-monitor.c | 32 ++--- qom/object.c | 9 ++--- qom/object_interfaces.c| 6 ++-- tests/check-qom-proplist.c | 58 --- util/cutils.c | 5 +++ util/qemu-option.c | 71 +++--- vl.c | 53 ++-- 12 files changed, 227 insertions(+), 60 deletions(-) -- 2.19.0.rc1
[Qemu-devel] [PATCH v2 11/12] hostmem: add some properties description
Signed-off-by: Marc-André Lureau
---
backends/hostmem-memfd.c | 9 +
backends/hostmem.c | 14 ++
2 files changed, 23 insertions(+)
diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
index 1e20fe0ba8..789c8c3f87 100644
--- a/backends/hostmem-memfd.c
+++ b/backends/hostmem-memfd.c
@@ -144,14 +144,23 @@ memfd_backend_class_init(ObjectClass *oc, void *data)
memfd_backend_get_hugetlb,
memfd_backend_set_hugetlb,
&error_abort);
+object_class_property_set_description(oc, "hugetlb",
+ "Use huge pages",
+ &error_abort);
object_class_property_add(oc, "hugetlbsize", "int",
memfd_backend_get_hugetlbsize,
memfd_backend_set_hugetlbsize,
NULL, NULL, &error_abort);
+object_class_property_set_description(oc, "hugetlbsize",
+ "Huge pages size (ex: 2M, 1G)",
+ &error_abort);
object_class_property_add_bool(oc, "seal",
memfd_backend_get_seal,
memfd_backend_set_seal,
&error_abort);
+object_class_property_set_description(oc, "seal",
+ "Seal growing & shrinking",
+ &error_abort);
}
static const TypeInfo memfd_backend_info = {
diff --git a/backends/hostmem.c b/backends/hostmem.c
index 4908946cd3..1a89342039 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -397,27 +397,41 @@ host_memory_backend_class_init(ObjectClass *oc, void
*data)
object_class_property_add_bool(oc, "merge",
host_memory_backend_get_merge,
host_memory_backend_set_merge, &error_abort);
+object_class_property_set_description(oc, "merge",
+"Mark memory as mergeable", &error_abort);
object_class_property_add_bool(oc, "dump",
host_memory_backend_get_dump,
host_memory_backend_set_dump, &error_abort);
+object_class_property_set_description(oc, "dump",
+"Set to 'off' to exclude from core dump", &error_abort);
object_class_property_add_bool(oc, "prealloc",
host_memory_backend_get_prealloc,
host_memory_backend_set_prealloc, &error_abort);
+object_class_property_set_description(oc, "prealloc",
+"Preallocate memory", &error_abort);
object_class_property_add(oc, "size", "int",
host_memory_backend_get_size,
host_memory_backend_set_size,
NULL, NULL, &error_abort);
+object_class_property_set_description(oc, "size",
+"Size of the memory region (ex: 500M)", &error_abort);
object_class_property_add(oc, "host-nodes", "int",
host_memory_backend_get_host_nodes,
host_memory_backend_set_host_nodes,
NULL, NULL, &error_abort);
+object_class_property_set_description(oc, "host-nodes",
+"Binds memory to the list of NUMA host nodes", &error_abort);
object_class_property_add_enum(oc, "policy", "HostMemPolicy",
&HostMemPolicy_lookup,
host_memory_backend_get_policy,
host_memory_backend_set_policy, &error_abort);
+object_class_property_set_description(oc, "policy",
+"Set the NUMA policy", &error_abort);
object_class_property_add_bool(oc, "share",
host_memory_backend_get_share, host_memory_backend_set_share,
&error_abort);
+object_class_property_set_description(oc, "share",
+"Mark the memory as private to QEMU or shared", &error_abort);
}
static const TypeInfo host_memory_backend_info = {
--
2.19.0.rc1
[Qemu-devel] [PATCH v2 04/12] qemu-option: improve qemu_opts_print_help() output
Modify qemu_opts_print_help():
- to print expected argument type
- skip description if not available
- sort lines
- prefix with the list name (like qdev, to avoid confusion)
- drop 16-chars alignment, use a '-' as seperator for option name and
description
For ex, "-spice help" output is changed from:
port No description available
tls-port No description available
addr No description available
[...]
gl No description available
rendernode No description available
to:
spice.addr=str
spice.agent-mouse=bool (on/off)
spice.disable-agent-file-xfer=bool (on/off)
[...]
spice.x509-key-password=str
spice.zlib-glz-wan-compression=str
"qemu-img create -f qcow2 -o help", changed from:
size Virtual disk size
compat Compatibility level (0.10 or 1.1)
backing_file File name of a base image
[...]
lazy_refcounts Postpone refcount updates
refcount_bitsWidth of a reference count entry in bits
to:
backing_file=str - File name of a base image
backing_fmt=str - Image format of the base image
cluster_size=size - qcow2 cluster size
[...]
refcount_bits=num - Width of a reference count entry in bits
size=size - Virtual disk size
Signed-off-by: Marc-André Lureau
Reviewed-by: Eric Blake
---
util/qemu-option.c | 38 --
1 file changed, 36 insertions(+), 2 deletions(-)
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 557b6c6626..069de36d2c 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -208,17 +208,51 @@ out:
return result;
}
+static const char *opt_type_to_string(enum QemuOptType type)
+{
+switch (type) {
+case QEMU_OPT_STRING:
+return "str";
+case QEMU_OPT_BOOL:
+return "bool (on/off)";
+case QEMU_OPT_NUMBER:
+return "num";
+case QEMU_OPT_SIZE:
+return "size";
+}
+
+g_assert_not_reached();
+}
+
void qemu_opts_print_help(QemuOptsList *list)
{
QemuOptDesc *desc;
+int i;
+GPtrArray *array = g_ptr_array_new();
assert(list);
desc = list->desc;
while (desc && desc->name) {
-printf("%-16s %s\n", desc->name,
- desc->help ? desc->help : "No description available");
+GString *str = g_string_new(NULL);
+if (list->name) {
+g_string_append_printf(str, "%s.", list->name);
+}
+g_string_append_printf(str, "%s=%s", desc->name,
+ opt_type_to_string(desc->type));
+if (desc->help) {
+g_string_append_printf(str, " - %s", desc->help);
+}
+g_ptr_array_add(array, g_string_free(str, false));
desc++;
}
+
+g_ptr_array_sort(array, (GCompareFunc)qemu_pstrcmp);
+for (i = 0; i < array->len; i++) {
+printf("%s\n", (char *)array->pdata[i]);
+}
+g_ptr_array_set_free_func(array, g_free);
+g_ptr_array_free(array, true);
+
}
/* -- */
--
2.19.0.rc1
[Qemu-devel] [PATCH v2 02/12] cutils: add qemu_pstrcmp
A char** variant of strcmp().
Signed-off-by: Marc-André Lureau
---
include/qemu/cutils.h | 12
util/cutils.c | 5 +
2 files changed, 17 insertions(+)
diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
index 47aaa3b0b9..0941639f36 100644
--- a/include/qemu/cutils.h
+++ b/include/qemu/cutils.h
@@ -169,4 +169,16 @@ bool test_buffer_is_zero_next_accel(void);
int uleb128_encode_small(uint8_t *out, uint32_t n);
int uleb128_decode_small(const uint8_t *in, uint32_t *n);
+/**
+ * qemu_pstrcmp:
+ * @str1: a pointer to a C string
+ * @str2: a pointer to a C string
+ *
+ * Compares *str1 and *str2 with g_strcmp0().
+ *
+ * Returns: an integer less than, equal to, or greater than zero, if
+ * *str1 is <, == or > than *str2 .
+ */
+int qemu_pstrcmp(const char **str1, const char **str2);
+
#endif
diff --git a/util/cutils.c b/util/cutils.c
index 9205e09031..e2abc64278 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -769,3 +769,8 @@ char *size_to_str(uint64_t val)
return g_strdup_printf("%0.3g %sB", (double)val / div, suffixes[i]);
}
+
+int qemu_pstrcmp(const char **str1, const char **str2)
+{
+return g_strcmp0(*str1, *str2);
+}
--
2.19.0.rc1
[Qemu-devel] [PATCH v2 03/12] qemu-option: add help fallback to print the list of options
QDev options accept 'help' (or '?', but that's problematic with shell
globing) in the list of parameters, which is handy to list the
available options.
Unfortunately, this isn't built in QemuOpts. qemu_opts_parse_noisily()
seems to be the common path for command line options, so place a
fallback to print help, listing the available options.
This is quite handy, for example with qemu "-spice help".
Signed-off-by: Marc-André Lureau
Reviewed-by: Eric Blake
---
util/qemu-option.c | 33 ++---
1 file changed, 22 insertions(+), 11 deletions(-)
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 01886efe90..557b6c6626 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -486,7 +486,7 @@ int qemu_opt_unset(QemuOpts *opts, const char *name)
}
static void opt_set(QemuOpts *opts, const char *name, char *value,
-bool prepend, Error **errp)
+bool prepend, bool *invalidp, Error **errp)
{
QemuOpt *opt;
const QemuOptDesc *desc;
@@ -496,6 +496,9 @@ static void opt_set(QemuOpts *opts, const char *name, char
*value,
if (!desc && !opts_accepts_any(opts)) {
g_free(value);
error_setg(errp, QERR_INVALID_PARAMETER, name);
+if (invalidp) {
+*invalidp = true;
+}
return;
}
@@ -519,7 +522,7 @@ static void opt_set(QemuOpts *opts, const char *name, char
*value,
void qemu_opt_set(QemuOpts *opts, const char *name, const char *value,
Error **errp)
{
-opt_set(opts, name, g_strdup(value), false, errp);
+opt_set(opts, name, g_strdup(value), false, NULL, errp);
}
void qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val,
@@ -750,7 +753,8 @@ void qemu_opts_print(QemuOpts *opts, const char *separator)
}
static void opts_do_parse(QemuOpts *opts, const char *params,
- const char *firstname, bool prepend, Error **errp)
+ const char *firstname, bool prepend,
+ bool *invalidp, Error **errp)
{
char *option = NULL;
char *value = NULL;
@@ -785,7 +789,7 @@ static void opts_do_parse(QemuOpts *opts, const char
*params,
}
if (strcmp(option, "id") != 0) {
/* store and parse */
-opt_set(opts, option, value, prepend, &local_err);
+opt_set(opts, option, value, prepend, invalidp, &local_err);
value = NULL;
if (local_err) {
error_propagate(errp, local_err);
@@ -814,11 +818,12 @@ static void opts_do_parse(QemuOpts *opts, const char
*params,
void qemu_opts_do_parse(QemuOpts *opts, const char *params,
const char *firstname, Error **errp)
{
-opts_do_parse(opts, params, firstname, false, errp);
+opts_do_parse(opts, params, firstname, false, NULL, errp);
}
static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
-bool permit_abbrev, bool defaults, Error **errp)
+bool permit_abbrev, bool defaults,
+bool *invalidp, Error **errp)
{
const char *firstname;
char *id = NULL;
@@ -850,7 +855,7 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char
*params,
return NULL;
}
-opts_do_parse(opts, params, firstname, defaults, &local_err);
+opts_do_parse(opts, params, firstname, defaults, invalidp, &local_err);
if (local_err) {
error_propagate(errp, local_err);
qemu_opts_del(opts);
@@ -870,7 +875,7 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char
*params,
QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params,
bool permit_abbrev, Error **errp)
{
-return opts_parse(list, params, permit_abbrev, false, errp);
+return opts_parse(list, params, permit_abbrev, false, NULL, errp);
}
/**
@@ -886,10 +891,16 @@ QemuOpts *qemu_opts_parse_noisily(QemuOptsList *list,
const char *params,
{
Error *err = NULL;
QemuOpts *opts;
+bool invalidp = false;
-opts = opts_parse(list, params, permit_abbrev, false, &err);
+opts = opts_parse(list, params, permit_abbrev, false, &invalidp, &err);
if (err) {
-error_report_err(err);
+if (invalidp && has_help_option(params)) {
+qemu_opts_print_help(list);
+error_free(err);
+} else {
+error_report_err(err);
+}
}
return opts;
}
@@ -899,7 +910,7 @@ void qemu_opts_set_defaults(QemuOptsList *list, const char
*params,
{
QemuOpts *opts;
-opts = opts_parse(list, params, permit_abbrev, true, NULL);
+opts = opts_parse(list, params, permit_abbrev, true, NULL, NULL);
assert(opts);
}
--
2.19.0.rc1
[Qemu-devel] [PATCH v2 05/12] qom/object: fix iterating properties over a class
object_class_property_iter_init() starts from the given class, so the
next class should continue with the parent class.
Signed-off-by: Marc-André Lureau
---
qom/object.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qom/object.c b/qom/object.c
index 75d1d48944..d8666de3f2 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1108,7 +1108,7 @@ void
object_class_property_iter_init(ObjectPropertyIterator *iter,
ObjectClass *klass)
{
g_hash_table_iter_init(&iter->iter, klass->properties);
-iter->nextclass = klass;
+iter->nextclass = object_class_get_parent(klass);
}
ObjectProperty *object_class_property_find(ObjectClass *klass, const char
*name,
--
2.19.0.rc1
[Qemu-devel] [PATCH v2 08/12] tests/qom-proplist: check properties are not listed multiple times
And factor out a common function used by the follow class properties
iterator test.
Signed-off-by: Marc-André Lureau
---
tests/check-qom-proplist.c | 44 +-
1 file changed, 24 insertions(+), 20 deletions(-)
diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
index 0f6d9c1ce3..8e1b9c27f3 100644
--- a/tests/check-qom-proplist.c
+++ b/tests/check-qom-proplist.c
@@ -520,32 +520,19 @@ static void test_dummy_getenum(void)
}
-static void test_dummy_iterator(void)
+static void test_dummy_prop_iterator(ObjectPropertyIterator *iter)
{
-Object *parent = object_get_objects_root();
-DummyObject *dobj = DUMMY_OBJECT(
-object_new_with_props(TYPE_DUMMY,
- parent,
- "dummy0",
- &error_abort,
- "bv", "yes",
- "sv", "Hiss hiss hiss",
- "av", "platypus",
- NULL));
-
-ObjectProperty *prop;
-ObjectPropertyIterator iter;
bool seenbv = false, seensv = false, seenav = false, seentype;
+ObjectProperty *prop;
-object_property_iter_init(&iter, OBJECT(dobj));
-while ((prop = object_property_iter_next(&iter))) {
-if (g_str_equal(prop->name, "bv")) {
+while ((prop = object_property_iter_next(iter))) {
+if (!seenbv && g_str_equal(prop->name, "bv")) {
seenbv = true;
-} else if (g_str_equal(prop->name, "sv")) {
+} else if (!seensv && g_str_equal(prop->name, "sv")) {
seensv = true;
-} else if (g_str_equal(prop->name, "av")) {
+} else if (!seenav && g_str_equal(prop->name, "av")) {
seenav = true;
-} else if (g_str_equal(prop->name, "type")) {
+} else if (!seentype && g_str_equal(prop->name, "type")) {
/* This prop comes from the base Object class */
seentype = true;
} else {
@@ -557,7 +544,24 @@ static void test_dummy_iterator(void)
g_assert(seenav);
g_assert(seensv);
g_assert(seentype);
+}
+
+static void test_dummy_iterator(void)
+{
+Object *parent = object_get_objects_root();
+DummyObject *dobj = DUMMY_OBJECT(
+object_new_with_props(TYPE_DUMMY,
+ parent,
+ "dummy0",
+ &error_abort,
+ "bv", "yes",
+ "sv", "Hiss hiss hiss",
+ "av", "platypus",
+ NULL));
+ObjectPropertyIterator iter;
+object_property_iter_init(&iter, OBJECT(dobj));
+test_dummy_prop_iterator(&iter);
object_unparent(OBJECT(dobj));
}
--
2.19.0.rc1
[Qemu-devel] [PATCH v2 07/12] tests/qom-proplist: check duplicate "bv" property registration failed
"bv" is already a class property.
Signed-off-by: Marc-André Lureau
---
tests/check-qom-proplist.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
index 92898e1520..0f6d9c1ce3 100644
--- a/tests/check-qom-proplist.c
+++ b/tests/check-qom-proplist.c
@@ -125,10 +125,13 @@ static char *dummy_get_sv(Object *obj,
static void dummy_init(Object *obj)
{
+Error *err = NULL;
+
object_property_add_bool(obj, "bv",
dummy_get_bv,
dummy_set_bv,
- NULL);
+ &err);
+error_free_or_abort(&err);
}
--
2.19.0.rc1
[Qemu-devel] [PATCH v2 06/12] qom/object: register 'type' property as class property
Let's save a few byte in each object instance.
(Is this property really used anywhere? Only qom-get could read it)
Signed-off-by: Marc-André Lureau
---
qom/object.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/qom/object.c b/qom/object.c
index d8666de3f2..185d1dd9f8 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -2423,9 +2423,10 @@ void object_class_property_set_description(ObjectClass
*klass,
op->description = g_strdup(description);
}
-static void object_instance_init(Object *obj)
+static void object_class_init(ObjectClass *klass, void *data)
{
-object_property_add_str(obj, "type", qdev_get_type, NULL, NULL);
+object_class_property_add_str(klass, "type", qdev_get_type,
+ NULL, &error_abort);
}
static void register_types(void)
@@ -2439,7 +2440,7 @@ static void register_types(void)
static TypeInfo object_info = {
.name = TYPE_OBJECT,
.instance_size = sizeof(Object),
-.instance_init = object_instance_init,
+.class_init = object_class_init,
.abstract = true,
};
--
2.19.0.rc1
[Qemu-devel] [PATCH v2 09/12] tests/qom-proplist: check class properties iterator
This test failed before "fix iterating properties over a class".
Signed-off-by: Marc-André Lureau
---
tests/check-qom-proplist.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
index 8e1b9c27f3..7ed16b704b 100644
--- a/tests/check-qom-proplist.c
+++ b/tests/check-qom-proplist.c
@@ -565,6 +565,14 @@ static void test_dummy_iterator(void)
object_unparent(OBJECT(dobj));
}
+static void test_dummy_class_iterator(void)
+{
+ObjectPropertyIterator iter;
+ObjectClass *klass = object_class_by_name(TYPE_DUMMY);
+
+object_class_property_iter_init(&iter, klass);
+test_dummy_prop_iterator(&iter);
+}
static void test_dummy_delchild(void)
{
@@ -636,6 +644,7 @@ int main(int argc, char **argv)
g_test_add_func("/qom/proplist/badenum", test_dummy_badenum);
g_test_add_func("/qom/proplist/getenum", test_dummy_getenum);
g_test_add_func("/qom/proplist/iterator", test_dummy_iterator);
+g_test_add_func("/qom/proplist/class_iterator", test_dummy_class_iterator);
g_test_add_func("/qom/proplist/delchild", test_dummy_delchild);
g_test_add_func("/qom/resolve/partial", test_qom_partial_path);
--
2.19.0.rc1
[Qemu-devel] [PATCH v2 10/12] vl: handle -object help
List the user creatable objects.
Signed-off-by: Marc-André Lureau
---
vl.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/vl.c b/vl.c
index 5ba06adf78..71765a2982 100644
--- a/vl.c
+++ b/vl.c
@@ -2731,6 +2731,19 @@ static int machine_set_property(void *opaque,
*/
static bool object_create_initial(const char *type)
{
+if (is_help_option(type)) {
+GSList *l, *list;
+
+printf("List of user creatable objects:\n");
+list = object_class_get_list_sorted(TYPE_USER_CREATABLE, false);
+for (l = list; l != NULL; l = l->next) {
+ObjectClass *oc = OBJECT_CLASS(l->data);
+printf("%s\n", object_class_get_name(oc));
+}
+g_slist_free(list);
+exit(0);
+}
+
if (g_str_equal(type, "rng-egd") ||
g_str_has_prefix(type, "pr-manager-")) {
return false;
--
2.19.0.rc1
[Qemu-devel] [PATCH v2 12/12] vl: list user creatable properties when 'help' is argument
Iterate over the writable class properties, sort and print them out
with the description if available.
Ex: qemu -object memory-backend-file,help
memory-backend-file.align=int
memory-backend-file.discard-data=bool
memory-backend-file.dump=bool - Set to 'off' to exclude from core dump
memory-backend-file.host-nodes=int - Binds memory to the list of NUMA host nodes
memory-backend-file.mem-path=string
memory-backend-file.merge=bool - Mark memory as mergeable
memory-backend-file.pmem=bool
memory-backend-file.policy=HostMemPolicy - Set the NUMA policy
memory-backend-file.prealloc=bool - Preallocate memory
memory-backend-file.share=bool - Mark the memory as private to QEMU or shared
memory-backend-file.size=int - Size of the memory region (ex: 500M)
Signed-off-by: Marc-André Lureau
---
qom/object_interfaces.c | 6 +++---
vl.c| 40 +---
2 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 72b97a8bed..941fd63afd 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -141,14 +141,14 @@ Object *user_creatable_add_opts(QemuOpts *opts, Error
**errp)
int user_creatable_add_opts_foreach(void *opaque, QemuOpts *opts, Error **errp)
{
-bool (*type_predicate)(const char *) = opaque;
+bool (*type_opt_predicate)(const char *, QemuOpts *) = opaque;
Object *obj = NULL;
Error *err = NULL;
const char *type;
type = qemu_opt_get(opts, "qom-type");
-if (type && type_predicate &&
-!type_predicate(type)) {
+if (type && type_opt_predicate &&
+!type_opt_predicate(type, opts)) {
return 0;
}
diff --git a/vl.c b/vl.c
index 71765a2982..880676ecc1 100644
--- a/vl.c
+++ b/vl.c
@@ -2729,8 +2729,10 @@ static int machine_set_property(void *opaque,
* cannot be created here, as it depends on the chardev
* already existing.
*/
-static bool object_create_initial(const char *type)
+static bool object_create_initial(const char *type, QemuOpts *opts)
{
+ObjectClass *klass;
+
if (is_help_option(type)) {
GSList *l, *list;
@@ -2744,6 +2746,38 @@ static bool object_create_initial(const char *type)
exit(0);
}
+klass = object_class_by_name(type);
+if (klass && qemu_opt_has_help_opt(opts)) {
+ObjectPropertyIterator iter;
+ObjectProperty *prop;
+GPtrArray *array = g_ptr_array_new();
+int i;
+
+object_class_property_iter_init(&iter, klass);
+while ((prop = object_property_iter_next(&iter))) {
+GString *str;
+
+if (!prop->set) {
+continue;
+}
+
+str = g_string_new(NULL);
+g_string_append_printf(str, "%s.%s=%s", type,
+ prop->name, prop->type);
+if (prop->description) {
+g_string_append_printf(str, " - %s", prop->description);
+}
+g_ptr_array_add(array, g_string_free(str, false));
+}
+g_ptr_array_sort(array, (GCompareFunc)qemu_pstrcmp);
+for (i = 0; i < array->len; i++) {
+printf("%s\n", (char *)array->pdata[i]);
+}
+g_ptr_array_set_free_func(array, g_free);
+g_ptr_array_free(array, true);
+exit(0);
+}
+
if (g_str_equal(type, "rng-egd") ||
g_str_has_prefix(type, "pr-manager-")) {
return false;
@@ -2790,9 +2824,9 @@ static bool object_create_initial(const char *type)
* The remainder of object creation happens after the
* creation of chardev, fsdev, net clients and device data types.
*/
-static bool object_create_delayed(const char *type)
+static bool object_create_delayed(const char *type, QemuOpts *opts)
{
-return !object_create_initial(type);
+return !object_create_initial(type, opts);
}
--
2.19.0.rc1
Re: [Qemu-devel] backend for blk or fs with guaranteed blocking/synchronous I/O
No. I don't need realtime behavior. Realtime implies determinism, but determinism doesn't implies realtime. Of course, I realize that there are other sources of non-determinism exist, but these are separate stories. Here I just trying to eliminate one of them - asynchronous emulation of I/O inside qemu. Realtime isn't solution here. Firstly, implementing realtime still leaves dependency on host machine (its performance, hardware configuration, etc.) and number of containers running. Yes, it will be deterministic, but results are tied to given host and containers count. Secondly, it's just an overkill for problem being solved. The problem area is bounded by guest and QEMU implementation. Using realtime requires to fight complexities on host also (host kernel must be realtime, system configuration must be tuned, all possible latencies must be carefully traced, etc.). I perfectly understand how complex to design realtime system in generic, and implementing it using linux makes things even more complex. Thirdly, it works only for KVM (and possibly other virtualization hypervisors). It's not my case, since my guest running with TCG and -icount,sleep=off. It seems you got me wrong. I'll try to explain problem in other way. Guest virtual clock must run independent of realtime (host) clock. They might be synchronized only in order to wait for some QEMU/host operation to be completed, i.e. guest time is being frozen by host performance bottlenecks, but it's transparent for guest. This is how works (or, at least, should work) "-icount,sleep=off" in time domain of CPU emulation. But I/O operations are seems to not respect this "policy". When QEMU processes I/O request from guest, it allows virtual time to run freely until backend completes operation and result passed back to guest. And this is what makes guest to "feel" speed/latency of I/O. It's the core of the problem. To explain problem even better I've written a simple script (test_run_multiple_containers.sh), which emulates execution of multiple containers: #!/bin/bash N=$1 for i in $(seq 1 $N); do dd if=/dev/zero of=/tmp/testfile_$i bs=1K count=10 2>&1 | sed -n 's/^.*, \(.*\)$/\1/p' & done wait rm -f /tmp/testfile* Where N is a number of containers running in parallel, and /tmp/testfile_$i is a file located in $i container's rootfs (dedicated mount point, blk device or something else). Running ./test_run_multiple_containers.sh 1 on real machine should output value, which corresponds to maximum write speed. Lets define it as "max_io_throughput". Running this script on real machine with different N values should give ouptuts with roughly identical values like "max_io_throughput / N". What I need is that running this script on guest should always give identical and constant values, not depending on N value, current host load or something else external to guest. (No magic. While running emulation will cause at most "max_io_throughput" load on host (in terms of real time), QEMU will throttle guest virtual clock to be N times slower relative to realtime clock.) Also I forgot to mention that container's rootfs aren't required to be persistent and stay on host during execution of containers. They may be transferred to guest RAM before execution. They're just source images of rootfs. чт, 6 сент. 2018 г. в 21:08, Michael S. Tsirkin : > On Thu, Sep 06, 2018 at 04:24:12PM +0600, Artem Pisarenko wrote: > > Hi all, > > > > I'm developing paravirtualized target linux system which runs multiple > linux > > containers (LXC) inside itself. (For those, who unfamiliar with LXC, > simply > > put, it's an isolated group of userspace processes with their own > rootfs.) Each > > container should be provided access to its rootfs located at host and > execution > > of container should be deterministic. Particularly, it means that > container I/O > > operations must be synchronized within some predefined quantum of guest > > _virtual_ time, i.e. its I/O activity shouldn't be delayed by host > performance > > or activities on host and other containers. In other words, guest should > see > > it's like either infinite throughput and zero latency, or some predefined > > throughput/latency characteristics guaranteed per each rootfs. > > > > While other sources of non-determinism are seem to be eliminated (using > TCG, > > -icount, etc.), asynchronous I/O still introduces it. > > ... > > Just that you should realize that the issues are not limited to QEMU: to > get real time behaviour out of a Linux host you need a real-time kernel > and real-time capable hardware/firmware. I'm not an expert on this at > all, but see e.g. these old presentations: > https://lwn.net/Articles/656807/ > > -- > MST > -- С уважением, Артем Писаренко
Re: [Qemu-devel] [PATCH v2 01/12] qdev-monitor: print help to stdout
On 2018-09-07 09:59, Marc-André Lureau wrote: > qdev_device_help() is used from command line "-device help", or from > HMP "device_add". If used from command line, print help to stdout > (it is only printed on explicit demand). Good idea, it always bugged me that "-device help" behaves differently than "-help"! > Signed-off-by: Marc-André Lureau > --- > include/monitor/monitor.h | 2 ++ > monitor.c | 16 +--- > qdev-monitor.c| 32 +++- > 3 files changed, 34 insertions(+), 16 deletions(-) > > diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h > index 2ef5e04b37..e7667fda35 100644 > --- a/include/monitor/monitor.h > +++ b/include/monitor/monitor.h > @@ -47,4 +47,6 @@ int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd); > void monitor_fdset_dup_fd_remove(int dup_fd); > int monitor_fdset_dup_fd_find(int dup_fd); > > +void out_vprintf(FILE *stream, const char *fmt, va_list ap) GCC_FMT_ATTR(2, > 0); The name is a little bit too generic for my taste ... but I also fail to come up with really good suggestions... maybe "mon_file_vprintf()" or something similar? Thomas
Re: [Qemu-devel] [PATCH v2 02/12] cutils: add qemu_pstrcmp
On 2018-09-07 09:59, Marc-André Lureau wrote: > A char** variant of strcmp(). > > Signed-off-by: Marc-André Lureau > --- > include/qemu/cutils.h | 12 > util/cutils.c | 5 + > 2 files changed, 17 insertions(+) > > diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h > index 47aaa3b0b9..0941639f36 100644 > --- a/include/qemu/cutils.h > +++ b/include/qemu/cutils.h > @@ -169,4 +169,16 @@ bool test_buffer_is_zero_next_accel(void); > int uleb128_encode_small(uint8_t *out, uint32_t n); > int uleb128_decode_small(const uint8_t *in, uint32_t *n); > > +/** > + * qemu_pstrcmp: > + * @str1: a pointer to a C string > + * @str2: a pointer to a C string > + * > + * Compares *str1 and *str2 with g_strcmp0(). > + * > + * Returns: an integer less than, equal to, or greater than zero, if > + * *str1 is <, == or > than *str2 . > + */ Maybe add a comment that str1 or str2 can also be NULL? Reviewed-by: Thomas Huth
Re: [Qemu-devel] [PATCH v2 02/12] cutils: add qemu_pstrcmp
Hi On Fri, Sep 7, 2018 at 12:26 PM Thomas Huth wrote: > > On 2018-09-07 09:59, Marc-André Lureau wrote: > > A char** variant of strcmp(). > > > > Signed-off-by: Marc-André Lureau > > --- > > include/qemu/cutils.h | 12 > > util/cutils.c | 5 + > > 2 files changed, 17 insertions(+) > > > > diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h > > index 47aaa3b0b9..0941639f36 100644 > > --- a/include/qemu/cutils.h > > +++ b/include/qemu/cutils.h > > @@ -169,4 +169,16 @@ bool test_buffer_is_zero_next_accel(void); > > int uleb128_encode_small(uint8_t *out, uint32_t n); > > int uleb128_decode_small(const uint8_t *in, uint32_t *n); > > > > +/** > > + * qemu_pstrcmp: > > + * @str1: a pointer to a C string > > + * @str2: a pointer to a C string > > + * > > + * Compares *str1 and *str2 with g_strcmp0(). > > + * > > + * Returns: an integer less than, equal to, or greater than zero, if > > + * *str1 is <, == or > than *str2 . > > + */ > > Maybe add a comment that str1 or str2 can also be NULL? *str1 or *str2 (not str1 or str2). ok > > Reviewed-by: Thomas Huth > -- Marc-André Lureau
Re: [Qemu-devel] [PATCH v2 02/12] cutils: add qemu_pstrcmp
On 2018-09-07 10:29, Marc-André Lureau wrote: > Hi > > On Fri, Sep 7, 2018 at 12:26 PM Thomas Huth wrote: >> >> On 2018-09-07 09:59, Marc-André Lureau wrote: >>> A char** variant of strcmp(). >>> >>> Signed-off-by: Marc-André Lureau >>> --- >>> include/qemu/cutils.h | 12 >>> util/cutils.c | 5 + >>> 2 files changed, 17 insertions(+) >>> >>> diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h >>> index 47aaa3b0b9..0941639f36 100644 >>> --- a/include/qemu/cutils.h >>> +++ b/include/qemu/cutils.h >>> @@ -169,4 +169,16 @@ bool test_buffer_is_zero_next_accel(void); >>> int uleb128_encode_small(uint8_t *out, uint32_t n); >>> int uleb128_decode_small(const uint8_t *in, uint32_t *n); >>> >>> +/** >>> + * qemu_pstrcmp: >>> + * @str1: a pointer to a C string >>> + * @str2: a pointer to a C string >>> + * >>> + * Compares *str1 and *str2 with g_strcmp0(). >>> + * >>> + * Returns: an integer less than, equal to, or greater than zero, if >>> + * *str1 is <, == or > than *str2 . >>> + */ >> >> Maybe add a comment that str1 or str2 can also be NULL? > > *str1 or *str2 (not str1 or str2). ok Ah, right, that's what I meant. Maybe clarify both conditions in the comment ;-) Thomas
Re: [Qemu-devel] [PATCH v2] hw/ppc: on 40p machine, change default firmware to OpenBIOS
On 07/09/18 02:32, David Gibson wrote: > On Thu, Sep 06, 2018 at 05:38:26AM +0100, Mark Cave-Ayland wrote: >> On 05/09/18 01:13, David Gibson wrote: >> >>> On Tue, Sep 04, 2018 at 09:49:03PM +0200, Hervé Poussineau wrote: OpenBIOS gained 40p support in 5b20e4cacecb62fb2bdc6867c11d44cddd77c4ff Use it, instead of relying on an unmaintained and very limited firmware. Signed-off-by: Hervé Poussineau >>> >>> Uh.. against current ppc-for-3.1, plase. >> >> I was a bit confused as to why this failed to apply since the original >> had been part of a local branch for a while, but just noticed it was >> because of this change to Hervé's original which I had missed: >> >> [dwg: Drop prep from boot-serial test to avoid deprecation warnings] >> >> Included below is the new diff against ppc-for-3.1: David, is this >> enough for you to be able to fix up manually without a v3? > > Well, I could have fixed it up manually from v2 - but I'm pushing that > busy work back on you as a contributor, because I'm having trouble > enough finding dtc maintenance time as it is. Fair comment :) Although I do have a few 40p-related patches waiting on this, I'm keen for this particular patch to come from Hervé, since it's quite a big change and I'm not a PReP maintainer. ATB, Mark.
Re: [Qemu-devel] [RFC PATCH v2 2/7] Add plugin support
Pavel Dovgalyuk writes:
> This patch adds support for dynamically loaded plugins.
> Every plugin is a dynamic library with a set of optional exported
> functions that will be called from QEMU.
>
> Signed-off-by: Pavel Dovgalyuk
> ---
> qemu-options.hx | 10 +
> vl.c |8
There are a couple of trivial conflicts vs master here, simple to fix.
> 7 files changed, 143 insertions(+), 1 deletion(-)
> create mode 100644 include/qemu/plugins.h
> create mode 100644 plugins/include/plugins.h
> create mode 100644 plugins/plugins.c
>
> diff --git a/Makefile.target b/Makefile.target
> index dad2cf8..4cffd96 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -93,6 +93,7 @@ all: $(PROGS) stap
> # cpu emulator library
> obj-y += exec.o
> obj-y += accel/
> +obj-$(CONFIG_PLUGINS) += plugins/plugins.o
> obj-$(CONFIG_TCG) += tcg/tcg.o tcg/tcg-op.o tcg/tcg-op-vec.o
> tcg/tcg-op-gvec.o
> obj-$(CONFIG_TCG) += tcg/tcg-common.o tcg/optimize.o
> obj-$(CONFIG_TCG_INTERPRETER) += tcg/tci.o
> diff --git a/configure b/configure
> index a71bf9b..34e6f00 100755
> --- a/configure
> +++ b/configure
> @@ -373,6 +373,7 @@ EXESUF=""
> DSOSUF=".so"
> LDFLAGS_SHARED="-shared"
> modules="no"
> +plugins="no"
> prefix="/usr/local"
> mandir="\${prefix}/share/man"
> datadir="\${prefix}/share"
> @@ -922,6 +923,12 @@ for opt do
>--disable-modules)
>modules="no"
>;;
> + --enable-plugins)
> + plugins="yes"
> + ;;
> + --disable-plugins)
> + plugins="no"
> + ;;
>--cpu=*)
>;;
>--target-list=*) target_list="$optarg"
> @@ -1567,6 +1574,7 @@ disabled with --disable-FEATURE, default is enabled if
> available:
>guest-agent-msi build guest agent Windows MSI installation package
>pie Position Independent Executables
>modules modules support
> + plugins plugins support
>debug-tcg TCG debugging (default is disabled)
>debug-info debugging information
>sparse sparse checker
> @@ -3392,7 +3400,7 @@ else
> glib_req_ver=2.22
> fi
> glib_modules=gthread-2.0
> -if test "$modules" = yes; then
> +if test "$modules" = yes || test "$plugins" = yes; then
> glib_modules="$glib_modules gmodule-export-2.0"
> fi
>
> @@ -5777,6 +5785,7 @@ if test "$slirp" = "yes" ; then
> echo "smbd $smbd"
> fi
> echo "module support$modules"
> +echo "plugin support$plugins"
> echo "host CPU $cpu"
> echo "host big endian $bigendian"
> echo "target list $target_list"
> @@ -6111,6 +6120,9 @@ if test "$modules" = "yes"; then
>echo "CONFIG_STAMP=_$( (echo $qemu_version; echo $pkgversion; cat $0) |
> $shacmd - | cut -f1 -d\ )" >> $config_host_mak
>echo "CONFIG_MODULES=y" >> $config_host_mak
> fi
> +if test "$plugins" = "yes"; then
> + echo "CONFIG_PLUGINS=y" >> $config_host_mak
> +fi
> if test "$have_x11" = "yes" -a "$need_x11" = "yes"; then
>echo "CONFIG_X11=y" >> $config_host_mak
>echo "X11_CFLAGS=$x11_cflags" >> $config_host_mak
> diff --git a/include/qemu/plugins.h b/include/qemu/plugins.h
> new file mode 100644
> index 000..4464822
> --- /dev/null
> +++ b/include/qemu/plugins.h
> @@ -0,0 +1,8 @@
> +#ifndef PLUGINS_H
> +#define PLUGINS_H
> +
> +void qemu_plugin_parse_cmd_args(const char *optarg);
> +void qemu_plugin_load(const char *filename, const char *args);
> +void qemu_plugins_init(void);
> +
> +#endif /* PLUGINS_H */
> diff --git a/plugins/include/plugins.h b/plugins/include/plugins.h
> new file mode 100644
> index 000..100a786
> --- /dev/null
> +++ b/plugins/include/plugins.h
> @@ -0,0 +1,12 @@
> +#ifndef PLUGINS_INTERFACE_H
> +#define PLUGINS_INTERFACE_H
> +
> +#include
> +
> +/* Plugin interface */
> +
> +bool plugin_init(const char *args);
> +bool plugin_needs_before_insn(uint64_t pc, void *cpu);
> +void plugin_before_insn(uint64_t pc, void *cpu);
> +
> +#endif /* PLUGINS_INTERFACE_H */
> diff --git a/plugins/plugins.c b/plugins/plugins.c
> new file mode 100644
> index 000..eabc931
> --- /dev/null
> +++ b/plugins/plugins.c
> @@ -0,0 +1,91 @@
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "qemu/error-report.h"
> +#include "qemu/plugins.h"
> +#include "qemu/queue.h"
> +#include
> +
> +typedef bool (*PluginInitFunc)(const char *);
> +typedef bool (*PluginNeedsBeforeInsnFunc)(uint64_t, void *);
> +typedef void (*PluginBeforeInsnFunc)(uint64_t, void *);
> +
> +typedef struct QemuPluginInfo {
> +const char *filename;
> +const char *args;
> +GModule *g_module;
> +
> +PluginInitFunc init;
> +PluginNeedsBeforeInsnFunc needs_before_insn;
It seems a bit heavyweight to have a query function here. Is this
dynamic state the plugin might change during execution? If not could we
not better report plugin requirements during initialisation?
> +PluginBeforeInsnFunc before_insn;
> +
> +QLIST_ENTRY(QemuPluginInfo) next;
> +} QemuPluginInfo;
> +
> +static QLIST_HEAD(, QemuPlugin
[Qemu-devel] [PATCH 1/3] Improve xen_disk batching behaviour
When I/O consists of many small requests, performance is improved by
batching them together in a single io_submit() call. When there are
relatively few requests, the extra overhead is not worth it. This
introduces a check to start batching I/O requests via blk_io_plug()/
blk_io_unplug() in an amount proportional to the number which were
already in flight at the time we started reading the ring.
Signed-off-by: Tim Smith
---
hw/block/xen_disk.c | 29 +
1 file changed, 29 insertions(+)
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 36eff94f84..6cb40d66fa 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -101,6 +101,9 @@ struct XenBlkDev {
AioContext *ctx;
};
+/* Threshold of in-flight requests above which we will start using
+ * blk_io_plug()/blk_io_unplug() to batch requests */
+#define IO_PLUG_THRESHOLD 1
/* - */
static void ioreq_reset(struct ioreq *ioreq)
@@ -542,6 +545,8 @@ static void blk_handle_requests(struct XenBlkDev *blkdev)
{
RING_IDX rc, rp;
struct ioreq *ioreq;
+int inflight_atstart = blkdev->requests_inflight;
+int batched = 0;
blkdev->more_work = 0;
@@ -550,6 +555,16 @@ static void blk_handle_requests(struct XenBlkDev *blkdev)
xen_rmb(); /* Ensure we see queued requests up to 'rp'. */
blk_send_response_all(blkdev);
+/* If there was more than IO_PLUG_THRESHOLD ioreqs in flight
+ * when we got here, this is an indication that there the bottleneck
+ * is below us, so it's worth beginning to batch up I/O requests
+ * rather than submitting them immediately. The maximum number
+ * of requests we're willing to batch is the number already in
+ * flight, so it can grow up to max_requests when the bottleneck
+ * is below us */
+if (inflight_atstart > IO_PLUG_THRESHOLD) {
+blk_io_plug(blkdev->blk);
+}
while (rc != rp) {
/* pull request from ring */
if (RING_REQUEST_CONS_OVERFLOW(&blkdev->rings.common, rc)) {
@@ -589,7 +604,21 @@ static void blk_handle_requests(struct XenBlkDev *blkdev)
continue;
}
+if (inflight_atstart > IO_PLUG_THRESHOLD && batched >=
inflight_atstart) {
+blk_io_unplug(blkdev->blk);
+}
ioreq_runio_qemu_aio(ioreq);
+if (inflight_atstart > IO_PLUG_THRESHOLD) {
+if (batched >= inflight_atstart) {
+blk_io_plug(blkdev->blk);
+batched=0;
+} else {
+batched++;
+}
+}
+}
+if (inflight_atstart > IO_PLUG_THRESHOLD) {
+blk_io_unplug(blkdev->blk);
}
if (blkdev->more_work && blkdev->requests_inflight < blkdev->max_requests)
{
[Qemu-devel] [PATCH 3/3] Avoid repeated memory allocation in xen_disk
xen_disk currently allocates memory to hold the data for each ioreq
as that ioreq is used, and frees it afterwards. Because it requires
page-aligned blocks, this interacts poorly with non-page-aligned
allocations and balloons the heap.
Instead, allocate the maximum possible requirement, which is
BLKIF_MAX_SEGMENTS_PER_REQUEST pages (currently 11 pages) when
the ioreq is created, and keep that allocation until it is destroyed.
Since the ioreqs themselves are re-used via a free list, this
should actually improve memory usage.
Signed-off-by: Tim Smith
---
hw/block/xen_disk.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index c11cd21d37..67f894bba5 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -112,7 +112,6 @@ static void ioreq_reset(struct ioreq *ioreq)
memset(&ioreq->req, 0, sizeof(ioreq->req));
ioreq->status = 0;
ioreq->start = 0;
-ioreq->buf = NULL;
ioreq->size = 0;
ioreq->presync = 0;
@@ -137,6 +136,10 @@ static struct ioreq *ioreq_start(struct XenBlkDev *blkdev)
/* allocate new struct */
ioreq = g_malloc0(sizeof(*ioreq));
ioreq->blkdev = blkdev;
+/* We cannot need more pages per ioreq than this, and we do re-use
ioreqs,
+ * so allocate the memory once here, to be freed in blk_free() when the
+ * ioreq is freed. */
+ioreq->buf = qemu_memalign(XC_PAGE_SIZE,
BLKIF_MAX_SEGMENTS_PER_REQUEST * XC_PAGE_SIZE);
blkdev->requests_total++;
qemu_iovec_init(&ioreq->v, 1);
} else {
@@ -313,14 +316,12 @@ static void qemu_aio_complete(void *opaque, int ret)
if (ret == 0) {
ioreq_grant_copy(ioreq);
}
-qemu_vfree(ioreq->buf);
break;
case BLKIF_OP_WRITE:
case BLKIF_OP_FLUSH_DISKCACHE:
if (!ioreq->req.nr_segments) {
break;
}
-qemu_vfree(ioreq->buf);
break;
default:
break;
@@ -392,12 +393,10 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
{
struct XenBlkDev *blkdev = ioreq->blkdev;
-ioreq->buf = qemu_memalign(XC_PAGE_SIZE, ioreq->size);
if (ioreq->req.nr_segments &&
(ioreq->req.operation == BLKIF_OP_WRITE ||
ioreq->req.operation == BLKIF_OP_FLUSH_DISKCACHE) &&
ioreq_grant_copy(ioreq)) {
-qemu_vfree(ioreq->buf);
goto err;
}
@@ -989,6 +988,7 @@ static int blk_free(struct XenDevice *xendev)
ioreq = QLIST_FIRST(&blkdev->freelist);
QLIST_REMOVE(ioreq, list);
qemu_iovec_destroy(&ioreq->v);
+qemu_vfree(ioreq->buf);
g_free(ioreq);
}
[Qemu-devel] [PATCH 2/3] Improve xen_disk response latency
If the I/O ring is full, the guest cannot send any more requests
until some responses are sent. Only sending all available responses
just before checking for new work does not leave much time for the
guest to supply new work, so this will cause stalls if the ring gets
full. Also, not completing reads as soon as possible adds latency
to the guest.
To alleviate that, complete IO requests as soon as they come back.
blk_send_response() already returns a value indicating whether
a notify should be sent, which is all the batching we need.
Signed-off-by: Tim Smith
---
hw/block/xen_disk.c | 43 ---
1 file changed, 12 insertions(+), 31 deletions(-)
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 6cb40d66fa..c11cd21d37 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -83,11 +83,9 @@ struct XenBlkDev {
/* request lists */
QLIST_HEAD(inflight_head, ioreq) inflight;
-QLIST_HEAD(finished_head, ioreq) finished;
QLIST_HEAD(freelist_head, ioreq) freelist;
int requests_total;
int requests_inflight;
-int requests_finished;
unsigned intmax_requests;
gbooleanfeature_discard;
@@ -104,6 +102,9 @@ struct XenBlkDev {
/* Threshold of in-flight requests above which we will start using
* blk_io_plug()/blk_io_unplug() to batch requests */
#define IO_PLUG_THRESHOLD 1
+
+static int blk_send_response(struct ioreq *ioreq);
+
/* - */
static void ioreq_reset(struct ioreq *ioreq)
@@ -155,12 +156,10 @@ static void ioreq_finish(struct ioreq *ioreq)
struct XenBlkDev *blkdev = ioreq->blkdev;
QLIST_REMOVE(ioreq, list);
-QLIST_INSERT_HEAD(&blkdev->finished, ioreq, list);
blkdev->requests_inflight--;
-blkdev->requests_finished++;
}
-static void ioreq_release(struct ioreq *ioreq, bool finish)
+static void ioreq_release(struct ioreq *ioreq)
{
struct XenBlkDev *blkdev = ioreq->blkdev;
@@ -168,11 +167,7 @@ static void ioreq_release(struct ioreq *ioreq, bool finish)
ioreq_reset(ioreq);
ioreq->blkdev = blkdev;
QLIST_INSERT_HEAD(&blkdev->freelist, ioreq, list);
-if (finish) {
-blkdev->requests_finished--;
-} else {
-blkdev->requests_inflight--;
-}
+blkdev->requests_inflight--;
}
/*
@@ -351,6 +346,10 @@ static void qemu_aio_complete(void *opaque, int ret)
default:
break;
}
+if (blk_send_response(ioreq)) {
+xen_pv_send_notify(&blkdev->xendev);
+}
+ioreq_release(ioreq);
qemu_bh_schedule(blkdev->bh);
done:
@@ -455,7 +454,7 @@ err:
return -1;
}
-static int blk_send_response_one(struct ioreq *ioreq)
+static int blk_send_response(struct ioreq *ioreq)
{
struct XenBlkDev *blkdev = ioreq->blkdev;
int send_notify = 0;
@@ -504,22 +503,6 @@ static int blk_send_response_one(struct ioreq *ioreq)
return send_notify;
}
-/* walk finished list, send outstanding responses, free requests */
-static void blk_send_response_all(struct XenBlkDev *blkdev)
-{
-struct ioreq *ioreq;
-int send_notify = 0;
-
-while (!QLIST_EMPTY(&blkdev->finished)) {
-ioreq = QLIST_FIRST(&blkdev->finished);
-send_notify += blk_send_response_one(ioreq);
-ioreq_release(ioreq, true);
-}
-if (send_notify) {
-xen_pv_send_notify(&blkdev->xendev);
-}
-}
-
static int blk_get_request(struct XenBlkDev *blkdev, struct ioreq *ioreq,
RING_IDX rc)
{
switch (blkdev->protocol) {
@@ -554,7 +537,6 @@ static void blk_handle_requests(struct XenBlkDev *blkdev)
rp = blkdev->rings.common.sring->req_prod;
xen_rmb(); /* Ensure we see queued requests up to 'rp'. */
-blk_send_response_all(blkdev);
/* If there was more than IO_PLUG_THRESHOLD ioreqs in flight
* when we got here, this is an indication that there the bottleneck
* is below us, so it's worth beginning to batch up I/O requests
@@ -597,10 +579,10 @@ static void blk_handle_requests(struct XenBlkDev *blkdev)
break;
};
-if (blk_send_response_one(ioreq)) {
+if (blk_send_response(ioreq)) {
xen_pv_send_notify(&blkdev->xendev);
}
-ioreq_release(ioreq, false);
+ioreq_release(ioreq);
continue;
}
@@ -645,7 +627,6 @@ static void blk_alloc(struct XenDevice *xendev)
trace_xen_disk_alloc(xendev->name);
QLIST_INIT(&blkdev->inflight);
-QLIST_INIT(&blkdev->finished);
QLIST_INIT(&blkdev->freelist);
blkdev->iothread = iothread_create(xendev->name, &err);
Re: [Qemu-devel] [PATCH v2 3/4] qga: win32: fix crashes when PCI info cannot be retrived
On Wed, 05 Sep 2018 18:21:07 -0500
Michael Roth wrote:
> Quoting Tomáš Golembiovský (2018-08-07 05:51:37)
> > The guest-get-fsinfo command collects also information about PCI
> > controller where the disk is attached. When this fails for some reasons
> > it tries to return just the partial information. However in certain
> > cases the pointer to the structure was not initialized and was set to
> > NULL. This breaks the serializer and lead to crasehs of the guest agent.
> >
> > Signed-off-by: Tomáš Golembiovský
> > ---
> > qga/commands-win32.c | 27 ++-
> > 1 file changed, 22 insertions(+), 5 deletions(-)
> >
> > diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> > index 36d76c22c0..995f62c2e4 100644
> > --- a/qga/commands-win32.c
> > +++ b/qga/commands-win32.c
> > @@ -642,15 +642,32 @@ static GuestDiskAddressList
> > *build_guest_disk_info(char *guid, Error **errp)
> > g_debug("getting pci-controller info");
> > if (DeviceIoControl(vol_h, IOCTL_SCSI_GET_ADDRESS, NULL, 0,
> > scsi_ad,
> > sizeof(SCSI_ADDRESS), &len, NULL)) {
> > +Error *local_err = NULL;
> > disk->unit = addr.Lun;
> > disk->target = addr.TargetId;
> > disk->bus = addr.PathId;
> > -disk->pci_controller = get_pci_info(name, errp);
> > +g_debug("unit=%lld target=%lld bus=%lld",
> > +disk->unit, disk->target, disk->bus);
> > +disk->pci_controller = get_pci_info(name, &local_err);
> > +
> > +if (local_err) {
> > +slog("failed to get PCI controller info: %s",
> > +error_get_pretty(local_err));
>
> slog() is more for logging/auditing events that a guest administrator might
> be interested in knowing about, like when qga is accessing files, freezing
> filesystems, etc. General qga-side error reporting and debug logging should
> go through the normal g_debug/g_warning/etc interfaces to be captured in
> qga's log file.
ok
>
> We should also moved patch 1 after this so we don't expose a breakage
> prior to the fix.
ok
>
> How often are you seeing failures with the pci info?
On Windows 10 Enterprise every the time. On Windows 8 the original code
fails terribly much sooner.
> And does the
> information for the non-failures look valid to you?
I'll tell you when I see that. :)
> I tried to fix the
> CONFIG_QGA_NTDDSCSI naming screw-up a while back and some values like
> PCI func/bus/etc looked bogus, SPDRP_BUSNUMBER/SPDRP_ADDRESS/SPDRP_BUSNUMBER
> didn't seem to be returning what the current code thinks they are. If that's
> still the case it would be good to fix that before we final re-enable this
> code.
Does that mean the queries for SPDRP_* properties work for you?
The issue is that it fails every time as the request is for a
volume not a disk.
Unfortunately I don't know how to fix that at the moment. See my comment
in the followup version of the series that I will send shortly.
Tomas
>
> > +error_free(local_err);
> > +} else if (disk->pci_controller != NULL) {
> > +g_debug("pci: domain=%lld bus=%lld slot=%lld
> > function=%lld",
> > +disk->pci_controller->domain,
> > +disk->pci_controller->bus,
> > +disk->pci_controller->slot,
> > +disk->pci_controller->function);
> > +}
> > }
> > -/* We do not set error in this case, because we still have enough
> > - * information about volume. */
> > -} else {
> > - disk->pci_controller = NULL;
> > +}
> > +/* We do not set error in case pci_controller is NULL, because we still
> > + * have enough information about volume. */
> > +if (disk->pci_controller == NULL) {
> > +g_debug("no PCI controller info");
> > +disk->pci_controller = g_malloc0(sizeof(GuestPCIAddress));
> > }
> >
> > list = g_malloc0(sizeof(*list));
> > --
> > 2.18.0
> >
>
--
Tomáš Golembiovský
Re: [Qemu-devel] [PATCH v2] qga: ignore non present cpus when handling qmp_guest_get_vcpus()
On Thu, 6 Sep 2018 16:13:52 +0200
Laszlo Ersek wrote:
> On 09/06/18 14:51, Igor Mammedov wrote:
> > If VM has VCPUs plugged sparselly (for example a VM started with
> > 3 VCPUs (cpu0, cpu1 and cpu2) and then cpu1 was hotunplugged so
> > only cpu0 and cpu2 are present), QGA will rise a error
> > error: internal error: unable to execute QEMU agent command
> > 'guest-get-vcpus':
> > open("/sys/devices/system/cpu/cpu1/"): No such file or directory
> > when
> > virsh vcpucount FOO --guest
> > is executed.
> > Fix it by ignoring non present CPUs when fetching CPUs status from sysfs.
> >
> > Signed-off-by: Igor Mammedov
> > ---
> > v2:
> > do not create CPU entry if cpu isn't present
> > (Laszlo Ersek )
> > ---
> > qga/commands-posix.c | 115
> > ++-
> > 1 file changed, 59 insertions(+), 56 deletions(-)
> >
> > diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> > index 37e8a2d..42d30f0 100644
> > --- a/qga/commands-posix.c
> > +++ b/qga/commands-posix.c
> > @@ -2035,61 +2035,56 @@ static long sysconf_exact(int name, const char
> > *name_str, Error **errp)
> > * Written members remain unmodified on error.
> > */
> > static void transfer_vcpu(GuestLogicalProcessor *vcpu, bool sys2vcpu,
> > - Error **errp)
> > + char *dirpath, Error **errp)
> > {
> > -char *dirpath;
> > +int fd;
> > +int res;
> > int dirfd;
> > +static const char fn[] = "online";
> >
> > -dirpath = g_strdup_printf("/sys/devices/system/cpu/cpu%" PRId64 "/",
> > - vcpu->logical_id);
> > dirfd = open(dirpath, O_RDONLY | O_DIRECTORY);
> > if (dirfd == -1) {
> > error_setg_errno(errp, errno, "open(\"%s\")", dirpath);
> > -} else {
> > -static const char fn[] = "online";
> > -int fd;
> > -int res;
> > -
> > -fd = openat(dirfd, fn, sys2vcpu ? O_RDONLY : O_RDWR);
> > -if (fd == -1) {
> > -if (errno != ENOENT) {
> > -error_setg_errno(errp, errno, "open(\"%s/%s\")", dirpath,
> > fn);
> > -} else if (sys2vcpu) {
> > -vcpu->online = true;
> > -vcpu->can_offline = false;
> > -} else if (!vcpu->online) {
> > -error_setg(errp, "logical processor #%" PRId64 " can't be "
> > - "offlined", vcpu->logical_id);
> > -} /* otherwise pretend successful re-onlining */
> > -} else {
> > -unsigned char status;
> > -
> > -res = pread(fd, &status, 1, 0);
> > -if (res == -1) {
> > -error_setg_errno(errp, errno, "pread(\"%s/%s\")", dirpath,
> > fn);
> > -} else if (res == 0) {
> > -error_setg(errp, "pread(\"%s/%s\"): unexpected EOF",
> > dirpath,
> > - fn);
> > -} else if (sys2vcpu) {
> > -vcpu->online = (status != '0');
> > -vcpu->can_offline = true;
> > -} else if (vcpu->online != (status != '0')) {
> > -status = '0' + vcpu->online;
> > -if (pwrite(fd, &status, 1, 0) == -1) {
> > -error_setg_errno(errp, errno, "pwrite(\"%s/%s\")",
> > dirpath,
> > - fn);
> > -}
> > -} /* otherwise pretend successful re-(on|off)-lining */
> > +return;
> > +}
> >
> > -res = close(fd);
> > -g_assert(res == 0);
> > -}
> > +fd = openat(dirfd, fn, sys2vcpu ? O_RDONLY : O_RDWR);
> > +if (fd == -1) {
> > +if (errno != ENOENT) {
> > +error_setg_errno(errp, errno, "open(\"%s/%s\")", dirpath, fn);
> > +} else if (sys2vcpu) {
> > +vcpu->online = true;
> > +vcpu->can_offline = false;
> > +} else if (!vcpu->online) {
> > +error_setg(errp, "logical processor #%" PRId64 " can't be "
> > + "offlined", vcpu->logical_id);
> > +} /* otherwise pretend successful re-onlining */
> > +} else {
> > +unsigned char status;
> > +
> > +res = pread(fd, &status, 1, 0);
> > +if (res == -1) {
> > +error_setg_errno(errp, errno, "pread(\"%s/%s\")", dirpath, fn);
> > +} else if (res == 0) {
> > +error_setg(errp, "pread(\"%s/%s\"): unexpected EOF", dirpath,
> > + fn);
> > +} else if (sys2vcpu) {
> > +vcpu->online = (status != '0');
> > +vcpu->can_offline = true;
> > +} else if (vcpu->online != (status != '0')) {
> > +status = '0' + vcpu->online;
> > +if (pwrite(fd, &status, 1, 0) == -1) {
> > +error_setg_errno(errp, errno, "pwrite(\"%s/%s\")", dirpath,
> > + fn);
> > +}
> > +} /* otherwise pretend successful
Re: [Qemu-devel] [PATCH for-3.1 v10 04/31] block: Add BDS.auto_backing_file
On 2018-09-05 16:22, Alberto Garcia wrote: > On Thu 09 Aug 2018 11:35:01 PM CEST, Max Reitz wrote: >> If the backing file is overridden, this most probably does change the >> guest-visible data of a BDS. Therefore, we will need to consider this >> in bdrv_refresh_filename(). >> >> To see whether it has been overridden, we might want to compare >> bs->backing_file and bs->backing->bs->filename. However, >> bs->backing_file is changed by bdrv_set_backing_hd() (which is just used >> to change the backing child at runtime, without modifying the image >> header), so bs->backing_file most of the time simply contains a copy of >> bs->backing->bs->filename anyway, so it is useless for such a >> comparison. > > What's the point of bs->backing_file then? In what cases is it different > from backing->bs->filename? Good question! Yes, why? One thing is when you have detached the backing file, bs->backing_file will stay the same, even though backing is now NULL. But that is pretty much useless, I couldn't find any part in the block layer which relies on this. The other is... Fun. When you create the BDS, it is different, because the format driver will just put the filename it got from the image header there. So imagine you have this configuration: $ mkdir foo $ qemu-img create -f qcow2 foo/base.qcow2 64M $ qemu-img create -f qcow2 -b base.qcow2 foo/top.qcow2 Now when you open foo/top.qcow2, backing_file will contain "base.qcow2" at first. This is what bdrv_open_backing_file() expects (because that's what bdrv_get_full_backing_filename() is for). But when the backing BDS is attached, bdrv_set_backing_hd() (through bdrv_backing_attach()) will update backing_file to "foo/base.qcow2" (or probably the absolute equivalent, I can't remember). That seems really pointless to me, and after more investigation, I couldn't find any reason for why bdrv_backing_attach() should update backing_file. Especially considering this behavior breaks things, like bdrv_find_backing_image(), which again would expect "base.qcow2" (i.e. a filename relative to the overlay). So in short, functions in the block layer that query backing_file generally expect it to be what's in the image header; while on the other hand functions that set backing_file ignore that expectation half of the time. So consequentially, there is a "block: Leave BDS.backing_file constant" patch in my "block: Deal with filters" series. It makes backing_file always report what the image header says. Now you will probably ask: OK, nice, but why do you still need auto_backing_file then? Wouldn't that change be enough to just use backing_file? The issue here is that the image header may contain a filename that bdrv_refresh_filename() will transform. Say your image header says "nbd:localhost:10809" for the backing file. So that's what backing_file will report, too. bdrv_refresh_filename() however will make backing->bs->filename report "nbd://localhost:10809". So if you'd compare backing_file against backing->bs->filename, you'd see a difference and you'd have to assume the backing file was overridden, when in fact it wasn't. That's what we still need auto_backing_file for: It does not always reflect the image header, instead it is supposed to reflect the bdrv_refresh_filename() result for the BDS that is opened when you open the backing filename given in the image header. (At least that would be desirable; we cannot always conform to that specification, but we'll try at least.) Max signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH for-3.1 v10 31/31] iotests: Test json:{} filenames of internal BDSs
On 2018-09-05 16:44, Alberto Garcia wrote: > On Thu 09 Aug 2018 11:35:28 PM CEST, Max Reitz wrote: >> +vm.shutdown() >> + >> +#assert top_name[:5] == 'json:' >> +#top_options = json.loads(top_name[5:]) >> + >> +#if filter_node_name: >> +## This should be present and set >> +#assert top_options['backing']['driver'] == 'commit_top' >> +## And the mid image is commit_top's backing image >> +#mid_options = top_options['backing']['backing'] >> +#else: >> +## The mid image should appear as the immediate backing BDS >> +## of top >> +#mid_options = top_options['backing'] >> + >> +#assert mid_options['driver'] == iotests.imgfmt >> +#assert mid_options['file']['filename'] == mid_img_path > > What's with all these commented lines? I don't see how you are checking > the filename in this test, it's also not in the output file. Indeed, what is with them? I have no idea. They shouldn't be commented out. Sorry. Max signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v4 05/15] block/mirror: don't install backing chain on abort
On 2018-09-05 17:39, John Snow wrote:
>
>
> On 09/05/2018 06:40 AM, Max Reitz wrote:
>> On 2018-09-04 19:09, John Snow wrote:
>>> In cases where we abort the block/mirror job, there's no point in
>>> installing the new backing chain before we finish aborting.
>>>
>>> Move this to the "success" portion of mirror_exit.
>>
>> Sounds a bit weird now that you don't do any moving.
>>
>
> I ought to proofread my commit messages when I make changes...
>
>>> Signed-off-by: John Snow
>>> ---
>>> block/mirror.c | 7 ++-
>>> 1 file changed, 2 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/block/mirror.c b/block/mirror.c
>>> index cba555b4ef..3365bcfdfb 100644
>>> --- a/block/mirror.c
>>> +++ b/block/mirror.c
>>> @@ -642,7 +642,7 @@ static void mirror_exit(Job *job)
>>> * required before it could become a backing file of target_bs. */
>>> bdrv_child_try_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL,
>>> &error_abort);
>>> -if (s->backing_mode == MIRROR_SOURCE_BACKING_CHAIN) {
>>> +if (ret == 0 && s->backing_mode == MIRROR_SOURCE_BACKING_CHAIN) {
>>> BlockDriverState *backing = s->is_none_mode ? src : s->base;
>>> if (backing_bs(target_bs) != backing) {
>>> bdrv_set_backing_hd(target_bs, backing, &local_err);
>>> @@ -659,10 +659,7 @@ static void mirror_exit(Job *job)
>>> }
>>>
>>> if (s->should_complete && ret == 0) {
>>> -BlockDriverState *to_replace = src;
>>> -if (s->to_replace) {
>>> -to_replace = s->to_replace;
>>> -}
>>> +BlockDriverState *to_replace = s->to_replace ? s->to_replace : src;
>>>
>>> if (bdrv_get_flags(target_bs) != bdrv_get_flags(to_replace)) {
>>> bdrv_reopen(target_bs, bdrv_get_flags(to_replace), NULL);
>>
>> And this hunk now looks out of place. Sure, it makes sense, but why is
>> it in this patch now? :-)
>>
>> (Moving it into the next patch would make more sense, I think.)
>>
>> I'd like to give an R-b anyway, but I know that I shouldn't, so I won't.
>>
>> Max
>>
>
> I have to admit that my appetite for patch purity is just... low. I know
> it's something we care a lot in the QEMU project, but after a number of
> years I'm just not overwhelmed to care about it in any significant capacity.
>
> I suppose the main argument for this practice is ease of backporting, yes?
I suppose. And ease of bisecting. And ease of reviewing.
Max
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH v3 3/5] qga: win32: add debugging information
The windows code generaly lacks debug information (compared to posix
code). This patch adds some related to HW info in guest-get-fsinfo
command.
Signed-off-by: Tomáš Golembiovský
---
qga/commands-win32.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 9c959122d9..e16c58275e 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -89,6 +89,12 @@ static OpenFlags guest_file_open_modes[] = {
{"a+b", FILE_GENERIC_APPEND|GENERIC_READ, OPEN_ALWAYS }
};
+#define g_debug_err(msg) do { \
+char *suffix = g_win32_error_message(GetLastError()); \
+g_debug("%s: %s", (msg), suffix); \
+g_free(suffix); \
+} while(0)
+
static OpenFlags *find_open_flag(const char *mode_str)
{
int mode;
@@ -498,6 +504,7 @@ static GuestPCIAddress *get_pci_info(char *guid, Error
**errp)
goto out;
}
+g_debug("enumerating devices");
dev_info_data.cbSize = sizeof(SP_DEVINFO_DATA);
for (i = 0; SetupDiEnumDeviceInfo(dev_info, i, &dev_info_data); i++) {
DWORD addr, bus, slot, func, dev, data, size2;
@@ -522,6 +529,7 @@ static GuestPCIAddress *get_pci_info(char *guid, Error
**errp)
if (g_strcmp0(buffer, dev_name)) {
continue;
}
+g_debug("found device %s", dev_name);
/* There is no need to allocate buffer in the next functions. The size
* is known and ULONG according to
@@ -530,6 +538,7 @@ static GuestPCIAddress *get_pci_info(char *guid, Error
**errp)
*/
if (!SetupDiGetDeviceRegistryProperty(dev_info, &dev_info_data,
SPDRP_BUSNUMBER, &data, (PBYTE)&bus, size, NULL)) {
+g_debug_err("failed to get bus");
break;
}
@@ -537,6 +546,7 @@ static GuestPCIAddress *get_pci_info(char *guid, Error
**errp)
* transformed into device function and number */
if (!SetupDiGetDeviceRegistryProperty(dev_info, &dev_info_data,
SPDRP_ADDRESS, &data, (PBYTE)&addr, size, NULL)) {
+g_debug_err("failed to get address");
break;
}
@@ -544,6 +554,7 @@ static GuestPCIAddress *get_pci_info(char *guid, Error
**errp)
* This number is typically a user-perceived slot number. */
if (!SetupDiGetDeviceRegistryProperty(dev_info, &dev_info_data,
SPDRP_UI_NUMBER, &data, (PBYTE)&slot, size, NULL)) {
+g_debug_err("failed to get slot");
break;
}
@@ -608,6 +619,7 @@ static GuestDiskAddressList *build_guest_disk_info(char
*guid, Error **errp)
scsi_ad = &addr;
char *name = g_strndup(guid, strlen(guid)-1);
+g_debug("getting disk info for: %s", name);
vol_h = CreateFile(name, 0, FILE_SHARE_READ, NULL, OPEN_EXISTING,
0, NULL);
if (vol_h == INVALID_HANDLE_VALUE) {
@@ -615,6 +627,7 @@ static GuestDiskAddressList *build_guest_disk_info(char
*guid, Error **errp)
goto out_free;
}
+g_debug("getting bus type");
bus = get_disk_bus_type(vol_h, errp);
if (bus < 0) {
goto out_close;
@@ -622,6 +635,7 @@ static GuestDiskAddressList *build_guest_disk_info(char
*guid, Error **errp)
disk = g_malloc0(sizeof(*disk));
disk->bus_type = find_bus_type(bus);
+g_debug("bus type %d", disk->bus_type);
if (bus == BusTypeScsi || bus == BusTypeAta || bus == BusTypeRAID
#if (_WIN32_WINNT >= 0x0600)
/* This bus type is not supported before Windows Server 2003 SP1 */
@@ -631,6 +645,7 @@ static GuestDiskAddressList *build_guest_disk_info(char
*guid, Error **errp)
/* We are able to use the same ioctls for different bus types
* according to Microsoft docs
* https://technet.microsoft.com/en-us/library/ee851589(v=ws.10).aspx
*/
+g_debug("getting pci-controller info");
if (DeviceIoControl(vol_h, IOCTL_SCSI_GET_ADDRESS, NULL, 0, scsi_ad,
sizeof(SCSI_ADDRESS), &len, NULL)) {
Error *local_err = NULL;
--
2.18.0
[Qemu-devel] [PATCH v3 2/5] build: rename CONFIG_QGA_NTDDDISK to CONFIG_QGA_NTDDSCSI
There was inconsistency between commits: 50cbebb9a3 configure: add configure check for ntdddisk.h a3ef3b2272 qga: added bus type and disk location path The first commit added #define CONFIG_QGA_NTDDDISK but the second commit expected the name to be CONFIG_QGA_NTDDSCSI. As a result the code in second patch was never used. Renaming the option to CONFIG_QGA_NTDDSCSI to match the name of header file that is being checked for. Signed-off-by: Tomáš Golembiovský --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index 58862d2ae8..0f168607e8 100755 --- a/configure +++ b/configure @@ -6201,7 +6201,7 @@ if test "$mingw32" = "yes" ; then echo "WIN_SDK=\"$win_sdk\"" >> $config_host_mak fi if test "$guest_agent_ntddscsi" = "yes" ; then -echo "CONFIG_QGA_NTDDDISK=y" >> $config_host_mak +echo "CONFIG_QGA_NTDDSCSI=y" >> $config_host_mak fi if test "$guest_agent_msi" = "yes"; then echo "QEMU_GA_MSI_ENABLED=yes" >> $config_host_mak -- 2.18.0
[Qemu-devel] [PATCH v3 1/5] qga: win32: fix crashes when PCI info cannot be retrived
The guest-get-fsinfo command collects also information about PCI
controller where the disk is attached. When this fails for some reasons
it tries to return just the partial information. However in certain
cases the pointer to the structure was not initialized and was set to
NULL. This breaks the serializer and leads to a crash of the guest agent.
Signed-off-by: Tomáš Golembiovský
---
qga/commands-win32.c | 27 ++-
1 file changed, 22 insertions(+), 5 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 98d9735389..9c959122d9 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -633,15 +633,32 @@ static GuestDiskAddressList *build_guest_disk_info(char
*guid, Error **errp)
* https://technet.microsoft.com/en-us/library/ee851589(v=ws.10).aspx
*/
if (DeviceIoControl(vol_h, IOCTL_SCSI_GET_ADDRESS, NULL, 0, scsi_ad,
sizeof(SCSI_ADDRESS), &len, NULL)) {
+Error *local_err = NULL;
disk->unit = addr.Lun;
disk->target = addr.TargetId;
disk->bus = addr.PathId;
-disk->pci_controller = get_pci_info(name, errp);
+g_debug("unit=%lld target=%lld bus=%lld",
+disk->unit, disk->target, disk->bus);
+disk->pci_controller = get_pci_info(name, &local_err);
+
+if (local_err) {
+g_debug("failed to get PCI controller info: %s",
+error_get_pretty(local_err));
+error_free(local_err);
+} else if (disk->pci_controller != NULL) {
+g_debug("pci: domain=%lld bus=%lld slot=%lld function=%lld",
+disk->pci_controller->domain,
+disk->pci_controller->bus,
+disk->pci_controller->slot,
+disk->pci_controller->function);
+}
}
-/* We do not set error in this case, because we still have enough
- * information about volume. */
-} else {
- disk->pci_controller = NULL;
+}
+/* We do not set error in case pci_controller is NULL, because we still
+ * have enough information about volume. */
+if (disk->pci_controller == NULL) {
+g_debug("no PCI controller info");
+disk->pci_controller = g_malloc0(sizeof(GuestPCIAddress));
}
list = g_malloc0(sizeof(*list));
--
2.18.0
[Qemu-devel] [PATCH v3 0/5] qga: report serial number and disk node
Note that PCI controller reporting on Windows was and still is broken. Unfortunately I don't know how to fix it at the momemnt. See commit message and code comment. If anyone has environment where the original code works let me know. CCing author of the code In case I missed something obvious. v3: - fix typos - add configure test for libudev - change order of patches fixing PCI controller info and build fix to avoid exposing broken code - split reporting of serial number and device node into two separate patches v2: - fix checkpatch error Patches 1 and 3 are bug-fixes of existing code Patch 2 is optional and contains debug messages -- the windows QGA seriously lacks debug messages... Patch 4 adds reporting of disk serial number and device node of the disk. Tomáš Golembiovský (5): qga: win32: fix crashes when PCI info cannot be retrived build: rename CONFIG_QGA_NTDDDISK to CONFIG_QGA_NTDDSCSI qga: win32: add debugging information qga: report disk serial number qga: return disk device in guest-get-fsinfo configure| 24 - qga/Makefile.objs| 1 + qga/commands-posix.c | 27 + qga/commands-win32.c | 248 --- qga/qapi-schema.json | 5 +- 5 files changed, 264 insertions(+), 41 deletions(-) -- 2.18.0
[Qemu-devel] [PATCH v3 5/5] qga: return disk device in guest-get-fsinfo
Report device node of the disk. It is implemented for Linux (needs udev)
and Windows. The node is reported e.g. as "/dev/sda2" on Linux and as
"\\?\PhysicalDriveX" on Windows.
As part of this effort the Windows code was changed to probe disk
extents and return list of all disks. Originally only first disk of
composite volume was returned.
Note that the patch changes get_pci_info() from one state of brokenness
into a different state of brokenness. In other words it still does not do
what it's supposed to do (see comment in code). If anyone knows how to
fix it, please step in.
Signed-off-by: Tomáš Golembiovský
---
qga/commands-posix.c | 7 +-
qga/commands-win32.c | 163 +++
qga/qapi-schema.json | 3 +-
3 files changed, 142 insertions(+), 31 deletions(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 37fedd123b..d0d6ba49cb 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -947,7 +947,12 @@ static void build_guest_fsinfo_for_real_device(char const
*syspath,
if (udev == NULL || udevice == NULL) {
g_debug("failed to query udev");
} else {
-const char *serial;
+const char *devnode, *serial;
+devnode = udev_device_get_devnode(udevice);
+if (devnode != NULL) {
+disk->dev = g_strdup(devnode);
+disk->has_dev = true;
+}
serial = udev_device_get_property_value(udevice, "ID_SERIAL");
if (serial != NULL && *serial != 0) {
disk->serial = g_strdup(serial);
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index fa186154a8..2cde6bd0af 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -477,9 +477,26 @@ static GuestDiskBusType find_bus_type(STORAGE_BUS_TYPE bus)
return win2qemu[(int)bus];
}
+/* XXX: The following function is BROKEN!
+ *
+ * It does not work and probably has never worked. When we query for list of
+ * disks we get cryptic names like "\Device\001d" instead of
+ * "\PhysicalDriveX" or "\HarddiskX". Whether the names can be translated one
+ * way or the other for comparison is an open question.
+ *
+ * When we query volume names (the original version) we are able to match those
+ * but then the property queries report error "Invalid function". (duh!)
+ */
+
+/*
DEFINE_GUID(GUID_DEVINTERFACE_VOLUME,
0x53f5630dL, 0xb6bf, 0x11d0, 0x94, 0xf2,
0x00, 0xa0, 0xc9, 0x1e, 0xfb, 0x8b);
+*/
+DEFINE_GUID(GUID_DEVINTERFACE_DISK,
+0x53f56307L, 0xb6bf, 0x11d0, 0x94, 0xf2,
+0x00, 0xa0, 0xc9, 0x1e, 0xfb, 0x8b);
+
static GuestPCIAddress *get_pci_info(char *guid, Error **errp)
{
@@ -497,7 +514,7 @@ static GuestPCIAddress *get_pci_info(char *guid, Error
**errp)
goto out;
}
-dev_info = SetupDiGetClassDevs(&GUID_DEVINTERFACE_VOLUME, 0, 0,
+dev_info = SetupDiGetClassDevs(&GUID_DEVINTERFACE_DISK, 0, 0,
DIGCF_PRESENT | DIGCF_DEVICEINTERFACE);
if (dev_info == INVALID_HANDLE_VALUE) {
error_setg_win32(errp, GetLastError(), "failed to get devices tree");
@@ -632,31 +649,24 @@ out_free:
return;
}
-/* VSS provider works with volumes, thus there is no difference if
- * the volume consist of spanned disks. Info about the first disk in the
- * volume is returned for the spanned disk group (LVM) */
-static GuestDiskAddressList *build_guest_disk_info(char *guid, Error **errp)
+static void get_single_disk_info(GuestDiskAddress *disk, Error **errp)
{
-GuestDiskAddressList *list = NULL;
-GuestDiskAddress *disk;
SCSI_ADDRESS addr, *scsi_ad;
DWORD len;
-HANDLE vol_h;
+HANDLE disk_h;
Error *local_err = NULL;
scsi_ad = &addr;
-char *name = g_strndup(guid, strlen(guid)-1);
-g_debug("getting disk info for: %s", name);
-vol_h = CreateFile(name, 0, FILE_SHARE_READ, NULL, OPEN_EXISTING,
+g_debug("getting disk info for: %s", disk->dev);
+disk_h = CreateFile(disk->dev, 0, FILE_SHARE_READ, NULL, OPEN_EXISTING,
0, NULL);
-if (vol_h == INVALID_HANDLE_VALUE) {
-error_setg_win32(errp, GetLastError(), "failed to open volume");
-goto err;
+if (disk_h == INVALID_HANDLE_VALUE) {
+error_setg_win32(errp, GetLastError(), "failed to open disk");
+return;
}
-disk = g_malloc0(sizeof(*disk));
-get_disk_properties(vol_h, disk, &local_err);
+get_disk_properties(disk_h, disk, &local_err);
if (local_err) {
error_propagate(errp, local_err);
goto err_close;
@@ -674,20 +684,20 @@ static GuestDiskAddressList *build_guest_disk_info(char
*guid, Error **errp)
* according to Microsoft docs
* https://technet.microsoft.com/en-us/library/ee851589(v=ws.10).aspx
*/
g_debug("getting pci-controller info");
-if (DeviceIoControl(vol_h, IOCTL_SCSI_GET_ADDRESS, NULL, 0, scsi_ad,
+if (DeviceIoControl(disk_h, IOCTL_SCSI_GET_ADDRESS, NULL, 0, scsi_ad,
[Qemu-devel] [PATCH v3 4/5] qga: report disk serial number
The feature is implemented for Windows and Linux. Reporting of serial
number on Linux depends on libudev.
Example from Linux:
{
"name": "dm-2",
"mountpoint": "/",
...
"disk": [
{
"serial": "SAMSUNG_MZ7LN512HCHP-000L1_S1ZKNXAG822493",
...
}
],
}
Signed-off-by: Tomáš Golembiovský
---
configure| 22 ++
qga/Makefile.objs| 1 +
qga/commands-posix.c | 22 ++
qga/commands-win32.c | 71
qga/qapi-schema.json | 4 ++-
5 files changed, 100 insertions(+), 20 deletions(-)
diff --git a/configure b/configure
index 0f168607e8..ac24cb3975 100755
--- a/configure
+++ b/configure
@@ -477,6 +477,7 @@ libxml2=""
docker="no"
debug_mutex="no"
libpmem=""
+libudev="no"
# cross compilers defaults, can be overridden with --cross-cc-ARCH
cross_cc_aarch64="aarch64-linux-gnu-gcc"
@@ -873,6 +874,7 @@ Linux)
vhost_vsock="yes"
QEMU_INCLUDES="-I\$(SRC_PATH)/linux-headers -I$(pwd)/linux-headers
$QEMU_INCLUDES"
supported_os="yes"
+ libudev="yes"
;;
esac
@@ -5676,6 +5678,20 @@ if test "$libnfs" != "no" ; then
fi
fi
+##
+# Do we have libudev
+if test "$libudev" != "no" ; then
+ if $pkg_config libudev; then
+libudev="yes"
+libudev_libs=$($pkg_config --libs libudev)
+ else
+if test "$libudev" = "yes" ; then
+ feature_not_found "libudev" "Install systemd development files"
+fi
+libudev="no"
+ fi
+fi
+
# Now we've finished running tests it's OK to add -Werror to the compiler flags
if test "$werror" = "yes"; then
QEMU_CFLAGS="-Werror $QEMU_CFLAGS"
@@ -6100,6 +6116,7 @@ echo "VxHS block device $vxhs"
echo "capstone $capstone"
echo "docker$docker"
echo "libpmem support $libpmem"
+echo "libudev $libudev"
if test "$sdl_too_old" = "yes"; then
echo "-> Your SDL version is too old - please upgrade to have SDL support"
@@ -6944,6 +6961,11 @@ if test "$docker" != "no"; then
echo "HAVE_USER_DOCKER=y" >> $config_host_mak
fi
+if test "$libudev" != "no"; then
+echo "CONFIG_LIBUDEV=y" >> $config_host_mak
+echo "LIBUDEV_LIBS=$libudev_libs" >> $config_host_mak
+fi
+
# use included Linux headers
if test "$linux" = "yes" ; then
mkdir -p linux-headers
diff --git a/qga/Makefile.objs b/qga/Makefile.objs
index ed08c5917c..80e6bb3c2e 100644
--- a/qga/Makefile.objs
+++ b/qga/Makefile.objs
@@ -1,3 +1,4 @@
+commands-posix.o-libs := $(LIBUDEV_LIBS)
qga-obj-y = commands.o guest-agent-command-state.o main.o
qga-obj-$(CONFIG_POSIX) += commands-posix.o channel-posix.o
qga-obj-$(CONFIG_WIN32) += commands-win32.o channel-win32.o service-win32.o
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 37e8a2d791..37fedd123b 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -47,6 +47,7 @@ extern char **environ;
#include
#include
#include
+#include
#ifdef FIFREEZE
#define CONFIG_FSFREEZE
@@ -872,6 +873,10 @@ static void build_guest_fsinfo_for_real_device(char const
*syspath,
GuestDiskAddressList *list = NULL;
bool has_ata = false, has_host = false, has_tgt = false;
char *p, *q, *driver = NULL;
+#ifdef CONFIG_LIBUDEV
+struct udev *udev = NULL;
+struct udev_device *udevice = NULL;
+#endif
p = strstr(syspath, "/devices/pci");
if (!p || sscanf(p + 12, "%*x:%*x/%x:%x:%x.%x%n",
@@ -936,6 +941,21 @@ static void build_guest_fsinfo_for_real_device(char const
*syspath,
list = g_malloc0(sizeof(*list));
list->value = disk;
+#ifdef CONFIG_LIBUDEV
+udev = udev_new();
+udevice = udev_device_new_from_syspath(udev, syspath);
+if (udev == NULL || udevice == NULL) {
+g_debug("failed to query udev");
+} else {
+const char *serial;
+serial = udev_device_get_property_value(udevice, "ID_SERIAL");
+if (serial != NULL && *serial != 0) {
+disk->serial = g_strdup(serial);
+disk->has_serial = true;
+}
+}
+#endif
+
if (strcmp(driver, "ata_piix") == 0) {
/* a host per ide bus, target*:0::0 */
if (!has_host || !has_tgt) {
@@ -1003,6 +1023,8 @@ cleanup:
qapi_free_GuestDiskAddressList(list);
}
g_free(driver);
+udev_unref(udev);
+udev_device_unref(udevice);
}
static void build_guest_fsinfo_for_device(char const *devpath,
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index e16c58275e..fa186154a8 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -583,25 +583,53 @@ out:
return pci;
}
-static int get_disk_bus_type(HANDLE vol_h, Error **errp)
+static void get_disk_properties(HANDLE vol_h, GuestDiskAddress *disk,
+Error **errp)
{
STORAGE_PROPERTY_QUERY query;
STORAGE_DEVICE_DESCRIPTOR *dev_desc, buf;
DWORD received;
+ULONG size = sizeof(buf);
dev_desc = &buf;
-dev_desc->Size = sizeof(buf);
query.PropertyId = St
Re: [Qemu-devel] [PATCH v2] qga: ignore non present cpus when handling qmp_guest_get_vcpus()
On 09/07/18 13:30, Igor Mammedov wrote:
> On Thu, 6 Sep 2018 16:13:52 +0200
> Laszlo Ersek wrote:
>
>> On 09/06/18 14:51, Igor Mammedov wrote:
>>> If VM has VCPUs plugged sparselly (for example a VM started with
>>> 3 VCPUs (cpu0, cpu1 and cpu2) and then cpu1 was hotunplugged so
>>> only cpu0 and cpu2 are present), QGA will rise a error
>>> error: internal error: unable to execute QEMU agent command
>>> 'guest-get-vcpus':
>>> open("/sys/devices/system/cpu/cpu1/"): No such file or directory
>>> when
>>> virsh vcpucount FOO --guest
>>> is executed.
>>> Fix it by ignoring non present CPUs when fetching CPUs status from sysfs.
>>>
>>> Signed-off-by: Igor Mammedov
>>> ---
>>> v2:
>>> do not create CPU entry if cpu isn't present
>>> (Laszlo Ersek )
>>> ---
>>> qga/commands-posix.c | 115
>>> ++-
>>> 1 file changed, 59 insertions(+), 56 deletions(-)
>>>
>>> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
>>> index 37e8a2d..42d30f0 100644
>>> --- a/qga/commands-posix.c
>>> +++ b/qga/commands-posix.c
>>> @@ -2035,61 +2035,56 @@ static long sysconf_exact(int name, const char
>>> *name_str, Error **errp)
>>> * Written members remain unmodified on error.
>>> */
>>> static void transfer_vcpu(GuestLogicalProcessor *vcpu, bool sys2vcpu,
>>> - Error **errp)
>>> + char *dirpath, Error **errp)
>>> {
>>> -char *dirpath;
>>> +int fd;
>>> +int res;
>>> int dirfd;
>>> +static const char fn[] = "online";
>>>
>>> -dirpath = g_strdup_printf("/sys/devices/system/cpu/cpu%" PRId64 "/",
>>> - vcpu->logical_id);
>>> dirfd = open(dirpath, O_RDONLY | O_DIRECTORY);
>>> if (dirfd == -1) {
>>> error_setg_errno(errp, errno, "open(\"%s\")", dirpath);
>>> -} else {
>>> -static const char fn[] = "online";
>>> -int fd;
>>> -int res;
>>> -
>>> -fd = openat(dirfd, fn, sys2vcpu ? O_RDONLY : O_RDWR);
>>> -if (fd == -1) {
>>> -if (errno != ENOENT) {
>>> -error_setg_errno(errp, errno, "open(\"%s/%s\")", dirpath,
>>> fn);
>>> -} else if (sys2vcpu) {
>>> -vcpu->online = true;
>>> -vcpu->can_offline = false;
>>> -} else if (!vcpu->online) {
>>> -error_setg(errp, "logical processor #%" PRId64 " can't be "
>>> - "offlined", vcpu->logical_id);
>>> -} /* otherwise pretend successful re-onlining */
>>> -} else {
>>> -unsigned char status;
>>> -
>>> -res = pread(fd, &status, 1, 0);
>>> -if (res == -1) {
>>> -error_setg_errno(errp, errno, "pread(\"%s/%s\")", dirpath,
>>> fn);
>>> -} else if (res == 0) {
>>> -error_setg(errp, "pread(\"%s/%s\"): unexpected EOF",
>>> dirpath,
>>> - fn);
>>> -} else if (sys2vcpu) {
>>> -vcpu->online = (status != '0');
>>> -vcpu->can_offline = true;
>>> -} else if (vcpu->online != (status != '0')) {
>>> -status = '0' + vcpu->online;
>>> -if (pwrite(fd, &status, 1, 0) == -1) {
>>> -error_setg_errno(errp, errno, "pwrite(\"%s/%s\")",
>>> dirpath,
>>> - fn);
>>> -}
>>> -} /* otherwise pretend successful re-(on|off)-lining */
>>> +return;
>>> +}
>>>
>>> -res = close(fd);
>>> -g_assert(res == 0);
>>> -}
>>> +fd = openat(dirfd, fn, sys2vcpu ? O_RDONLY : O_RDWR);
>>> +if (fd == -1) {
>>> +if (errno != ENOENT) {
>>> +error_setg_errno(errp, errno, "open(\"%s/%s\")", dirpath, fn);
>>> +} else if (sys2vcpu) {
>>> +vcpu->online = true;
>>> +vcpu->can_offline = false;
>>> +} else if (!vcpu->online) {
>>> +error_setg(errp, "logical processor #%" PRId64 " can't be "
>>> + "offlined", vcpu->logical_id);
>>> +} /* otherwise pretend successful re-onlining */
>>> +} else {
>>> +unsigned char status;
>>> +
>>> +res = pread(fd, &status, 1, 0);
>>> +if (res == -1) {
>>> +error_setg_errno(errp, errno, "pread(\"%s/%s\")", dirpath, fn);
>>> +} else if (res == 0) {
>>> +error_setg(errp, "pread(\"%s/%s\"): unexpected EOF", dirpath,
>>> + fn);
>>> +} else if (sys2vcpu) {
>>> +vcpu->online = (status != '0');
>>> +vcpu->can_offline = true;
>>> +} else if (vcpu->online != (status != '0')) {
>>> +status = '0' + vcpu->online;
>>> +if (pwrite(fd, &status, 1, 0) == -1) {
>>> +error_setg_errno(errp, errno, "pwrite(\"%s/%s\")", dirpath,
>>> + fn);
>>> +}
[Qemu-devel] [PATCH v2 0/3] util: add qemu_write_pidfile()
Hi, Here are a few PID file related patches extracted from "[PATCH v4 00/29] vhost-user for input & GPU" series, with suggestions from Daniel Berrangé. thanks v2: - use an exit notifier (instead of atexit handler) - add an r-b tag Marc-André Lureau (3): util: add qemu_write_pidfile() util: use fcntl() for qemu_write_pidfile() locking Delete PID file on exit include/qemu/osdep.h | 3 +- os-posix.c| 24 -- os-win32.c| 25 --- qga/main.c| 54 ++-- scsi/qemu-pr-helper.c | 40 +++- util/oslib-posix.c| 73 +++ util/oslib-win32.c| 27 vl.c | 18 +-- 8 files changed, 132 insertions(+), 132 deletions(-) -- 2.19.0.rc1
[Qemu-devel] [PATCH v2 3/3] Delete PID file on exit
Register an exit notifier to remove the PID file. By the time atexit()
is called, qemu_write_pidfile() guarantees QEMU owns the PID file,
thus we could safely remove it when exiting.
Signed-off-by: Marc-André Lureau
---
vl.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/vl.c b/vl.c
index 0eaf948d32..51af016602 100644
--- a/vl.c
+++ b/vl.c
@@ -2603,6 +2603,16 @@ static void qemu_run_exit_notifiers(void)
notifier_list_notify(&exit_notifiers, NULL);
}
+static const char *pid_file;
+static Notifier qemu_unlink_pidfile_notifier;
+
+static void qemu_unlink_pidfile(Notifier *n, void *data)
+{
+if (pid_file) {
+unlink(pid_file);
+}
+}
+
bool machine_init_done;
void qemu_add_machine_init_done_notifier(Notifier *notify)
@@ -2927,7 +2937,6 @@ int main(int argc, char **argv, char **envp)
const char *vga_model = NULL;
const char *qtest_chrdev = NULL;
const char *qtest_log = NULL;
-const char *pid_file = NULL;
const char *incoming = NULL;
bool userconfig = true;
bool nographic = false;
@@ -4001,6 +4010,9 @@ int main(int argc, char **argv, char **envp)
exit(1);
}
+qemu_unlink_pidfile_notifier.notify = qemu_unlink_pidfile;
+qemu_add_exit_notifier(&qemu_unlink_pidfile_notifier);
+
if (qemu_init_main_loop(&main_loop_err)) {
error_report_err(main_loop_err);
exit(1);
--
2.19.0.rc1
[Qemu-devel] [PATCH v2 2/3] util: use fcntl() for qemu_write_pidfile() locking
Daniel Berrangé suggested to use fcntl() locks rather than lockf().
'man lockf':
On Linux, lockf() is just an interface on top of fcntl(2) locking.
Many other systems implement lockf() in this way, but note that
POSIX.1 leaves the relationship between lockf() and fcntl(2) locks
unspecified. A portable application should probably avoid mixing
calls to these interfaces.
IOW, if its just a shim around fcntl() on many systems, it is clearer
if we just use fcntl() directly, as we then know how fcntl() locks will
behave if they're on a network filesystem like NFS.
Suggested-by: Daniel P. Berrangé
Signed-off-by: Marc-André Lureau
---
util/oslib-posix.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 0e3ab9d959..fbd0dc8c57 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -95,6 +95,11 @@ bool qemu_write_pidfile(const char *path, Error **errp)
while (1) {
struct stat a, b;
+struct flock lock = {
+.l_type = F_WRLCK,
+.l_whence = SEEK_SET,
+.l_len = 0,
+};
fd = qemu_open(path, O_CREAT | O_WRONLY, S_IRUSR | S_IWUSR);
if (fd == -1) {
@@ -107,7 +112,7 @@ bool qemu_write_pidfile(const char *path, Error **errp)
goto fail_close;
}
-if (lockf(fd, F_TLOCK, 0) < 0) {
+if (fcntl(fd, F_SETLK, &lock)) {
error_setg_errno(errp, errno, "Cannot lock pid file");
goto fail_close;
}
--
2.19.0.rc1
Re: [Qemu-devel] [RFC PATCH v2 1/7] tcg: add headers for non-target helpers
Pavel Dovgalyuk writes:
> From: Pavel Dovgalyuk
>
> This patch adds functions and headers for adding the helpers from
> the modules other than the target translators.
>
> Signed-off-by: Pavel Dovgalyuk
> ---
> include/exec/helper-register.h | 53
>
> tcg/tcg.c | 12 +
> tcg/tcg.h |3 ++
> 3 files changed, 68 insertions(+)
> create mode 100644 include/exec/helper-register.h
>
> diff --git a/include/exec/helper-register.h b/include/exec/helper-register.h
> new file mode 100644
> index 000..aeface9
> --- /dev/null
> +++ b/include/exec/helper-register.h
> @@ -0,0 +1,53 @@
> +#ifndef HELPER_REGISTER_H
> +#define HELPER_REGISTER_H
> +
> +#include "exec/helper-head.h"
> +
> +/* Need one more level of indirection before stringification
> + to get all the macros expanded first. */
> +#define str(s) #s
> +
> +#define DEF_HELPER_FLAGS_0(NAME, FLAGS, ret) \
> +tcg_register_helper(HELPER(NAME), str(NAME), FLAGS, dh_sizemask(ret, 0));
> +
> +#define DEF_HELPER_FLAGS_1(NAME, FLAGS, ret, t1) \
> +tcg_register_helper(HELPER(NAME), str(NAME), FLAGS, \
> +dh_sizemask(ret, 0) | dh_sizemask(t1, 1));
> +
> +#define DEF_HELPER_FLAGS_2(NAME, FLAGS, ret, t1, t2) \
> +tcg_register_helper(HELPER(NAME), str(NAME), FLAGS, \
> +dh_sizemask(ret, 0) | dh_sizemask(t1, 1) | dh_sizemask(t2, 2));
> +
> +#define DEF_HELPER_FLAGS_3(NAME, FLAGS, ret, t1, t2, t3) \
> +tcg_register_helper(HELPER(NAME), str(NAME), FLAGS, \
> +dh_sizemask(ret, 0) | dh_sizemask(t1, 1) | dh_sizemask(t2, 2) \
> +| dh_sizemask(t3, 3));
> +
> +#define DEF_HELPER_FLAGS_4(NAME, FLAGS, ret, t1, t2, t3, t4) \
> +tcg_register_helper(HELPER(NAME), str(NAME), FLAGS, \
> +dh_sizemask(ret, 0) | dh_sizemask(t1, 1) | dh_sizemask(t2, 2) \
> +| dh_sizemask(t3, 3) | dh_sizemask(t4, 4));
> +
> +#define DEF_HELPER_FLAGS_5(NAME, FLAGS, ret, t1, t2, t3, t4, t5) \
> +tcg_register_helper(HELPER(NAME), str(NAME), FLAGS, \
> +dh_sizemask(ret, 0) | dh_sizemask(t1, 1) | dh_sizemask(t2, 2) \
> +| dh_sizemask(t3, 3) | dh_sizemask(t4, 4) | dh_sizemask(t5, 5));
> +
> +#define DEF_HELPER_FLAGS_6(NAME, FLAGS, ret, t1, t2, t3, t4, t5, t6) \
> +tcg_register_helper(HELPER(NAME), str(NAME), FLAGS, \
> +dh_sizemask(ret, 0) | dh_sizemask(t1, 1) | dh_sizemask(t2, 2) \
> +| dh_sizemask(t3, 3) | dh_sizemask(t4, 4) | dh_sizemask(t5, 5) \
> +| dh_sizemask(t6, 6));
> +
> +#include "helper.h"
> +
> +#undef str
> +#undef DEF_HELPER_FLAGS_0
> +#undef DEF_HELPER_FLAGS_1
> +#undef DEF_HELPER_FLAGS_2
> +#undef DEF_HELPER_FLAGS_3
> +#undef DEF_HELPER_FLAGS_4
> +#undef DEF_HELPER_FLAGS_5
> +#undef DEF_HELPER_FLAGS_6
> +
> +#endif /* HELPER_REGISTER_H */
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index 6eeebe0..8191381 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -1623,6 +1623,18 @@ static inline const char *tcg_find_helper(TCGContext
> *s, uintptr_t val)
> return ret;
> }
>
> +void tcg_register_helper(void *func, const char *name,
> + unsigned flags, unsigned sizemask)
> +{
> +TCGHelperInfo *info = g_new0(TCGHelperInfo, 1);
> +info->func = func;
> +info->name = name;
> +info->flags = flags;
> +info->sizemask = sizemask;
> +
> +g_hash_table_insert(helper_table, func, info);
> +}
> +
> static const char * const cond_name[] =
> {
> [TCG_COND_NEVER] = "never",
> diff --git a/tcg/tcg.h b/tcg/tcg.h
> index 08f8bbf..7a4b750 100644
> --- a/tcg/tcg.h
> +++ b/tcg/tcg.h
> @@ -890,6 +890,9 @@ void tcg_register_thread(void);
> void tcg_prologue_init(TCGContext *s);
> void tcg_func_start(TCGContext *s);
>
A small comment might be worthwhile here, maybe something like:
/**
* tcg_register_helper
*
* Used by the helper-register.h machinery to register non-core helpers
* from plugins. Built-in helpers are installed via all_helpers[]
*/
> +void tcg_register_helper(void *func, const char *name,
> + unsigned flags, unsigned sizemask);
> +
> int tcg_gen_code(TCGContext *s, TranslationBlock *tb);
>
> void tcg_set_frame(TCGContext *s, TCGReg reg, intptr_t start, intptr_t size);
--
Alex Bennée
[Qemu-devel] [PATCH v2 1/3] util: add qemu_write_pidfile()
There are variants of qemu_create_pidfile() in qemu-pr-helper and
qemu-ga. Let's have a common implementation in libqemuutil.
The code is initially based from pr-helper write_pidfile(), with
various improvements and suggestions from Daniel Berrangé:
QEMU will leave the pidfile existing on disk when it exits which
initially made me think it avoids the deletion race. The app
managing QEMU, however, may well delete the pidfile after it has
seen QEMU exit, and even if the app locks the pidfile before
deleting it, there is still a race.
eg consider the following sequence
QEMU 1libvirtdQEMU 2
1.lock(pidfile)
2.exit()
3. open(pidfile)
4. lock(pidfile)
5. open(pidfile)
6. unlink(pidfile)
7. close(pidfile)
8. lock(pidfile)
IOW, at step 8 the new QEMU has successfully acquired the lock, but
the pidfile no longer exists on disk because it was deleted after
the original QEMU exited.
While we could just say no external app should ever delete the
pidfile, I don't think that is satisfactory as people don't read
docs, and admins don't like stale pidfiles being left around on
disk.
To make this robust, I think we might want to copy libvirt's
approach to pidfile acquisition which runs in a loop and checks that
the file on disk /after/ acquiring the lock matches the file that
was locked. Then we could in fact safely let QEMU delete its own
pidfiles on clean exit..
Signed-off-by: Marc-André Lureau
Reviewed-by: Daniel P. Berrangé
---
include/qemu/osdep.h | 3 +-
os-posix.c| 24 ---
os-win32.c| 25
qga/main.c| 54 +++---
scsi/qemu-pr-helper.c | 40 -
util/oslib-posix.c| 68 +++
util/oslib-win32.c| 27 +
vl.c | 4 +--
8 files changed, 114 insertions(+), 131 deletions(-)
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index a91068df0e..47fa570bd4 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -448,7 +448,8 @@ bool qemu_has_ofd_lock(void);
#define FMT_pid "%d"
#endif
-int qemu_create_pidfile(const char *filename);
+bool qemu_write_pidfile(const char *pidfile, Error **errp);
+
int qemu_get_thread_id(void);
#ifndef CONFIG_IOVEC
diff --git a/os-posix.c b/os-posix.c
index 9ce6f74513..0e9403b4ff 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -352,30 +352,6 @@ void os_set_line_buffering(void)
setvbuf(stdout, NULL, _IOLBF, 0);
}
-int qemu_create_pidfile(const char *filename)
-{
-char buffer[128];
-int len;
-int fd;
-
-fd = qemu_open(filename, O_RDWR | O_CREAT, 0600);
-if (fd == -1) {
-return -1;
-}
-if (lockf(fd, F_TLOCK, 0) == -1) {
-close(fd);
-return -1;
-}
-len = snprintf(buffer, sizeof(buffer), FMT_pid "\n", getpid());
-if (write(fd, buffer, len) != len) {
-close(fd);
-return -1;
-}
-
-/* keep pidfile open & locked forever */
-return 0;
-}
-
bool is_daemonized(void)
{
return daemonize;
diff --git a/os-win32.c b/os-win32.c
index 0674f94b57..0e0d7f50f3 100644
--- a/os-win32.c
+++ b/os-win32.c
@@ -97,28 +97,3 @@ int os_parse_cmd_args(int index, const char *optarg)
{
return -1;
}
-
-int qemu_create_pidfile(const char *filename)
-{
-char buffer[128];
-int len;
-HANDLE file;
-OVERLAPPED overlap;
-BOOL ret;
-memset(&overlap, 0, sizeof(overlap));
-
-file = CreateFile(filename, GENERIC_WRITE, FILE_SHARE_READ, NULL,
- OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
-
-if (file == INVALID_HANDLE_VALUE) {
-return -1;
-}
-len = snprintf(buffer, sizeof(buffer), "%d\n", getpid());
-ret = WriteFile(file, (LPCVOID)buffer, (DWORD)len,
-NULL, &overlap);
-CloseHandle(file);
-if (ret == 0) {
-return -1;
-}
-return 0;
-}
diff --git a/qga/main.c b/qga/main.c
index 6d70242d05..c399320d3c 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -340,46 +340,6 @@ static FILE *ga_open_logfile(const char *logfile)
return f;
}
-#ifndef _WIN32
-static bool ga_open_pidfile(const char *pidfile)
-{
-int pidfd;
-char pidstr[32];
-
-pidfd = qemu_open(pidfile, O_CREAT|O_WRONLY, S_IRUSR|S_IWUSR);
-if (pidfd == -1 || lockf(pidfd, F_TLOCK, 0)) {
-g_critical("Cannot lock pid file, %s", strerror(errno));
-if (pidfd != -1) {
-close(pidfd);
-}
-return false;
-}
-
-if (ftruncate(pidfd, 0)) {
-g_critical("Failed to truncate pid file");
-goto fail;
-}
-snprintf(pidstr, sizeof(pidstr), "%d\n", getpid());
-if (write(pidfd, pidstr, strlen(pidstr)) != strlen(pidstr)) {
-g_critical("Failed to write pid file
Re: [Qemu-devel] [PATCH for-3.1 v10 04/31] block: Add BDS.auto_backing_file
On Fri 07 Sep 2018 01:32:58 PM CEST, Max Reitz wrote: > On 2018-09-05 16:22, Alberto Garcia wrote: >> On Thu 09 Aug 2018 11:35:01 PM CEST, Max Reitz wrote: >>> If the backing file is overridden, this most probably does change the >>> guest-visible data of a BDS. Therefore, we will need to consider this >>> in bdrv_refresh_filename(). >>> >>> To see whether it has been overridden, we might want to compare >>> bs->backing_file and bs->backing->bs->filename. However, >>> bs->backing_file is changed by bdrv_set_backing_hd() (which is just used >>> to change the backing child at runtime, without modifying the image >>> header), so bs->backing_file most of the time simply contains a copy of >>> bs->backing->bs->filename anyway, so it is useless for such a >>> comparison. >> >> What's the point of bs->backing_file then? In what cases is it different >> from backing->bs->filename? > > Good question! Yes, why? > > One thing is when you have detached the backing file, bs->backing_file > will stay the same, even though backing is now NULL. But that is > pretty much useless, I couldn't find any part in the block layer which > relies on this. [...] > But when the backing BDS is attached, bdrv_set_backing_hd() (through > bdrv_backing_attach()) will update backing_file to "foo/base.qcow2" > (or probably the absolute equivalent, I can't remember). But then why do you need bs->backing_file at all? What are the cases where you need bs->backing_file but you can't use directly the value in bs->backing->bs->filename because it's wrong or missing? > So consequentially, there is a "block: Leave BDS.backing_file > constant" patch in my "block: Deal with filters" series. It makes > backing_file always report what the image header says. Ok, so after tha patch bs->backing_file is guaranteed to be exactly as it's written on the image header. And bs->auto_backing_file is the backing filename after having called bdrv_refresh_filename(). And that's supposed to be exactly the same as bs->backing->bs->filename. So I have the same question again, why do you need bs->auto_backing_filename and in what cases is it different from bs->auto_backing_file different from bs->backing->bs->filename? Berto
Re: [Qemu-devel] [PATCH 0/3] scsi: remove lsi53c895a_create() and lsi53c810_create() functions
On 07/09/18 07:27, Thomas Huth wrote:
> On 2018-09-06 19:15, Mark Cave-Ayland wrote:
>> On 06/09/18 17:40, Thomas Huth wrote:
> [...]
>> Amusingly the main reason I need to expose the LSIState at all is to be
>> able to call scsi_bus_legacy_handle_cmdline() on the SCSI bus object
>> itself. I guess you could say that this is an argument in favour of the
>> existing approach, but then you're effectively moving back to the
>> equivalent of _init() functions for this one particular case which these
>> days are considered to be bad.
>
> If you mind that the "init" function creates the object, too, then
> simply remove the two "lsi53c*_create" functions and provide a function
> that looks like this instead:
>
> void lsi53c8xx_handle_legacy_cmdline(DeviceState *lsi_dev)
> {
> LSIState *s = LSI53C895A(lsi_dev);
>
> scsi_bus_legacy_handle_cmdline(&s->bus);
> }
>
> Then you can create the device from another .c file and simply call this
> new function instead of scsi_bus_legacy_handle_cmdline() in that other
> .c file.
>
> In the long run, I think we should maybe also rather get rid of
> scsi_bus_legacy_handle_cmdline() and either remove -drive if=scsi or
> provide another more generic solution instead (e.g. some code that scans
> the qdev tree for SCSI controllers and attaches the devices declared
> with -drive if=scsi automatically there).
>
>> My feeling is that since the pattern of a separate header with struct
>> and QOM macros (or "modern" style) is used throughout the rest of the
>> codebase, why should we make an exception for this one particular case?
>
> I don't mind too much, so if you post your series again, I won't object
> again. But still, even if it's a little bit more work for you now, if we
> reconsider in a couple of years that information hiding would be a good
> idea, it very likely way more work to make the struct private again, so
> I'd really prefer if you could keep it private now if possible.
Well your modified function above certainly fixes my particular use case
- I'm just in the process of testing a v2 patchset that implements this
approach which I hope to post shortly.
At the end of the day I suspect that regardless of what is finally
committed, it won't really matter that much given the enormity of the
task to change the way that structs and scope are handled throughout the
codebase. So I'm happy either way :)
ATB,
Mark.
Re: [Qemu-devel] [RFC PATCH v2 2/7] Add plugin support
Pavel Dovgalyuk writes:
> This patch adds support for dynamically loaded plugins.
> Every plugin is a dynamic library with a set of optional exported
> functions that will be called from QEMU.
>
> Signed-off-by: Pavel Dovgalyuk
> ---
> Makefile.target |1
> configure | 14 ++-
> include/qemu/plugins.h|8
> plugins/include/plugins.h | 12 ++
> plugins/plugins.c | 91
> +
> qemu-options.hx | 10 +
> vl.c |8
> 7 files changed, 143 insertions(+), 1 deletion(-)
> create mode 100644 include/qemu/plugins.h
> create mode 100644 plugins/include/plugins.h
> create mode 100644 plugins/plugins.c
>
> diff --git a/Makefile.target b/Makefile.target
> index dad2cf8..4cffd96 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -93,6 +93,7 @@ all: $(PROGS) stap
> # cpu emulator library
> obj-y += exec.o
> obj-y += accel/
> +obj-$(CONFIG_PLUGINS) += plugins/plugins.o
> obj-$(CONFIG_TCG) += tcg/tcg.o tcg/tcg-op.o tcg/tcg-op-vec.o
> tcg/tcg-op-gvec.o
> obj-$(CONFIG_TCG) += tcg/tcg-common.o tcg/optimize.o
> obj-$(CONFIG_TCG_INTERPRETER) += tcg/tci.o
> diff --git a/configure b/configure
> index a71bf9b..34e6f00 100755
> --- a/configure
> +++ b/configure
> @@ -373,6 +373,7 @@ EXESUF=""
> DSOSUF=".so"
> LDFLAGS_SHARED="-shared"
> modules="no"
> +plugins="no"
> prefix="/usr/local"
> mandir="\${prefix}/share/man"
> datadir="\${prefix}/share"
> @@ -922,6 +923,12 @@ for opt do
>--disable-modules)
>modules="no"
>;;
> + --enable-plugins)
> + plugins="yes"
> + ;;
> + --disable-plugins)
> + plugins="no"
> + ;;
>--cpu=*)
>;;
>--target-list=*) target_list="$optarg"
> @@ -1567,6 +1574,7 @@ disabled with --disable-FEATURE, default is enabled if
> available:
>guest-agent-msi build guest agent Windows MSI installation package
>pie Position Independent Executables
>modules modules support
> + plugins plugins support
>debug-tcg TCG debugging (default is disabled)
>debug-info debugging information
>sparse sparse checker
> @@ -3392,7 +3400,7 @@ else
> glib_req_ver=2.22
> fi
> glib_modules=gthread-2.0
> -if test "$modules" = yes; then
> +if test "$modules" = yes || test "$plugins" = yes; then
> glib_modules="$glib_modules gmodule-export-2.0"
> fi
>
> @@ -5777,6 +5785,7 @@ if test "$slirp" = "yes" ; then
> echo "smbd $smbd"
> fi
> echo "module support$modules"
> +echo "plugin support$plugins"
> echo "host CPU $cpu"
> echo "host big endian $bigendian"
> echo "target list $target_list"
> @@ -6111,6 +6120,9 @@ if test "$modules" = "yes"; then
>echo "CONFIG_STAMP=_$( (echo $qemu_version; echo $pkgversion; cat $0) |
> $shacmd - | cut -f1 -d\ )" >> $config_host_mak
>echo "CONFIG_MODULES=y" >> $config_host_mak
> fi
> +if test "$plugins" = "yes"; then
> + echo "CONFIG_PLUGINS=y" >> $config_host_mak
> +fi
> if test "$have_x11" = "yes" -a "$need_x11" = "yes"; then
>echo "CONFIG_X11=y" >> $config_host_mak
>echo "X11_CFLAGS=$x11_cflags" >> $config_host_mak
> diff --git a/include/qemu/plugins.h b/include/qemu/plugins.h
> new file mode 100644
> index 000..4464822
> --- /dev/null
> +++ b/include/qemu/plugins.h
> @@ -0,0 +1,8 @@
> +#ifndef PLUGINS_H
> +#define PLUGINS_H
> +
> +void qemu_plugin_parse_cmd_args(const char *optarg);
> +void qemu_plugin_load(const char *filename, const char *args);
> +void qemu_plugins_init(void);
I think you want to have an #ifdef CONFIG_PLUGINS here and some empty
inlines for the non CONFIG_PLUGINS case so you don't need to ifdef so
much later.
> +
> +#endif /* PLUGINS_H */
> diff --git a/plugins/include/plugins.h b/plugins/include/plugins.h
> new file mode 100644
> index 000..100a786
> --- /dev/null
> +++ b/plugins/include/plugins.h
> @@ -0,0 +1,12 @@
> +#ifndef PLUGINS_INTERFACE_H
> +#define PLUGINS_INTERFACE_H
> +
> +#include
> +
> +/* Plugin interface */
> +
> +bool plugin_init(const char *args);
> +bool plugin_needs_before_insn(uint64_t pc, void *cpu);
> +void plugin_before_insn(uint64_t pc, void *cpu);
> +
> +#endif /* PLUGINS_INTERFACE_H */
> diff --git a/plugins/plugins.c b/plugins/plugins.c
> new file mode 100644
> index 000..eabc931
> --- /dev/null
> +++ b/plugins/plugins.c
> @@ -0,0 +1,91 @@
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "qemu/error-report.h"
> +#include "qemu/plugins.h"
> +#include "qemu/queue.h"
> +#include
> +
> +typedef bool (*PluginInitFunc)(const char *);
> +typedef bool (*PluginNeedsBeforeInsnFunc)(uint64_t, void *);
> +typedef void (*PluginBeforeInsnFunc)(uint64_t, void *);
> +
> +typedef struct QemuPluginInfo {
> +const char *filename;
> +const char *args;
> +GModule *g_module;
> +
> +PluginInitFunc init;
> +PluginNeedsBeforeInsnFunc needs_before_insn;
> +PluginBef
Re: [Qemu-devel] [Qemu-block] [PATCH v2 01/11] block: Mark commit and mirror as filter drivers
On Fri 10 Aug 2018 12:31:07 AM CEST, Max Reitz wrote: > The commit and mirror block nodes are filters, so they should be marked > as such. > > Signed-off-by: Max Reitz Reviewed-by: Alberto Garcia Berto
Re: [Qemu-devel] [PATCH for-3.1 v10 04/31] block: Add BDS.auto_backing_file
On 2018-09-07 14:28, Alberto Garcia wrote: > On Fri 07 Sep 2018 01:32:58 PM CEST, Max Reitz wrote: >> On 2018-09-05 16:22, Alberto Garcia wrote: >>> On Thu 09 Aug 2018 11:35:01 PM CEST, Max Reitz wrote: If the backing file is overridden, this most probably does change the guest-visible data of a BDS. Therefore, we will need to consider this in bdrv_refresh_filename(). To see whether it has been overridden, we might want to compare bs->backing_file and bs->backing->bs->filename. However, bs->backing_file is changed by bdrv_set_backing_hd() (which is just used to change the backing child at runtime, without modifying the image header), so bs->backing_file most of the time simply contains a copy of bs->backing->bs->filename anyway, so it is useless for such a comparison. >>> >>> What's the point of bs->backing_file then? In what cases is it different >>> from backing->bs->filename? >> >> Good question! Yes, why? >> >> One thing is when you have detached the backing file, bs->backing_file >> will stay the same, even though backing is now NULL. But that is >> pretty much useless, I couldn't find any part in the block layer which >> relies on this. > [...] >> But when the backing BDS is attached, bdrv_set_backing_hd() (through >> bdrv_backing_attach()) will update backing_file to "foo/base.qcow2" >> (or probably the absolute equivalent, I can't remember). > > But then why do you need bs->backing_file at all? What are the cases > where you need bs->backing_file but you can't use directly the value in > bs->backing->bs->filename because it's wrong or missing? The main case is when bs->backing has not been opened yet. Then bdrv_open_backing_file() needs some place that tells it what image file to open. >> So consequentially, there is a "block: Leave BDS.backing_file >> constant" patch in my "block: Deal with filters" series. It makes >> backing_file always report what the image header says. > > Ok, so after tha patch bs->backing_file is guaranteed to be exactly as > it's written on the image header. Yes. > And bs->auto_backing_file is the backing filename after having called > bdrv_refresh_filename(). And that's supposed to be exactly the same as > bs->backing->bs->filename. No. It's supposed to be exactly the same, *if* bs->backing->bs has been opened only based on bs->backing_file. The user can override the backing file (either as a whole through the @backing node reference, or at least some of its options as a dict under @backing), and then bs->auto_backing_filename is not supposed to be the same as bs->backing->bs->filename. > So I have the same question again, why do you > need bs->auto_backing_filename and in what cases is it different from > bs->auto_backing_file different from bs->backing->bs->filename? The whole purpose of bs->auto_backing_file is so you can compare it with bs->backing->bs->filename, see when it differs, and then you know that the user has changed the backing file from what it would be based on the image header alone (that is, if the user hadn't specified any @backing option at all). This is exactly what the commit message says. It's just that for this comparison to really make sense, we would need the refreshed filename of the BDS that is opened when you just take the image header's backing filename. If we know that the user has not specified any options that override it, we copy bs->backing->bs->filename to bs->auto_backing_filename, because we know it to be exactly that value that we want. Otherwise, we just have to hope that refreshing the filename does not change it from what the image header said. (Now you could ask yourself, if we know the user hasn't specified any options to override the backing file -- why don't we just store a flag? I tried that, Kevin wasn't really happy about it because it's hard to catch all corner cases. There are many ways to change a backing file, and if you want to find out when a backing file has been changed to exactly what the image header says (this is the ideal outcome of the commit block job, for instance), you basically get back to the same problem of comparing filenames.) Let me try to summarize what bs->auto_backing_filename is again. We want to compare it against bs->backing->bs->filename, which is basically bdrv_filename(bdrv_open(X)). Question is, is X the same as bs->backing_file? (assuming bs->backing_file is what the image header says) So ideally, bs->auto_backing_filename needs to be the result of bdrv_filename(bdrv_open(bs->backing_file)). But that is rather stupid to do, so we don't do that. The next best thing is just using bs->backing_file and hoping that bdrv_refresh_filename() didn't change anything. Sometimes, that hope is futile, however. So I try to get bdrv_refresh_filename() squeezed in there sometimes. When at any point we know that bs->backing->bs is exactly bdrv_open(bs->backing_file), we know that bs->backing->bs->filename is bdrv_filenam
[Qemu-devel] [PATCH v2 0/3] scsi: replace lsi53c895a_create() and lsi53c810_create() functions
As part of an upcoming 40p patchset I have a requirement to change the PCI configuration of the LSI SCSI. However since commits a64aa5785d "hw: Deprecate -drive if=scsi with non-onboard HBAs" and b891538e81 "hw/ppc/prep: Fix implicit creation of "-drive if=scsi", the lsi53c8*_create() wrapper functions don't return the device state itself. This patchset replaces the lsi53c895a_create() and lsi53c810_create() functions with a single lsi53c8xx_handle_legacy_cmdline() function as suggested by Thomas, which makes the caller responsible for initing the LSI SCSI device and hence allowing it to be configured as required. Signed-off-by: Mark Cave-Ayland v2: - Don't split LSIState into separate lsi53c895a.h header but instead use a new lsi53c8xx_handle_legacy_cmdline() function as suggested by Thomas Mark Cave-Ayland (3): scsi: add lsi53c8xx_handle_legacy_cmdline() function scsi: move lsi53c8xx_create() callers to lsi53c8xx_handle_legacy_cmdline() scsi: remove unused lsi53c895a_create() and lsi53c810_create() functions hw/arm/realview.c| 3 ++- hw/arm/versatilepb.c | 3 ++- hw/hppa/machine.c| 4 +++- hw/ppc/prep.c| 4 +++- hw/scsi/lsi53c895a.c | 11 ++- include/hw/pci/pci.h | 3 +-- 6 files changed, 13 insertions(+), 15 deletions(-) -- 2.11.0
[Qemu-devel] [PATCH v2 1/3] scsi: add lsi53c8xx_handle_legacy_cmdline() function
This is the function that will soon be used to replace lsi53c895a_create() and
lsi53c810_create().
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/lsi53c895a.c | 7 +++
include/hw/pci/pci.h | 1 +
2 files changed, 8 insertions(+)
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index 955ba94800..8f5ab82d57 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -2290,3 +2290,10 @@ void lsi53c810_create(PCIBus *bus, int devfn)
scsi_bus_legacy_handle_cmdline(&s->bus);
}
+
+void lsi53c8xx_handle_legacy_cmdline(DeviceState *lsi_dev)
+{
+LSIState *s = LSI53C895A(lsi_dev);
+
+scsi_bus_legacy_handle_cmdline(&s->bus);
+}
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 990d6fcbde..0d907dc084 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -709,6 +709,7 @@ PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const
char *name);
void lsi53c895a_create(PCIBus *bus);
void lsi53c810_create(PCIBus *bus, int devfn);
+void lsi53c8xx_handle_legacy_cmdline(DeviceState *lsi_dev);
qemu_irq pci_allocate_irq(PCIDevice *pci_dev);
void pci_set_irq(PCIDevice *pci_dev, int level);
--
2.11.0
[Qemu-devel] [PATCH v2 3/3] scsi: remove unused lsi53c895a_create() and lsi53c810_create() functions
Now that these functions are no longer required they can be removed.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/lsi53c895a.c | 14 --
include/hw/pci/pci.h | 2 --
2 files changed, 16 deletions(-)
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index 8f5ab82d57..f5cbbf653c 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -2277,20 +2277,6 @@ static void lsi53c895a_register_types(void)
type_init(lsi53c895a_register_types)
-void lsi53c895a_create(PCIBus *bus)
-{
-LSIState *s = LSI53C895A(pci_create_simple(bus, -1, "lsi53c895a"));
-
-scsi_bus_legacy_handle_cmdline(&s->bus);
-}
-
-void lsi53c810_create(PCIBus *bus, int devfn)
-{
-LSIState *s = LSI53C895A(pci_create_simple(bus, devfn, "lsi53c810"));
-
-scsi_bus_legacy_handle_cmdline(&s->bus);
-}
-
void lsi53c8xx_handle_legacy_cmdline(DeviceState *lsi_dev)
{
LSIState *s = LSI53C895A(lsi_dev);
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 0d907dc084..e6514bba23 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -707,8 +707,6 @@ PCIDevice *pci_create_simple_multifunction(PCIBus *bus, int
devfn,
PCIDevice *pci_create(PCIBus *bus, int devfn, const char *name);
PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name);
-void lsi53c895a_create(PCIBus *bus);
-void lsi53c810_create(PCIBus *bus, int devfn);
void lsi53c8xx_handle_legacy_cmdline(DeviceState *lsi_dev);
qemu_irq pci_allocate_irq(PCIDevice *pci_dev);
--
2.11.0
[Qemu-devel] [Bug 1716292] Re: User mode emulation returns wrong value for write(fd, NULL, 0)
This happens for me also, with qemu version 2.12.0 (Debian
1:2.12+dfsg-3).
An initial patch was proposed here: https://lists.gnu.org/archive/html
/qemu-devel/2017-09/msg08073.html
Discussion pointed out some problems, and the patch languished and was
not accepted.
Here is a summary of the changes needed for it to be more likely for the
patch to be accepted: https://lists.gnu.org/archive/html/qemu-
devel/2018-02/msg03964.html
- change from "ret = 0" to something like "ret = get_errno(safe_write(arg1,
NULL, 0))"
- change TARGET_NR_read to do the same, instead of its current short-circuit
behaviour for count==0
- check pread64/pwrite64 to see if they need a similar change as well
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1716292
Title:
User mode emulation returns wrong value for write(fd, NULL, 0)
Status in QEMU:
New
Bug description:
QEMU version: latest master (fcea73709b966a7ded9efa7b106ea50c7fe9025c)
OS version: Ubuntu 14.04.5
Configured with: ../configure --target-list=x86_64-linux-user
QEMU Linux usermode emulation does not handle write() syscalls with
zero length and a null pointer correctly: on Linux this returns 0, but
in emulation this returns -1.
I ran into this while using an aarch64 abuild-tar from Alpine Linux in
user-mode emulation; here's the minimized reproduction test case:
zhuowei@zhuowei-tablet:/tmp$ cat writezerobytes.c
#include
#include
#include
int main() {
ssize_t ret = write(STDOUT_FILENO, NULL, 0);
fprintf(stderr, "write returned %ld\n", ret);
return 0;
}
zhuowei@zhuowei-tablet:/tmp$ gcc -o writezerobytes writezerobytes.c
zhuowei@zhuowei-tablet:/tmp$ uname -a
Linux zhuowei-tablet 3.13.0-129-generic #178-Ubuntu SMP Fri Aug 11 12:48:20
UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
zhuowei@zhuowei-tablet:/tmp$ ./writezerobytes
write returned 0
zhuowei@zhuowei-tablet:/tmp$
/media/zhuowei/redhd/docs/repos/qemu/build4/x86_64-linux-user/qemu-x86_64
./writezerobytes
write returned -1
zhuowei@zhuowei-tablet:/tmp$
/media/zhuowei/redhd/docs/repos/qemu/build4/x86_64-linux-user/qemu-x86_64
--version
qemu-x86_64 version 2.10.50 (v2.10.0-471-gfcea737-dirty)
Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1716292/+subscriptions
[Qemu-devel] [PATCH v2 2/3] scsi: move lsi53c8xx_create() callers to lsi53c8xx_handle_legacy_cmdline()
As part of commits a64aa5785d "hw: Deprecate -drive if=scsi with non-onboard
HBAs" and b891538e81 "hw/ppc/prep: Fix implicit creation of "-drive if=scsi"
devices" the lsi53c895a_create() and lsi53c810_create() functions were added
to wrap pci_create_simple() and scsi_bus_legacy_handle_cmdline().
Unfortunately this prevents us from changing qdev properties on the device
and/or changing the PCI configuration. By switching over to using the new
lsi53c8xx_handle_legacy_cmdline() function then the caller can now configure
and realize the LSI SCSI device exactly as required.
Signed-off-by: Mark Cave-Ayland
---
hw/arm/realview.c| 3 ++-
hw/arm/versatilepb.c | 3 ++-
hw/hppa/machine.c| 4 +++-
hw/ppc/prep.c| 4 +++-
4 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index ab8c14fde3..242f5a87b6 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -257,7 +257,8 @@ static void realview_init(MachineState *machine,
}
n = drive_get_max_bus(IF_SCSI);
while (n >= 0) {
-lsi53c895a_create(pci_bus);
+dev = DEVICE(pci_create_simple(pci_bus, -1, "lsi53c895a"));
+lsi53c8xx_handle_legacy_cmdline(dev);
n--;
}
}
diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c
index 8b74857059..22b09a1e61 100644
--- a/hw/arm/versatilepb.c
+++ b/hw/arm/versatilepb.c
@@ -278,7 +278,8 @@ static void versatile_init(MachineState *machine, int
board_id)
}
n = drive_get_max_bus(IF_SCSI);
while (n >= 0) {
-lsi53c895a_create(pci_bus);
+dev = DEVICE(pci_create_simple(pci_bus, -1, "lsi53c895a"));
+lsi53c8xx_handle_legacy_cmdline(dev);
n--;
}
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index cf7c61c6cc..0fb8fb877e 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -59,6 +59,7 @@ static void machine_hppa_init(MachineState *machine)
const char *kernel_filename = machine->kernel_filename;
const char *kernel_cmdline = machine->kernel_cmdline;
const char *initrd_filename = machine->initrd_filename;
+DeviceState *dev;
PCIBus *pci_bus;
ISABus *isa_bus;
qemu_irq rtc_irq, serial_irq;
@@ -115,7 +116,8 @@ static void machine_hppa_init(MachineState *machine)
}
/* SCSI disk setup. */
-lsi53c895a_create(pci_bus);
+dev = DEVICE(pci_create_simple(pci_bus, -1, "lsi53c895a"));
+lsi53c8xx_handle_legacy_cmdline(dev);
/* Network setup. e1000 is good enough, failing Tulip support. */
for (i = 0; i < nb_nics; i++) {
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 162b27a3b8..b0ea20416e 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -702,7 +702,9 @@ static void ibm_40p_init(MachineState *machine)
qdev_prop_set_uint32(dev, "equipment", 0xc0);
qdev_init_nofail(dev);
-lsi53c810_create(pci_bus, PCI_DEVFN(1, 0));
+dev = DEVICE(pci_create_simple(pci_bus, PCI_DEVFN(1, 0),
+ "lsi53c810"));
+lsi53c8xx_handle_legacy_cmdline(dev);
/* XXX: s3-trio at PCI_DEVFN(2, 0) */
pci_vga_init(pci_bus);
--
2.11.0
[Qemu-devel] [Bug 1716292] Re: User mode emulation returns wrong value for write(fd, NULL, 0)
** Patch added:
"0001-Bring-linux-user-write-2-handling-into-line-with-lin.patch"
https://bugs.launchpad.net/qemu/+bug/1716292/+attachment/5186008/+files/0001-Bring-linux-user-write-2-handling-into-line-with-lin.patch
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1716292
Title:
User mode emulation returns wrong value for write(fd, NULL, 0)
Status in QEMU:
New
Bug description:
QEMU version: latest master (fcea73709b966a7ded9efa7b106ea50c7fe9025c)
OS version: Ubuntu 14.04.5
Configured with: ../configure --target-list=x86_64-linux-user
QEMU Linux usermode emulation does not handle write() syscalls with
zero length and a null pointer correctly: on Linux this returns 0, but
in emulation this returns -1.
I ran into this while using an aarch64 abuild-tar from Alpine Linux in
user-mode emulation; here's the minimized reproduction test case:
zhuowei@zhuowei-tablet:/tmp$ cat writezerobytes.c
#include
#include
#include
int main() {
ssize_t ret = write(STDOUT_FILENO, NULL, 0);
fprintf(stderr, "write returned %ld\n", ret);
return 0;
}
zhuowei@zhuowei-tablet:/tmp$ gcc -o writezerobytes writezerobytes.c
zhuowei@zhuowei-tablet:/tmp$ uname -a
Linux zhuowei-tablet 3.13.0-129-generic #178-Ubuntu SMP Fri Aug 11 12:48:20
UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
zhuowei@zhuowei-tablet:/tmp$ ./writezerobytes
write returned 0
zhuowei@zhuowei-tablet:/tmp$
/media/zhuowei/redhd/docs/repos/qemu/build4/x86_64-linux-user/qemu-x86_64
./writezerobytes
write returned -1
zhuowei@zhuowei-tablet:/tmp$
/media/zhuowei/redhd/docs/repos/qemu/build4/x86_64-linux-user/qemu-x86_64
--version
qemu-x86_64 version 2.10.50 (v2.10.0-471-gfcea737-dirty)
Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1716292/+subscriptions
Re: [Qemu-devel] [PATCH v2 1/3] scsi: add lsi53c8xx_handle_legacy_cmdline() function
On 2018-09-07 14:56, Mark Cave-Ayland wrote: > This is the function that will soon be used to replace lsi53c895a_create() and > lsi53c810_create(). > > Signed-off-by: Mark Cave-Ayland > --- > hw/scsi/lsi53c895a.c | 7 +++ > include/hw/pci/pci.h | 1 + > 2 files changed, 8 insertions(+) Reviewed-by: Thomas Huth
Re: [Qemu-devel] [RFC PATCH v2 3/7] plugins: provide helper functions for plugins
Pavel Dovgalyuk writes:
> From: Pavel Dovgalyuk
>
> This patch adds interface functions that may be called from the loaded
> plugins.
> Such functions are needed to inspect the VM state and to pass data
> to the QEMU (e.g., QEMU-side logging).
>
> Signed-off-by: Pavel Dovgalyuk
Reviewed-by: Alex Bennée
> ---
> Makefile.target |2 +-
> plugins/include/plugins.h |6 ++
> plugins/qemulib.c | 31 +++
> 3 files changed, 38 insertions(+), 1 deletion(-)
> create mode 100644 plugins/qemulib.c
>
> diff --git a/Makefile.target b/Makefile.target
> index 4cffd96..5648c9c 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -93,7 +93,7 @@ all: $(PROGS) stap
> # cpu emulator library
> obj-y += exec.o
> obj-y += accel/
> -obj-$(CONFIG_PLUGINS) += plugins/plugins.o
> +obj-$(CONFIG_PLUGINS) += plugins/plugins.o plugins/qemulib.o
> obj-$(CONFIG_TCG) += tcg/tcg.o tcg/tcg-op.o tcg/tcg-op-vec.o
> tcg/tcg-op-gvec.o
> obj-$(CONFIG_TCG) += tcg/tcg-common.o tcg/optimize.o
> obj-$(CONFIG_TCG_INTERPRETER) += tcg/tci.o
> diff --git a/plugins/include/plugins.h b/plugins/include/plugins.h
> index 100a786..fa624ea 100644
> --- a/plugins/include/plugins.h
> +++ b/plugins/include/plugins.h
> @@ -9,4 +9,10 @@ bool plugin_init(const char *args);
> bool plugin_needs_before_insn(uint64_t pc, void *cpu);
> void plugin_before_insn(uint64_t pc, void *cpu);
>
> +/* QEMU interface */
> +
> +void qemulib_log(const char *fmt, ...) /*GCC_FMT_ATTR(1, 2)*/;
> +int qemulib_read_memory(void *cpu, uint64_t addr, uint8_t *buf, int len);
> +int qemulib_read_register(void *cpu, uint8_t *mem_buf, int reg);
> +
> #endif /* PLUGINS_INTERFACE_H */
> diff --git a/plugins/qemulib.c b/plugins/qemulib.c
> new file mode 100644
> index 000..eb812c1
> --- /dev/null
> +++ b/plugins/qemulib.c
> @@ -0,0 +1,31 @@
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "cpu.h"
> +#include "qemu/error-report.h"
> +#include "qemu/plugins.h"
> +#include "qemu/log.h"
> +#include "include/plugins.h"
> +
> +void qemulib_log(const char *fmt, ...)
> +{
> +va_list args;
> +va_start(args, fmt);
> +qemu_log_vprintf(fmt, args);
> +va_end(args);
> +}
> +
> +int qemulib_read_memory(void *cpu, uint64_t addr, uint8_t *buf, int len)
> +{
> +return cpu_memory_rw_debug(cpu, addr, buf, len, false);
> +}
> +
> +int qemulib_read_register(void *cpu, uint8_t *mem_buf, int reg)
> +{
> +CPUClass *cc = CPU_GET_CLASS(cpu);
> +
> +if (reg < cc->gdb_num_core_regs) {
> +return cc->gdb_read_register(cpu, mem_buf, reg);
> +}
> +
> +return 0;
> +}
--
Alex Bennée
Re: [Qemu-devel] [PATCH v2 2/3] scsi: move lsi53c8xx_create() callers to lsi53c8xx_handle_legacy_cmdline()
On 2018-09-07 14:56, Mark Cave-Ayland wrote: > As part of commits a64aa5785d "hw: Deprecate -drive if=scsi with non-onboard > HBAs" and b891538e81 "hw/ppc/prep: Fix implicit creation of "-drive if=scsi" > devices" the lsi53c895a_create() and lsi53c810_create() functions were added > to wrap pci_create_simple() and scsi_bus_legacy_handle_cmdline(). > > Unfortunately this prevents us from changing qdev properties on the device > and/or changing the PCI configuration. By switching over to using the new > lsi53c8xx_handle_legacy_cmdline() function then the caller can now configure > and realize the LSI SCSI device exactly as required. > > Signed-off-by: Mark Cave-Ayland > --- > hw/arm/realview.c| 3 ++- > hw/arm/versatilepb.c | 3 ++- > hw/hppa/machine.c| 4 +++- > hw/ppc/prep.c| 4 +++- > 4 files changed, 10 insertions(+), 4 deletions(-) Reviewed-by: Thomas Huth
Re: [Qemu-devel] [PATCH v2 3/3] scsi: remove unused lsi53c895a_create() and lsi53c810_create() functions
On 2018-09-07 14:56, Mark Cave-Ayland wrote: > Now that these functions are no longer required they can be removed. > > Signed-off-by: Mark Cave-Ayland > --- > hw/scsi/lsi53c895a.c | 14 -- > include/hw/pci/pci.h | 2 -- > 2 files changed, 16 deletions(-) Reviewed-by: Thomas Huth
Re: [Qemu-devel] [PATCH v4 00/29] vhost-user for input & GPU
Hi
On Wed, Aug 29, 2018 at 4:00 PM Marc-André Lureau
wrote:
>
> Hi
>
> On Wed, Aug 29, 2018 at 11:50 AM, Daniel P. Berrangé
> wrote:
> > On Fri, Jul 13, 2018 at 03:08:47PM +0200, Marc-André Lureau wrote:
> >> Hi,
> >>
> >> vhost-user allows to drive a virtio device in a seperate
> >> process. After vhost-user-net, we have seen
> >> vhost-user-{scsi,blk,crypto} added more recently.
> >>
> >> This series, initially proposed 2 years ago
> >> (https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg01905.html)
> >> contributes with vhost-user-input and vhost-user-gpu.
> >>
> >> Additionally, to factor out common code and ease the usage, a
> >> vhost-user-backend is introduced as an intermediary object between the
> >> backend and the qemu device.
> >>
> >> You may start a vhost-user-gpu with virgl rendering in a separate
> >> process like this:
> >>
> >> $ ./vhost-user-gpu --virgl -s vgpu.sock &
> >> $ qemu...
> >> -chardev socket,id=chr,path=vgpu.sock
> >> -object vhost-user-backend,id=vug,chardev=chr
> >> -device vhost-user-vga,vhost-user=vug
> >>
> >> You may also specify the backend command and the arguments as part of
> >> vhost-user-backend qemu arguments. For example, to start a
> >> vhost-user-input backend on input device /dev/input/event19:
> >>
> >> -object vhost-user-backend,id=vuid,cmd="vhost-user-input
> >> /dev/input/event19"
> >> -device virtio-input-host-pci,vhost-user=vuid
> >>
> >> The vhost-user-gpu backend requires virgl from git.
> >>
> >> The libvirt support is on-going work:
> >> https://github.com/elmarco/libvirt/commits/vhost-user-gpu
> >>
> >> The GPU benchmarks are encouraging, giving up to x5 performance on
> >> Unigine Heaven 4.0.
> >
> > What is the main driving motivation behind this featureset ? Is it aimed
> > at providing performance, or security, or allowing arbitrary out of tree
> > backends, or all three ?
>
> Mainly security/stability & performance. Allowing arbitrary out of
> tree backends is a bonus for me, I don't care much if we don't allow
> it.
>
> > Although we've got a number of vhost-user backends I'm pretty concerned
> > about the direction this is taking QEMU overall.
> >
> > Managing QEMU is non-trivial for a number of reasons. We've done a lot of
> > work to provide standardized modelling of CLI args, guest ABI stability
> > via association with machine types, migration data stream stability,
> > QEMU feature capabilities detection and so on.
> >
> > The move to outsource backends to external binaries is discarding some
> > or all of these items, rewinding alot of progress we've made in the
> > managability of QEMU. Each external binary has to now reinvent the
> > things that are already solved in QEMU, and you can be sure each impl
> > will reinvent the concepts differently.
> >
> > I can't help feeling that we are shooting ourselves in the foot here
> > long term.
>
> I have a bit of the same feeling. For ex, I was a bit reluctant having
> the TPM emulator as an external process (new protocol, external
> binaries, with various management work, "opaque" migration). But in
> the end, the integration with libvirt makes thing quite easy for the
> user. So I changed a bit my mind, and I think this is one task that
> libvirt can be really good at.
>
> >
> > We've always rejected the idea of having loadable modules in QEMU, but
> > as a result we've end up with outsourcing the backend entirely via the
> > vhost-user framework, so the end result is even more opaque than if we
> > had loadable modules, and is unable to take advantage of our standardized
> > modelling frameworks & capabilities :-(
>
> I agree, that's one of the reason I put together libvhostuser in the
> first place. If qemu ships its own backend, there is no reason we
> don't share modelling & code etc.
>
> For ex, I hope more vhost-users will use the -object
> vhost-user-backend to do basic connection and virtio setup. (in the
> series, -gpu and -input, I didn't really investigate -crypto, -net,
> -blk etc)
>
> > If we just look at performance & security, and ignore 3rd party impls
> > for a minute, I really don't like that to gain perf + security we have
> > to change from:
> >
> > $ qemu...
> >-device virtio-vga
> >
> > To
> >
> > $ ./vhost-user-gpu --virgl -s vgpu.sock &
> > $ qemu...
> >-chardev socket,id=chr,path=vgpu.sock
> >-object vhost-user-backend,id=vug,chardev=chr
> >-device vhost-user-vga,vhost-user=vug
> >
>
> That's a bit incovenient for qemu command line users. But who runs
> qemu this way but some developers? (others have higher-level scripts /
> tools or libvirt)
>
> > Once we have the ability to run the backend via an external process,
> > to gain performance & security benefits, assuming feature parity is
> > achieved, I question why anyone would want to continue with the old
> > in-process approach ? IOW the goal should be that the original args
> >
> > $ qemu...
> >-device virtio-vga
> >
> > should "do the right thing" to launch the externa
[Qemu-devel] [PATCH] target/ppc/cpu-models: Re-group the 970 CPUs together again
The addition of the POWER9 CPUs divided the entries for the 970 CPUs,
which is a little bit confusing when you look at the code. So let's
re-group the 970 CPUs together again, and since these chips have been
based on the POWER4 processor, move them also in front of the POWER5
chips now.
Signed-off-by: Thomas Huth
---
target/ppc/cpu-models.c | 40 +++-
1 file changed, 19 insertions(+), 21 deletions(-)
diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c
index 6c9bfde..7c75963 100644
--- a/target/ppc/cpu-models.c
+++ b/target/ppc/cpu-models.c
@@ -741,26 +741,8 @@
"PowerPC 7457A v1.2 (G4)")
/* 64 bits PowerPC */
#if defined (TARGET_PPC64)
-POWERPC_DEF("power5+_v2.1", CPU_POWERPC_POWER5P_v21,POWER5P,
-"POWER5+ v2.1")
-POWERPC_DEF("power7_v2.3", CPU_POWERPC_POWER7_v23, POWER7,
-"POWER7 v2.3")
-POWERPC_DEF("power7+_v2.1", CPU_POWERPC_POWER7P_v21,POWER7,
-"POWER7+ v2.1")
-POWERPC_DEF("power8e_v2.1", CPU_POWERPC_POWER8E_v21,POWER8,
-"POWER8E v2.1")
-POWERPC_DEF("power8_v2.0", CPU_POWERPC_POWER8_v20, POWER8,
-"POWER8 v2.0")
-POWERPC_DEF("power8nvl_v1.0", CPU_POWERPC_POWER8NVL_v10, POWER8,
-"POWER8NVL v1.0")
POWERPC_DEF("970_v2.2", CPU_POWERPC_970_v22,970,
"PowerPC 970 v2.2")
-
-POWERPC_DEF("power9_v1.0", CPU_POWERPC_POWER9_DD1, POWER9,
-"POWER9 v1.0")
-POWERPC_DEF("power9_v2.0", CPU_POWERPC_POWER9_DD20,POWER9,
-"POWER9 v2.0")
-
POWERPC_DEF("970fx_v1.0",CPU_POWERPC_970FX_v10, 970,
"PowerPC 970FX v1.0 (G5)")
POWERPC_DEF("970fx_v2.0",CPU_POWERPC_970FX_v20, 970,
@@ -775,6 +757,22 @@
"PowerPC 970MP v1.0")
POWERPC_DEF("970mp_v1.1",CPU_POWERPC_970MP_v11, 970,
"PowerPC 970MP v1.1")
+POWERPC_DEF("power5+_v2.1", CPU_POWERPC_POWER5P_v21,POWER5P,
+"POWER5+ v2.1")
+POWERPC_DEF("power7_v2.3", CPU_POWERPC_POWER7_v23, POWER7,
+"POWER7 v2.3")
+POWERPC_DEF("power7+_v2.1", CPU_POWERPC_POWER7P_v21,POWER7,
+"POWER7+ v2.1")
+POWERPC_DEF("power8e_v2.1", CPU_POWERPC_POWER8E_v21,POWER8,
+"POWER8E v2.1")
+POWERPC_DEF("power8_v2.0", CPU_POWERPC_POWER8_v20, POWER8,
+"POWER8 v2.0")
+POWERPC_DEF("power8nvl_v1.0", CPU_POWERPC_POWER8NVL_v10, POWER8,
+"POWER8NVL v1.0")
+POWERPC_DEF("power9_v1.0", CPU_POWERPC_POWER9_DD1, POWER9,
+"POWER9 v1.0")
+POWERPC_DEF("power9_v2.0", CPU_POWERPC_POWER9_DD20,POWER9,
+"POWER9 v2.0")
#endif /* defined (TARGET_PPC64) */
/***/
@@ -940,6 +938,9 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
{ "7457a", "7457a_v1.2" },
{ "apollo7pm", "7457a_v1.0" },
#if defined(TARGET_PPC64)
+{ "970", "970_v2.2" },
+{ "970fx", "970fx_v3.1" },
+{ "970mp", "970mp_v1.1" },
{ "power5+", "power5+_v2.1" },
{ "power5gs", "power5+_v2.1" },
{ "power7", "power7_v2.3" },
@@ -948,9 +949,6 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
{ "power8", "power8_v2.0" },
{ "power8nvl", "power8nvl_v1.0" },
{ "power9", "power9_v2.0" },
-{ "970", "970_v2.2" },
-{ "970fx", "970fx_v3.1" },
-{ "970mp", "970mp_v1.1" },
#endif
/* Generic PowerPCs */
--
1.8.3.1
Re: [Qemu-devel] [RFC PATCH v2 4/7] tcg: add instrumenting module
Pavel Dovgalyuk writes:
> From: Pavel Dovgalyuk
>
> This is a samples of the instrumenting interface and implementation
> of some instruction tracing tasks.
>
> Signed-off-by: Pavel Dovgalyuk
> ---
> accel/tcg/translator.c|5 +
> include/qemu/instrument.h |7 +++
> plugins/helper.h |1 +
> plugins/plugins.c | 41 +
> 4 files changed, 54 insertions(+)
> create mode 100644 include/qemu/instrument.h
> create mode 100644 plugins/helper.h
>
> diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
> index 0f9dca9..48773ac 100644
> --- a/accel/tcg/translator.c
> +++ b/accel/tcg/translator.c
> @@ -17,6 +17,7 @@
> #include "exec/gen-icount.h"
> #include "exec/log.h"
> #include "exec/translator.h"
> +#include "qemu/instrument.h"
>
> /* Pairs with tcg_clear_temp_count.
> To be called by #TranslatorOps.{translate_insn,tb_stop} if
> @@ -89,6 +90,10 @@ void translator_loop(const TranslatorOps *ops,
> DisasContextBase *db,
> }
> }
>
> +if (plugins_need_before_insn(db->pc_next, cpu)) {
> +plugins_instrument_before_insn(db->pc_next, cpu);
> +}
> +
I don't really see the need for a plugin_need_foo call here. Can't we
just iterate over all plugins that provide a before_insn binding and
leave it at that?
If the plugin decides it doesn't want to do anything with this
particular instruction it can always bail early.
> /* Disassemble one instruction. The translate_insn hook should
> update db->pc_next and db->is_jmp to indicate what should be
> done next -- either exiting this loop or locate the start of
> diff --git a/include/qemu/instrument.h b/include/qemu/instrument.h
> new file mode 100644
> index 000..e8f279f
> --- /dev/null
> +++ b/include/qemu/instrument.h
> @@ -0,0 +1,7 @@
> +#ifndef INSTRUMENT_H
> +#define INSTRUMENT_H
> +
> +bool plugins_need_before_insn(target_ulong pc, CPUState *cpu);
> +void plugins_instrument_before_insn(target_ulong pc, CPUState *cpu);
Need empty inline cases for #ifndef CONFIG_PLUGINS
> +
> +#endif /* INSTRUMENT_H */
> diff --git a/plugins/helper.h b/plugins/helper.h
> new file mode 100644
> index 000..007b395
> --- /dev/null
> +++ b/plugins/helper.h
> @@ -0,0 +1 @@
> +DEF_HELPER_2(before_insn, void, tl, ptr)
I wonder if we should have an explicit DEF_HELPER_FLAGS_2. Looking at
the flags though:
/* call flags */
/* Helper does not read globals (either directly or through an exception). It
implies TCG_CALL_NO_WRITE_GLOBALS. */
#define TCG_CALL_NO_READ_GLOBALS0x0010
/* Helper does not write globals */
#define TCG_CALL_NO_WRITE_GLOBALS 0x0020
/* Helper can be safely suppressed if the return value is not used. */
#define TCG_CALL_NO_SIDE_EFFECTS0x0040
/* convenience version of most used call flags */
#define TCG_CALL_NO_RWG TCG_CALL_NO_READ_GLOBALS
#define TCG_CALL_NO_WG TCG_CALL_NO_WRITE_GLOBALS
#define TCG_CALL_NO_SE TCG_CALL_NO_SIDE_EFFECTS
#define TCG_CALL_NO_RWG_SE (TCG_CALL_NO_RWG | TCG_CALL_NO_SE)
#define TCG_CALL_NO_WG_SE (TCG_CALL_NO_WG | TCG_CALL_NO_SE)
I guess no flags means machine state is fully consistent before we make
the helper call?
> diff --git a/plugins/plugins.c b/plugins/plugins.c
> index eabc931..5a08e71 100644
> --- a/plugins/plugins.c
> +++ b/plugins/plugins.c
> @@ -1,8 +1,13 @@
> #include "qemu/osdep.h"
> #include "qemu-common.h"
> +#include "cpu.h"
> #include "qemu/error-report.h"
> #include "qemu/plugins.h"
> +#include "qemu/instrument.h"
> +#include "tcg/tcg.h"
> +#include "tcg/tcg-op.h"
> #include "qemu/queue.h"
> +#include "qemu/option.h"
> #include
>
> typedef bool (*PluginInitFunc)(const char *);
> @@ -80,6 +85,40 @@ void qemu_plugin_load(const char *filename, const char
> *args)
> return;
> }
>
> +bool plugins_need_before_insn(target_ulong pc, CPUState *cpu)
> +{
> +QemuPluginInfo *info;
> +QLIST_FOREACH(info, &qemu_plugins, next) {
> +if (info->needs_before_insn && info->needs_before_insn(pc, cpu)) {
> +return true;
> +}
> +}
> +
> +return false;
> +}
> +
> +void plugins_instrument_before_insn(target_ulong pc, CPUState *cpu)
> +{
> +TCGv t_pc = tcg_const_tl(pc);
> +TCGv_ptr t_cpu = tcg_const_ptr(cpu);
> +/* We will dispatch plugins' callbacks in our own helper below */
> +gen_helper_before_insn(t_pc, t_cpu);
> +tcg_temp_free(t_pc);
> +tcg_temp_free_ptr(t_cpu);
> +}
OK I see what is happening here - but I worry about pushing off all
plugins into a single helper call with a fairly fixed amount of
information.
Would it be better to expose the generate helper API and maybe a few TCG
constants to the plugin function itself. It could then either emit
additional TCG operations or call to the helper - and deal with the
logic about if it should or shouldn't in a single call rather than doing
the need_foo call
[Qemu-devel] [PATCH v2 1/4] Fix segmentation fault when qemu_signal_init fails
Currently, when qemu_signal_init() fails it only returns a non-zero
value but without propagating any Error. But its callers need a
non-null err when runs error_report_err(err), or else 0->msg occurs.
To avoid such segmentation fault, add a new Error parameter to make
the call trace to propagate the err to the final caller.
This patch also adds the omitted error handling when creating signalfd
pipe fails in qemu_signalfd_compat().
Signed-off-by: Fei Li
---
include/qemu/osdep.h | 2 +-
util/compatfd.c | 9 ++---
util/main-loop.c | 10 +-
3 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index a91068df0e..09ed85fcb8 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -421,7 +421,7 @@ struct qemu_signalfd_siginfo {
additional fields in the future) */
};
-int qemu_signalfd(const sigset_t *mask);
+int qemu_signalfd(const sigset_t *mask, Error **errp);
void sigaction_invoke(struct sigaction *action,
struct qemu_signalfd_siginfo *info);
#endif
diff --git a/util/compatfd.c b/util/compatfd.c
index 980bd33e52..d3ed890405 100644
--- a/util/compatfd.c
+++ b/util/compatfd.c
@@ -16,6 +16,7 @@
#include "qemu/osdep.h"
#include "qemu-common.h"
#include "qemu/thread.h"
+#include "qapi/error.h"
#include
@@ -65,7 +66,7 @@ static void *sigwait_compat(void *opaque)
}
}
-static int qemu_signalfd_compat(const sigset_t *mask)
+static int qemu_signalfd_compat(const sigset_t *mask, Error **errp)
{
struct sigfd_compat_info *info;
QemuThread thread;
@@ -73,11 +74,13 @@ static int qemu_signalfd_compat(const sigset_t *mask)
info = malloc(sizeof(*info));
if (info == NULL) {
+error_setg(errp, "Failed to allocate signalfd memory");
errno = ENOMEM;
return -1;
}
if (pipe(fds) == -1) {
+error_setg(errp, "Failed to create signalfd pipe");
free(info);
return -1;
}
@@ -94,7 +97,7 @@ static int qemu_signalfd_compat(const sigset_t *mask)
return fds[0];
}
-int qemu_signalfd(const sigset_t *mask)
+int qemu_signalfd(const sigset_t *mask, Error **errp)
{
#if defined(CONFIG_SIGNALFD)
int ret;
@@ -106,5 +109,5 @@ int qemu_signalfd(const sigset_t *mask)
}
#endif
-return qemu_signalfd_compat(mask);
+return qemu_signalfd_compat(mask, errp);
}
diff --git a/util/main-loop.c b/util/main-loop.c
index affe0403c5..22aa2b1007 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -71,7 +71,7 @@ static void sigfd_handler(void *opaque)
}
}
-static int qemu_signal_init(void)
+static int qemu_signal_init(Error **errp)
{
int sigfd;
sigset_t set;
@@ -94,10 +94,10 @@ static int qemu_signal_init(void)
pthread_sigmask(SIG_BLOCK, &set, NULL);
sigdelset(&set, SIG_IPI);
-sigfd = qemu_signalfd(&set);
+sigfd = qemu_signalfd(&set, errp);
if (sigfd == -1) {
fprintf(stderr, "failed to create signalfd\n");
-return -errno;
+return -1;
}
fcntl_setfl(sigfd, O_NONBLOCK);
@@ -109,7 +109,7 @@ static int qemu_signal_init(void)
#else /* _WIN32 */
-static int qemu_signal_init(void)
+static int qemu_signal_init(Error **errp)
{
return 0;
}
@@ -148,7 +148,7 @@ int qemu_init_main_loop(Error **errp)
init_clocks(qemu_timer_notify_cb);
-ret = qemu_signal_init();
+ret = qemu_signal_init(errp);
if (ret) {
return ret;
}
--
2.13.7
[Qemu-devel] [PATCH v2 0/4] qemu_thread_create: propagate errors to callers to check
Hi, This idea comes from BiteSizedTasks, and this patch series implement the error checking of qemu_thread_create: make qemu_thread_create return a flag to indicate if it succeeded rather than failing with an error; make all callers check it. The first three patches apply to those call traces whose further callers already have the *errp to pass, thus add a new Error paramater in the call trace to propagate the error and let the further caller check it. The last patch modifies the qemu_thread_create() and makes it return a bool to all direct callers to indicate if it succeeds. v2: - Pass errp straightly instead of using a local_err & error_propagate - Return a bool: false/true to indicate if one function succeeds - Merge v1's last two patches into one to avoid the compling error - Fix one omitted error in patch1 and update some error messages Please help to review. Thanks. Fei Li (4): Fix segmentation fault when qemu_signal_init fails ui/vnc.c: polish vnc_init_func qemu_init_vcpu: add a new Error parameter to propagate qemu_thread_create: propagate the error to callers to handle accel/tcg/user-exec-stub.c | 2 +- cpus.c | 79 ++--- dump.c | 6 ++-- hw/misc/edu.c | 6 ++-- hw/ppc/spapr_hcall.c| 9 +++-- hw/rdma/rdma_backend.c | 3 +- hw/usb/ccid-card-emulated.c | 13 --- include/qemu/osdep.h| 2 +- include/qemu/thread.h | 4 +-- include/qom/cpu.h | 2 +- include/ui/console.h| 2 +- io/task.c | 3 +- iothread.c | 16 ++--- migration/migration.c | 49 + migration/postcopy-ram.c| 12 +-- migration/ram.c | 40 +++-- migration/savevm.c | 11 -- target/alpha/cpu.c | 4 ++- target/arm/cpu.c| 4 ++- target/cris/cpu.c | 4 ++- target/hppa/cpu.c | 4 ++- target/i386/cpu.c | 4 ++- target/lm32/cpu.c | 4 ++- target/m68k/cpu.c | 4 ++- target/microblaze/cpu.c | 4 ++- target/mips/cpu.c | 4 ++- target/moxie/cpu.c | 4 ++- target/nios2/cpu.c | 4 ++- target/openrisc/cpu.c | 4 ++- target/ppc/translate_init.inc.c | 4 ++- target/riscv/cpu.c | 4 ++- target/s390x/cpu.c | 4 ++- target/sh4/cpu.c| 4 ++- target/sparc/cpu.c | 4 ++- target/tilegx/cpu.c | 4 ++- target/tricore/cpu.c| 4 ++- target/unicore32/cpu.c | 4 ++- target/xtensa/cpu.c | 4 ++- tests/atomic_add-bench.c| 3 +- tests/iothread.c| 2 +- tests/qht-bench.c | 3 +- tests/rcutorture.c | 3 +- tests/test-aio.c| 2 +- tests/test-rcu-list.c | 3 +- ui/vnc-jobs.c | 17 ++--- ui/vnc-jobs.h | 2 +- ui/vnc.c| 12 +-- util/compatfd.c | 16 ++--- util/main-loop.c| 10 +++--- util/oslib-posix.c | 12 +-- util/qemu-thread-posix.c| 18 ++ util/qemu-thread-win32.c| 15 +--- util/rcu.c | 3 +- util/thread-pool.c | 4 ++- 54 files changed, 325 insertions(+), 143 deletions(-) -- 2.13.7
[Qemu-devel] [PATCH v2 3/4] qemu_init_vcpu: add a new Error parameter to propagate
The caller of qemu_init_vcpu() already passed the **errp to handle
errors. In view of this, add a new Error parameter to the following
call trace to propagate the error and let the further caller check it.
Besides, make qemu_init_vcpu() return a Boolean value to let its
callers know whether it succeeds.
Signed-off-by: Fei Li
---
accel/tcg/user-exec-stub.c | 2 +-
cpus.c | 34 +-
include/qom/cpu.h | 2 +-
target/alpha/cpu.c | 4 +++-
target/arm/cpu.c| 4 +++-
target/cris/cpu.c | 4 +++-
target/hppa/cpu.c | 4 +++-
target/i386/cpu.c | 4 +++-
target/lm32/cpu.c | 4 +++-
target/m68k/cpu.c | 4 +++-
target/microblaze/cpu.c | 4 +++-
target/mips/cpu.c | 4 +++-
target/moxie/cpu.c | 4 +++-
target/nios2/cpu.c | 4 +++-
target/openrisc/cpu.c | 4 +++-
target/ppc/translate_init.inc.c | 4 +++-
target/riscv/cpu.c | 4 +++-
target/s390x/cpu.c | 4 +++-
target/sh4/cpu.c| 4 +++-
target/sparc/cpu.c | 4 +++-
target/tilegx/cpu.c | 4 +++-
target/tricore/cpu.c| 4 +++-
target/unicore32/cpu.c | 4 +++-
target/xtensa/cpu.c | 4 +++-
24 files changed, 86 insertions(+), 36 deletions(-)
diff --git a/accel/tcg/user-exec-stub.c b/accel/tcg/user-exec-stub.c
index a32b4496af..38f6b928d4 100644
--- a/accel/tcg/user-exec-stub.c
+++ b/accel/tcg/user-exec-stub.c
@@ -10,7 +10,7 @@ void cpu_resume(CPUState *cpu)
{
}
-void qemu_init_vcpu(CPUState *cpu)
+bool qemu_init_vcpu(CPUState *cpu, Error **errp)
{
}
diff --git a/cpus.c b/cpus.c
index 8ee6e5db93..1feb308123 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1898,7 +1898,7 @@ void cpu_remove_sync(CPUState *cpu)
/* For temporary buffers for forming a name */
#define VCPU_THREAD_NAME_SIZE 16
-static void qemu_tcg_init_vcpu(CPUState *cpu)
+static void qemu_tcg_init_vcpu(CPUState *cpu, Error **errp)
{
char thread_name[VCPU_THREAD_NAME_SIZE];
static QemuCond *single_tcg_halt_cond;
@@ -1954,7 +1954,7 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
}
}
-static void qemu_hax_start_vcpu(CPUState *cpu)
+static void qemu_hax_start_vcpu(CPUState *cpu, Error **errp)
{
char thread_name[VCPU_THREAD_NAME_SIZE];
@@ -1971,7 +1971,7 @@ static void qemu_hax_start_vcpu(CPUState *cpu)
#endif
}
-static void qemu_kvm_start_vcpu(CPUState *cpu)
+static void qemu_kvm_start_vcpu(CPUState *cpu, Error **errp)
{
char thread_name[VCPU_THREAD_NAME_SIZE];
@@ -1984,7 +1984,7 @@ static void qemu_kvm_start_vcpu(CPUState *cpu)
cpu, QEMU_THREAD_JOINABLE);
}
-static void qemu_hvf_start_vcpu(CPUState *cpu)
+static void qemu_hvf_start_vcpu(CPUState *cpu, Error **errp)
{
char thread_name[VCPU_THREAD_NAME_SIZE];
@@ -2002,7 +2002,7 @@ static void qemu_hvf_start_vcpu(CPUState *cpu)
cpu, QEMU_THREAD_JOINABLE);
}
-static void qemu_whpx_start_vcpu(CPUState *cpu)
+static void qemu_whpx_start_vcpu(CPUState *cpu, Error **errp)
{
char thread_name[VCPU_THREAD_NAME_SIZE];
@@ -2018,7 +2018,7 @@ static void qemu_whpx_start_vcpu(CPUState *cpu)
#endif
}
-static void qemu_dummy_start_vcpu(CPUState *cpu)
+static void qemu_dummy_start_vcpu(CPUState *cpu, Error **errp)
{
char thread_name[VCPU_THREAD_NAME_SIZE];
@@ -2031,11 +2031,12 @@ static void qemu_dummy_start_vcpu(CPUState *cpu)
QEMU_THREAD_JOINABLE);
}
-void qemu_init_vcpu(CPUState *cpu)
+bool qemu_init_vcpu(CPUState *cpu, Error **errp)
{
cpu->nr_cores = smp_cores;
cpu->nr_threads = smp_threads;
cpu->stopped = true;
+Error *local_err = NULL;
if (!cpu->as) {
/* If the target cpu hasn't set up any address spaces itself,
@@ -2046,22 +2047,29 @@ void qemu_init_vcpu(CPUState *cpu)
}
if (kvm_enabled()) {
-qemu_kvm_start_vcpu(cpu);
+qemu_kvm_start_vcpu(cpu, &local_err);
} else if (hax_enabled()) {
-qemu_hax_start_vcpu(cpu);
+qemu_hax_start_vcpu(cpu, &local_err);
} else if (hvf_enabled()) {
-qemu_hvf_start_vcpu(cpu);
+qemu_hvf_start_vcpu(cpu, &local_err);
} else if (tcg_enabled()) {
-qemu_tcg_init_vcpu(cpu);
+qemu_tcg_init_vcpu(cpu, &local_err);
} else if (whpx_enabled()) {
-qemu_whpx_start_vcpu(cpu);
+qemu_whpx_start_vcpu(cpu, &local_err);
} else {
-qemu_dummy_start_vcpu(cpu);
+qemu_dummy_start_vcpu(cpu, &local_err);
+}
+
+if (local_err) {
+error_propagate(errp, local_err);
+return false;
}
while (!cpu->created) {
qemu_cond_wait(&qemu_cpu_cond, &qemu_global_mutex);
}
+
+return true;
}
void cpu_stop_current(void)
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index dc130c
[Qemu-devel] [PATCH v2 2/4] ui/vnc.c: polish vnc_init_func
Add a new Error parameter for vnc_display_init() to handle errors
in its caller: vnc_init_func(), just like vnc_display_open() does.
And let the call trace propagate the Error.
Besides, make vnc_start_worker_thread() return a bool to indicate
whether it succeeds instead of returning nothing.
Signed-off-by: Fei Li
---
include/ui/console.h | 2 +-
ui/vnc-jobs.c| 9 ++---
ui/vnc-jobs.h| 2 +-
ui/vnc.c | 12 +---
4 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/include/ui/console.h b/include/ui/console.h
index fb969caf70..c17803c530 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -453,7 +453,7 @@ void qemu_display_early_init(DisplayOptions *opts);
void qemu_display_init(DisplayState *ds, DisplayOptions *opts);
/* vnc.c */
-void vnc_display_init(const char *id);
+void vnc_display_init(const char *id, Error **errp);
void vnc_display_open(const char *id, Error **errp);
void vnc_display_add_client(const char *id, int csock, bool skipauth);
int vnc_display_password(const char *id, const char *password);
diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
index 929391f85d..8807d7217c 100644
--- a/ui/vnc-jobs.c
+++ b/ui/vnc-jobs.c
@@ -331,15 +331,18 @@ static bool vnc_worker_thread_running(void)
return queue; /* Check global queue */
}
-void vnc_start_worker_thread(void)
+bool vnc_start_worker_thread(Error **errp)
{
VncJobQueue *q;
-if (vnc_worker_thread_running())
-return ;
+if (vnc_worker_thread_running()) {
+goto out;
+}
q = vnc_queue_init();
qemu_thread_create(&q->thread, "vnc_worker", vnc_worker_thread, q,
QEMU_THREAD_DETACHED);
queue = q; /* Set global queue */
+out:
+return true;
}
diff --git a/ui/vnc-jobs.h b/ui/vnc-jobs.h
index 59f66bcc35..14640593db 100644
--- a/ui/vnc-jobs.h
+++ b/ui/vnc-jobs.h
@@ -37,7 +37,7 @@ void vnc_job_push(VncJob *job);
void vnc_jobs_join(VncState *vs);
void vnc_jobs_consume_buffer(VncState *vs);
-void vnc_start_worker_thread(void);
+bool vnc_start_worker_thread(Error **errp);
/* Locks */
static inline int vnc_trylock_display(VncDisplay *vd)
diff --git a/ui/vnc.c b/ui/vnc.c
index ccb1335d86..f632635ea4 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3206,7 +3206,7 @@ static const DisplayChangeListenerOps dcl_ops = {
.dpy_cursor_define= vnc_dpy_cursor_define,
};
-void vnc_display_init(const char *id)
+void vnc_display_init(const char *id, Error **errp)
{
VncDisplay *vd;
@@ -3236,7 +3236,9 @@ void vnc_display_init(const char *id)
vd->connections_limit = 32;
qemu_mutex_init(&vd->mutex);
-vnc_start_worker_thread();
+if (!vnc_start_worker_thread(errp)) {
+return;
+}
vd->dcl.ops = &dcl_ops;
register_displaychangelistener(&vd->dcl);
@@ -4079,7 +4081,11 @@ int vnc_init_func(void *opaque, QemuOpts *opts, Error
**errp)
char *id = (char *)qemu_opts_id(opts);
assert(id);
-vnc_display_init(id);
+vnc_display_init(id, &local_err);
+if (local_err) {
+error_reportf_err(local_err, "Failed to init VNC server: ");
+exit(1);
+}
vnc_display_open(id, &local_err);
if (local_err != NULL) {
error_reportf_err(local_err, "Failed to start VNC server: ");
--
2.13.7
Re: [Qemu-devel] [PATCH v2 03/12] qemu-option: add help fallback to print the list of options
On 09/07/2018 02:59 AM, Marc-André Lureau wrote: QDev options accept 'help' (or '?', but that's problematic with shell globing) in the list of parameters, which is handy to list the s/globing/globbing/ available options. Unfortunately, this isn't built in QemuOpts. qemu_opts_parse_noisily() seems to be the common path for command line options, so place a fallback to print help, listing the available options. This is quite handy, for example with qemu "-spice help". Signed-off-by: Marc-André Lureau Reviewed-by: Eric Blake --- util/qemu-option.c | 33 ++--- 1 file changed, 22 insertions(+), 11 deletions(-) R-b still stands. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH V9 2/4] tests/migration: Support cross compilation in generating boot header file
On 09/07/2018 02:04 AM, Andrew Jones wrote:
> On Thu, Sep 06, 2018 at 12:23:45PM -0400, Wei Huang wrote:
>>
>>
>> - Original Message -
>>> From: "Andrew Jones"
>>> To: "Wei Huang"
>>> Cc: lviv...@redhat.com, "peter maydell" ,
>>> quint...@redhat.com, qemu-devel@nongnu.org,
>>> dgilb...@redhat.com, "alex bennee"
>>> Sent: Thursday, September 6, 2018 9:00:33 AM
>>> Subject: Re: [Qemu-devel] [PATCH V9 2/4] tests/migration: Support cross
>>> compilation in generating boot header file
>>>
>>> On Thu, Sep 06, 2018 at 09:37:04AM -0400, Wei Huang wrote:
- Original Message -
> From: "Andrew Jones"
> To: "Wei Huang"
> Cc: qemu-devel@nongnu.org, lviv...@redhat.com, "peter maydell"
> , quint...@redhat.com,
> dgilb...@redhat.com, "alex bennee"
> Sent: Thursday, September 6, 2018 7:03:32 AM
> Subject: Re: [Qemu-devel] [PATCH V9 2/4] tests/migration: Support cross
> compilation in generating boot header file
>
> On Wed, Sep 05, 2018 at 03:15:32PM -0400, Wei Huang wrote:
>> Recently a new configure option, CROSS_CC_GUEST, was added to
>> $(TARGET)-softmmu/config-target.mak to support TCG-related tests. This
>> patch tries to leverage this option to support cross compilation when
>> the
>> migration boot block file is being re-generated:
>>
>> * The x86 related files are moved to a new sub-dir (named ./i386).
>> * A new top-layer Makefile is created in tests/migration/ directory.
>>This Makefile searches and parses CROSS_CC_GUEST to generate
>>CROSS_PREFIX.
>>The CROSS_PREFIX, if available, is then passed to
>>migration/$ARCH/Makefile.
>>
>> Reviewed-by: Juan Quintela
>> Signed-off-by: Wei Huang
>> ---
>> tests/migration-test.c | 2 +-
>> tests/migration/Makefile | 44
>> --
>> tests/migration/i386/Makefile | 22 +++
>> .../{x86-a-b-bootblock.S => i386/a-b-bootblock.S} | 4 --
>> .../{x86-a-b-bootblock.h => i386/a-b-bootblock.h} | 8 ++--
>> 5 files changed, 51 insertions(+), 29 deletions(-)
>> create mode 100644 tests/migration/i386/Makefile
>> rename tests/migration/{x86-a-b-bootblock.S => i386/a-b-bootblock.S}
>> (93%)
>> rename tests/migration/{x86-a-b-bootblock.h => i386/a-b-bootblock.h}
>> (92%)
>>
>> diff --git a/tests/migration-test.c b/tests/migration-test.c
>> index 0e687b7..fe6b41a 100644
>> --- a/tests/migration-test.c
>> +++ b/tests/migration-test.c
>> @@ -83,7 +83,7 @@ static const char *tmpfs;
>> /* A simple PC boot sector that modifies memory (1-100MB) quickly
>> * outputting a 'B' every so often if it's still running.
>> */
>> -#include "tests/migration/x86-a-b-bootblock.h"
>> +#include "tests/migration/i386/a-b-bootblock.h"
>>
>> static void init_bootfile_x86(const char *bootpath)
>> {
>> diff --git a/tests/migration/Makefile b/tests/migration/Makefile
>> index c0824b4..a9ed875 100644
>> --- a/tests/migration/Makefile
>> +++ b/tests/migration/Makefile
>> @@ -1,31 +1,35 @@
>> -# To specify cross compiler prefix, use CROSS_PREFIX=
>> -# $ make CROSS_PREFIX=x86_64-linux-gnu-
>> +#
>> +# Copyright (c) 2018 Red Hat, Inc. and/or its affiliates
>> +#
>> +# This work is licensed under the terms of the GNU GPL, version 2 or
>> later.
>> +# See the COPYING file in the top-level directory.
>> +#
>> +
>> +TARGET_LIST = i386
>> +
>> +SRC_PATH = ../..
>>
>> override define __note
>> -/* This file is automatically generated from
>> - * tests/migration/x86-a-b-bootblock.S, edit that and then run
>> - * tests/migration/rebuild-x86-bootblock.sh to update,
>> - * and then remember to send both in your patch submission.
>> +/* This file is automatically generated from the assembly file in
>> + * tests/migration/$@. Edit that file and then run "make all"
>> + * inside tests/migration to update, and then remember to send both
>> + * the header and the assembler differences in your patch submission.
>> */
>> endef
>> export __note
>>
>> -.PHONY: all clean
>> -all: x86-a-b-bootblock.h
>> -
>> -x86-a-b-bootblock.h: x86.bootsect
>> -echo "$$__note" > header.tmp
>> -xxd -i $< | sed -e 's/.*int.*//' >> header.tmp
>> -mv header.tmp $@
>> +find-arch-cross-cc = $(lastword $(shell grep -h "CROSS_CC_GUEST="
>> $(wildcard $(SRC_PATH)/$(patsubst
>> i386,*86*,$(1))-softmmu/config-target.mak)))
>
> The above function hangs unless configuring with
> '--target-list=x86_64-softmmu,aarch64-softmmu'. I tried just x86_64
> alone,
> just aarch64 alone, and also configuring both x86_64 and i386, but none
> of those worked. For some reason g
[Qemu-devel] [PATCH v2 4/4] qemu_thread_create: propagate the error to callers to handle
Make qemu_thread_create() return a Boolean to indicate if it succeeds
rather than failing with an error. And add an Error parameter to hold
the error message and let the callers handle it.
Besides, directly return if thread->data is NULL to avoid the
segmentation fault in qemu_thread_join in qemu-thread-win32.c.
Signed-off-by: Fei Li
---
cpus.c | 45 +++--
dump.c | 6 --
hw/misc/edu.c | 6 --
hw/ppc/spapr_hcall.c| 9 +++--
hw/rdma/rdma_backend.c | 3 ++-
hw/usb/ccid-card-emulated.c | 13
include/qemu/thread.h | 4 ++--
io/task.c | 3 ++-
iothread.c | 16 ++-
migration/migration.c | 49 +++--
migration/postcopy-ram.c| 12 +--
migration/ram.c | 40 +++-
migration/savevm.c | 11 +++---
tests/atomic_add-bench.c| 3 ++-
tests/iothread.c| 2 +-
tests/qht-bench.c | 3 ++-
tests/rcutorture.c | 3 ++-
tests/test-aio.c| 2 +-
tests/test-rcu-list.c | 3 ++-
ui/vnc-jobs.c | 8 ++--
util/compatfd.c | 7 +--
util/oslib-posix.c | 12 ---
util/qemu-thread-posix.c| 18 +++--
util/qemu-thread-win32.c| 15 +-
util/rcu.c | 3 ++-
util/thread-pool.c | 4 +++-
26 files changed, 210 insertions(+), 90 deletions(-)
diff --git a/cpus.c b/cpus.c
index 1feb308123..40db5c378f 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1928,15 +1928,20 @@ static void qemu_tcg_init_vcpu(CPUState *cpu, Error
**errp)
snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/TCG",
cpu->cpu_index);
-qemu_thread_create(cpu->thread, thread_name,
qemu_tcg_cpu_thread_fn,
- cpu, QEMU_THREAD_JOINABLE);
+if (!qemu_thread_create(cpu->thread, thread_name,
+qemu_tcg_cpu_thread_fn, cpu,
+QEMU_THREAD_JOINABLE, errp)) {
+return;
+}
} else {
/* share a single thread for all cpus with TCG */
snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "ALL CPUs/TCG");
-qemu_thread_create(cpu->thread, thread_name,
- qemu_tcg_rr_cpu_thread_fn,
- cpu, QEMU_THREAD_JOINABLE);
+if (!qemu_thread_create(cpu->thread, thread_name,
+qemu_tcg_rr_cpu_thread_fn, cpu,
+QEMU_THREAD_JOINABLE, errp)) {
+return;
+}
single_tcg_halt_cond = cpu->halt_cond;
single_tcg_cpu_thread = cpu->thread;
@@ -1964,8 +1969,10 @@ static void qemu_hax_start_vcpu(CPUState *cpu, Error
**errp)
snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/HAX",
cpu->cpu_index);
-qemu_thread_create(cpu->thread, thread_name, qemu_hax_cpu_thread_fn,
- cpu, QEMU_THREAD_JOINABLE);
+if (!qemu_thread_create(cpu->thread, thread_name, qemu_hax_cpu_thread_fn,
+cpu, QEMU_THREAD_JOINABLE, errp)) {
+return;
+}
#ifdef _WIN32
cpu->hThread = qemu_thread_get_handle(cpu->thread);
#endif
@@ -1980,8 +1987,10 @@ static void qemu_kvm_start_vcpu(CPUState *cpu, Error
**errp)
qemu_cond_init(cpu->halt_cond);
snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/KVM",
cpu->cpu_index);
-qemu_thread_create(cpu->thread, thread_name, qemu_kvm_cpu_thread_fn,
- cpu, QEMU_THREAD_JOINABLE);
+if (!qemu_thread_create(cpu->thread, thread_name, qemu_kvm_cpu_thread_fn,
+cpu, QEMU_THREAD_JOINABLE, errp)) {
+return;
+}
}
static void qemu_hvf_start_vcpu(CPUState *cpu, Error **errp)
@@ -1998,8 +2007,10 @@ static void qemu_hvf_start_vcpu(CPUState *cpu, Error
**errp)
snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/HVF",
cpu->cpu_index);
-qemu_thread_create(cpu->thread, thread_name, qemu_hvf_cpu_thread_fn,
- cpu, QEMU_THREAD_JOINABLE);
+if (!qemu_thread_create(cpu->thread, thread_name, qemu_hvf_cpu_thread_fn,
+cpu, QEMU_THREAD_JOINABLE, errp)) {
+return;
+}
}
static void qemu_whpx_start_vcpu(CPUState *cpu, Error **errp)
@@ -2011,8 +2022,10 @@ static void qemu_whpx_start_vcpu(CPUState *cpu, Error
**errp)
qemu_cond_init(cpu->halt_cond);
snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/WHPX",
cpu->cpu_index);
-qemu_thread_create(cpu->thread, thread_name, qemu_whpx_cpu_thread_fn,
- cpu, QEMU_THREAD_JOINABLE);
+if (!qemu_thread_create(cpu->thread,
Re: [Qemu-devel] [PULL 01/14] ppc: Remove deprecated ppcemb target
On 09/07/2018 02:31 AM, David Gibson wrote: From: Thomas Huth There is no known available OS for ppc around anymore that uses page sizes below 4k, so it does not make much sense that we keep wasting our time on building and testing the ppcemb-softmmu target. It has been deprecated since two releases, and nobody complained, so let's remove this now. Signed-off-by: Thomas Huth Signed-off-by: David Gibson --- +++ b/qapi/common.json @@ -146,6 +146,6 @@ 'data' : [ 'aarch64', 'alpha', 'arm', 'cris', 'hppa', 'i386', 'lm32', 'm68k', 'microblaze', 'microblazeel', 'mips', 'mips64', 'mips64el', 'mipsel', 'moxie', 'nios2', 'or1k', 'ppc', - 'ppc64', 'ppcemb', 'riscv32', 'riscv64', 's390x', 'sh4', + 'ppc64', 'riscv32', 'riscv64', 's390x', 'sh4', 'sh4eb', 'sparc', 'sparc64', 'tricore', 'unicore32', 'x86_64', 'xtensa', 'xtensaeb' ] } Can we also get documentation that this was deleted (probably as a followup, since I'm only now noticing this in the pull request)? See QKeyCode in qapi/ui.json for an example of documenting 'altgr: dropped in 2.10' -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH v2 01/12] qdev-monitor: print help to stdout
On 09/07/2018 02:59 AM, Marc-André Lureau wrote:
qdev_device_help() is used from command line "-device help", or from
HMP "device_add". If used from command line, print help to stdout
(it is only printed on explicit demand).
Signed-off-by: Marc-André Lureau
---
include/monitor/monitor.h | 2 ++
monitor.c | 16 +---
qdev-monitor.c| 32 +++-
3 files changed, 34 insertions(+), 16 deletions(-)
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 2ef5e04b37..e7667fda35 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -47,4 +47,6 @@ int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd);
void monitor_fdset_dup_fd_remove(int dup_fd);
int monitor_fdset_dup_fd_find(int dup_fd);
+void out_vprintf(FILE *stream, const char *fmt, va_list ap) GCC_FMT_ATTR(2, 0);
Perhaps name this monitor_vfprintf()? (That would match the fact that
you are using it like normal vfprintf).
+++ b/qdev-monitor.c
@@ -104,22 +104,31 @@ static bool qdev_class_has_alias(DeviceClass *dc)
return (qdev_class_get_alias(dc) != NULL);
}
+static void out_printf(const char *fmt, ...)
+{
+va_list ap;
+
+va_start(ap, fmt);
+out_vprintf(stdout, fmt, ap);
+va_end(ap);
+}
But this name seems reasonable.
And I just now see that Thomas also complained about the monitor.h
naming, but I like 'monitor_vfprintf' better than his 'mon_file_vprintf'.
If an improved name is your only change,
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [RFC PATCH v2 5/7] plugins: add plugin template
Pavel Dovgalyuk writes:
> From: Pavel Dovgalyuk
>
> This is a template of the QEMU plugin. It includes empty functions that
> plugins may implement.
>
I'm not sure it's worth having a null-template plugin if we can have one
or two well documented example plugins. It just runs the risk of
bitrot
> Signed-off-by: Pavel Dovgalyuk
> ---
> plugins/template/Makefile | 19 +++
> plugins/template/template.c | 19 +++
> 2 files changed, 38 insertions(+)
> create mode 100644 plugins/template/Makefile
> create mode 100644 plugins/template/template.c
>
> diff --git a/plugins/template/Makefile b/plugins/template/Makefile
> new file mode 100644
> index 000..b9d10da
> --- /dev/null
> +++ b/plugins/template/Makefile
> @@ -0,0 +1,19 @@
> +CFLAGS += -I../include -fno-PIE -fPIC -O3
> +LDFLAGS += -shared
> +# TODO: Windows
> +DSOSUF := .so
> +
> +NAME:= template
> +BIN := $(NAME)$(DSOSUF)
> +
> +FILES := template.o
> +
> +%.o: %.c
> + $(CC) -c -o $@ $< $(CFLAGS)
> +
> +all: $(FILES)
> + $(CC) $(LDFLAGS) -o $(BIN) $(FILES)
> +
> +clean:
> + rm $(FILES)
> + rm $(BIN)
> diff --git a/plugins/template/template.c b/plugins/template/template.c
> new file mode 100644
> index 000..fed1053
> --- /dev/null
> +++ b/plugins/template/template.c
> @@ -0,0 +1,19 @@
> +#include
> +#include
> +#include "plugins.h"
> +
> +bool plugin_init(const char *args)
> +{
> +printf("template plugin loaded successfully\n");
> +return true;
> +}
> +
> +bool plugin_needs_before_insn(uint64_t pc, void *cpu)
> +{
> +return true;
> +}
> +
> +void plugin_before_insn(uint64_t pc, void *cpu)
> +{
> +printf("executing instruction at %lx\n", pc);
> +}
--
Alex Bennée
Re: [Qemu-devel] [PATCH v2 12/12] vl: list user creatable properties when 'help' is argument
On 09/07/2018 02:59 AM, Marc-André Lureau wrote: Iterate over the writable class properties, sort and print them out with the description if available. Ex: qemu -object memory-backend-file,help memory-backend-file.align=int memory-backend-file.discard-data=bool memory-backend-file.dump=bool - Set to 'off' to exclude from core dump memory-backend-file.host-nodes=int - Binds memory to the list of NUMA host nodes memory-backend-file.mem-path=string memory-backend-file.merge=bool - Mark memory as mergeable memory-backend-file.pmem=bool memory-backend-file.policy=HostMemPolicy - Set the NUMA policy memory-backend-file.prealloc=bool - Preallocate memory memory-backend-file.share=bool - Mark the memory as private to QEMU or shared memory-backend-file.size=int - Size of the memory region (ex: 500M) Signed-off-by: Marc-André Lureau --- qom/object_interfaces.c | 6 +++--- vl.c| 40 +--- 2 files changed, 40 insertions(+), 6 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [RFC PATCH v2 6/7] plugin: add instruction execution logger
Pavel Dovgalyuk writes:
> From: Pavel Dovgalyuk
>
> This patch adds a plugin for logging addresses of all executed instructions,
> making a complete instruction-level trace.
This isn't a good example. You can do this now with a much simpler:
${QEMU} -singlestep -d nochain,trace:exec_tb -D $trace ${BINARY}
Or even with a binary log:
${QEMU} -singlestep -d nochain -trace enable=exec_tb,file=$trace ${BINARY}
Which is all currently built-in. For the example to be worthwhile we
need to show how we can do something we currently can't do with the
existing infrastructure.
Perhaps a better example would be logging each PC execution to a hash
table so we can compute the hottest PC?
However that is going to require another API to allow information to be
exported from the plugin itself to report it's results.
>
> Signed-off-by: Pavel Dovgalyuk
> ---
> plugins/exec-log/Makefile | 19 +++
> plugins/exec-log/exec-log.c | 18 ++
> 2 files changed, 37 insertions(+)
> create mode 100644 plugins/exec-log/Makefile
> create mode 100644 plugins/exec-log/exec-log.c
>
> diff --git a/plugins/exec-log/Makefile b/plugins/exec-log/Makefile
> new file mode 100644
> index 000..86374f4
> --- /dev/null
> +++ b/plugins/exec-log/Makefile
> @@ -0,0 +1,19 @@
> +CFLAGS += -I../include -fno-PIE -fPIC -O3
I would have:
QEMU_SRC=../..
CFLAGS += -I$(QEMU_SRC)/include -fno-PIE -fPIC -O3
to make it clearer for out of tree plugins.
> +LDFLAGS += -shared
> +# TODO: Windows
> +DSOSUF := .so
> +
> +NAME:= exec-log
> +BIN := $(NAME)$(DSOSUF)
> +
> +FILES := exec-log.o
> +
> +%.o: %.c
> + $(CC) -c -o $@ $< $(CFLAGS)
> +
> +all: $(FILES)
> + $(CC) $(LDFLAGS) -o $(BIN) $(FILES)
> +
> +clean:
> + rm $(FILES)
> + rm $(BIN)
If the example plugins are going to sit in the main tree we should build
them (and ideally test they load/work during make check/tcg-check).
> diff --git a/plugins/exec-log/exec-log.c b/plugins/exec-log/exec-log.c
> new file mode 100644
> index 000..7fc7975
> --- /dev/null
> +++ b/plugins/exec-log/exec-log.c
> @@ -0,0 +1,18 @@
> +#include
> +#include
> +#include "plugins.h"
> +
> +bool plugin_init(const char *args)
> +{
> +return true;
> +}
> +
> +bool plugin_needs_before_insn(uint64_t pc, void *cpu)
> +{
> +return true;
> +}
> +
> +void plugin_before_insn(uint64_t pc, void *cpu)
> +{
> +qemulib_log("executing instruction at %lx\n", pc);
> +}
--
Alex Bennée
Re: [Qemu-devel] [Bug 1716292] Re: User mode emulation returns wrong value for write(fd, NULL, 0)
On 09/07/2018 06:51 AM, Tony Garnock-Jones wrote: ** Patch added: "0001-Bring-linux-user-write-2-handling-into-line-with-lin.patch" https://bugs.launchpad.net/qemu/+bug/1716292/+attachment/5186008/+files/0001-Bring-linux-user-write-2-handling-into-line-with-lin.patch While a developer can chase a URL, our CI tools can't. Can you please also send that patch directly to qemu-devel@nongnu.org, so that it gets the same level of review as other patches? -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH v2 1/3] util: add qemu_write_pidfile()
On 09/07/2018 07:13 AM, Marc-André Lureau wrote: There are variants of qemu_create_pidfile() in qemu-pr-helper and qemu-ga. Let's have a common implementation in libqemuutil. Unrelated to this patch, but a question that this raises: should 'qemu-nbd' also have a mode for creating a pid file, since it has --fork for daemonizing the server process? -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [RFC PATCH v2 7/7] plugins: add syscall logging plugin sample
Pavel Dovgalyuk writes:
> This is an example of plugin which instruments only specific instructions:
> sysenter and sysexit. When executing them, it prints system call id
> and return code to the QEMU log.
Again I'm not sure this is a very useful example either. It doesn't
achieve anything we can't already do with the existing logging/strace
stuff and it is quite ugly in it's knowledge of a single architecture to
try and figure out what's going on.
>
> Signed-off-by: Pavel Dovgalyuk
> ---
> plugins/syscall-log/Makefile | 19
> plugins/syscall-log/syscall-log.c | 44
> +
> 2 files changed, 63 insertions(+)
> create mode 100644 plugins/syscall-log/Makefile
> create mode 100644 plugins/syscall-log/syscall-log.c
>
> diff --git a/plugins/syscall-log/Makefile b/plugins/syscall-log/Makefile
> new file mode 100644
> index 000..1bbdf04
> --- /dev/null
> +++ b/plugins/syscall-log/Makefile
> @@ -0,0 +1,19 @@
> +CFLAGS += -I../include -fno-PIE -fPIC -O3
> +LDFLAGS += -shared
> +# TODO: Windows
> +DSOSUF := .so
> +
> +NAME:= syscall-log
> +BIN := $(NAME)$(DSOSUF)
> +
> +FILES := syscall-log.o
> +
> +%.o: %.c
> + $(CC) -c -o $@ $< $(CFLAGS)
> +
> +all: $(FILES)
> + $(CC) $(LDFLAGS) -o $(BIN) $(FILES)
> +
> +clean:
> + rm $(FILES)
> + rm $(BIN)
> diff --git a/plugins/syscall-log/syscall-log.c
> b/plugins/syscall-log/syscall-log.c
> new file mode 100644
> index 000..1f5d55f
> --- /dev/null
> +++ b/plugins/syscall-log/syscall-log.c
> @@ -0,0 +1,44 @@
> +#include
> +#include
> +#include "plugins.h"
> +
> +bool plugin_init(const char *args)
> +{
> +return true;
> +}
> +
> +bool plugin_needs_before_insn(uint64_t pc, void *cpu)
> +{
> +uint8_t code = 0;
> +if (!qemulib_read_memory(cpu, pc, &code, 1)
> +&& code == 0x0f) {
> +if (qemulib_read_memory(cpu, pc + 1, &code, 1)) {
> +return false;
> +}
> +if (code == 0x34) {
> +/* sysenter */
> +return true;
> +}
> +if (code == 0x35) {
> +/* sysexit */
> +return true;
> +}
> +}
> +return false;
> +}
> +
> +void plugin_before_insn(uint64_t pc, void *cpu)
> +{
> +uint8_t code = 0;
> +uint32_t reg;
> +qemulib_read_memory(cpu, pc + 1, &code, 1);
> +/* Read EAX. There should be a header with register ids
> + or a function for reading the register by the name */
> +qemulib_read_register(cpu, (uint8_t*)®, 0);
> +/* log system calls */
> +if (code == 0x34) {
> +qemulib_log("sysenter %x\n", reg);
> +} else if (code == 0x35) {
> +qemulib_log("sysexit %x\n", reg);
> +}
> +}
--
Alex Bennée
Re: [Qemu-devel] [RFC PATCH v2 0/7] QEMU binary instrumentation prototype
Pavel Dovgalyuk writes: > Peter, what about this one? > > Pavel Dovgalyuk > >> -Original Message- >> From: Pavel Dovgalyuk [mailto:dovga...@ispras.ru] >> Sent: Tuesday, June 05, 2018 2:56 PM >> To: 'Peter Maydell'; 'Pavel Dovgalyuk' >> Cc: 'QEMU Developers'; maria.klimushenk...@ispras.ru; 'Paolo Bonzini'; >> 'Lluís Vilanova' >> Subject: RE: [RFC PATCH v2 0/7] QEMU binary instrumentation prototype >> >> > From: Peter Maydell [mailto:peter.mayd...@linaro.org] >> > >> > This series doesn't seem to add anything to Documentation/ that >> > describes the API we make available to plugins. I'm a lot more >> > interested in reviewing the API that will be used by plugins >> > than I am in the implementation at this stage. Can you provide >> > a description/documentation of the API for review, please? >> >> >> Here is the draft: >> >> Introduction >> >> >> This document describes an API for creating the QEMU >> instrumentation plugins. >> >> It is based on the following prior sources: >> - KVM Forum 2017 talk "Instrumenting, Introspection, and Debugging with >> QEMU" >>https://www.linux-kvm.org/images/3/3d/Introspect.pdf >> - Discussion on Lluis Vilanova instrumentation patch series >>https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg03357.html >> >> The aim of the instrumentation is implementing different runtime >> tracers that can track the executed instructions, memory and >> hardware operations. >> >> Instrumenting the code >> == >> >> Instrumentation subsystem exploits TCG helper mechanism to embed >> callbacks into the translation blocks. These callbacks may be inserted >> before the specific instructions, when the plugins require such filtering. >> >> Translator uses two functions for embedding the callbacks: >> - first function checks whether the current instruction should be >>instrumented >> - second function embeds the callback for executing the plugin-specific >>code before that instruction >> >> The similar method may be used for memory access instrumentation. >> >> QEMU->Plugin API >> >> >> Instrumentation layer passes the requests from the translator >> to the dynamically loaded plugins. Every plugin may provide >> the following functions to perform the instrumentation: >> >> 1. bool plugin_init(const char *args); >> Initialization function. May return false if the plugin >> can't work in the current environment. >> >> 2. bool plugin_needs_before_insn(uint64_t pc, void *cpu); >> Returns true if the plugin needs to instrument the current instruction. >> It may use the address (pc) for making the decision or the guest >> CPU state (cpu), which can be passed back to QEMU core API >> (e.g., for reading the guest memory). >> This function is called at both translation and execution phases. >> >> 3. void plugin_before_insn(uint64_t pc, void *cpu); >> If the previous function returned true for some instruction, >> then this function will be called. This process is repeated before >> every execution of the instruction, if it was instrumented. >> >> The similar pair of functions will also be added for the memory >> operations. >> >> Plugin->QEMU API >> >> >> QEMU core exports some functions to let the plugins introspect the guest >> or perform some interaction with other QEMU services (e.g., logging). >> API doesn't contain any data structures, because their memory layout depend >> on the compilation settings. >> >> QEMU exports the following functions that may be called from the plugins: >> >> 1. void qemulib_log(const char *fmt, ...); >> Wrapper for qemu_log. >> >> 2. int qemulib_read_memory(void *cpu, uint64_t addr, uint8_t *buf, int len); >> Reads guest memory into the buffer. Wrapper for cpu_memory_rw_debug. >> >> 3. int qemulib_read_register(void *cpu, uint8_t *mem_buf, int reg); >> Uses target gdb interface for accessing the guest registers. >> 'reg' is the id of the desired register as it is coded by gdb. >> >> There also should be a function for flushing the translated blocks to >> ensure that the instrumentation will occur in the case of changing >> the internal plugin state. This would certainly be the case if we could generate TCG code in the plugins. -- Alex Bennée
Re: [Qemu-devel] [PATCH 1/3] Improve xen_disk batching behaviour
> -Original Message-
> From: Qemu-devel [mailto:qemu-devel-
> bounces+paul.durrant=citrix@nongnu.org] On Behalf Of Tim Smith
> Sent: 07 September 2018 11:21
> To: qemu-devel@nongnu.org
> Subject: [Qemu-devel] [PATCH 1/3] Improve xen_disk batching behaviour
>
> When I/O consists of many small requests, performance is improved by
> batching them together in a single io_submit() call. When there are
> relatively few requests, the extra overhead is not worth it. This
> introduces a check to start batching I/O requests via blk_io_plug()/
> blk_io_unplug() in an amount proportional to the number which were
> already in flight at the time we started reading the ring.
>
> Signed-off-by: Tim Smith
Reviewed-by: Paul Durrant
> ---
> hw/block/xen_disk.c | 29 +
> 1 file changed, 29 insertions(+)
>
> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> index 36eff94f84..6cb40d66fa 100644
> --- a/hw/block/xen_disk.c
> +++ b/hw/block/xen_disk.c
> @@ -101,6 +101,9 @@ struct XenBlkDev {
> AioContext *ctx;
> };
>
> +/* Threshold of in-flight requests above which we will start using
> + * blk_io_plug()/blk_io_unplug() to batch requests */
> +#define IO_PLUG_THRESHOLD 1
> /* - */
>
> static void ioreq_reset(struct ioreq *ioreq)
> @@ -542,6 +545,8 @@ static void blk_handle_requests(struct XenBlkDev
> *blkdev)
> {
> RING_IDX rc, rp;
> struct ioreq *ioreq;
> +int inflight_atstart = blkdev->requests_inflight;
> +int batched = 0;
>
> blkdev->more_work = 0;
>
> @@ -550,6 +555,16 @@ static void blk_handle_requests(struct XenBlkDev
> *blkdev)
> xen_rmb(); /* Ensure we see queued requests up to 'rp'. */
>
> blk_send_response_all(blkdev);
> +/* If there was more than IO_PLUG_THRESHOLD ioreqs in flight
> + * when we got here, this is an indication that there the bottleneck
> + * is below us, so it's worth beginning to batch up I/O requests
> + * rather than submitting them immediately. The maximum number
> + * of requests we're willing to batch is the number already in
> + * flight, so it can grow up to max_requests when the bottleneck
> + * is below us */
> +if (inflight_atstart > IO_PLUG_THRESHOLD) {
> +blk_io_plug(blkdev->blk);
> +}
> while (rc != rp) {
> /* pull request from ring */
> if (RING_REQUEST_CONS_OVERFLOW(&blkdev->rings.common, rc)) {
> @@ -589,7 +604,21 @@ static void blk_handle_requests(struct XenBlkDev
> *blkdev)
> continue;
> }
>
> +if (inflight_atstart > IO_PLUG_THRESHOLD && batched >=
> inflight_atstart) {
> +blk_io_unplug(blkdev->blk);
> +}
> ioreq_runio_qemu_aio(ioreq);
> +if (inflight_atstart > IO_PLUG_THRESHOLD) {
> +if (batched >= inflight_atstart) {
> +blk_io_plug(blkdev->blk);
> +batched=0;
> +} else {
> +batched++;
> +}
> +}
> +}
> +if (inflight_atstart > IO_PLUG_THRESHOLD) {
> +blk_io_unplug(blkdev->blk);
> }
>
> if (blkdev->more_work && blkdev->requests_inflight < blkdev-
> >max_requests) {
>
Re: [Qemu-devel] [RFC PATCH v2 2/7] Add plugin support
Pavel Dovgalyuk writes:
> This patch adds support for dynamically loaded plugins.
> Every plugin is a dynamic library with a set of optional exported
> functions that will be called from QEMU.
>
> +
> +static QLIST_HEAD(, QemuPluginInfo) qemu_plugins
> += QLIST_HEAD_INITIALIZER(qemu_plugins);
> +
> +static QemuOptsList qemu_plugin_opts = {
> +.name = "plugin",
> +.head = QTAILQ_HEAD_INITIALIZER(qemu_plugin_opts.head),
> +.desc = {
> +{
> +.name = "file",
> +.type = QEMU_OPT_STRING,
> +},{
> +.name = "args",
> +.type = QEMU_OPT_STRING,
> +},
> +{ /* end of list */ }
> +},
> +};
> +
> +void qemu_plugin_parse_cmd_args(const char *optarg)
> +{
> +QemuOpts *opts = qemu_opts_parse_noisily(&qemu_plugin_opts, optarg,
> false);
> +qemu_plugin_load(qemu_opt_get(opts, "file"),
> +qemu_opt_get(opts, "args"));
> +}
Currently this is only available to system mode emulation. Can it be
extended to include linux-user as well?
> +
> +void qemu_plugin_load(const char *filename, const char *args)
> +{
> +GModule *g_module;
> +QemuPluginInfo *info = NULL;
> +if (!filename) {
> +error_report("plugin name was not specified");
> +return;
> +}
> +g_module = g_module_open(filename,
> +G_MODULE_BIND_LAZY | G_MODULE_BIND_LOCAL);
> +if (!g_module) {
> +error_report("can't load plugin '%s'", filename);
> +return;
> +}
> +info = g_new0(QemuPluginInfo, 1);
> +info->filename = g_strdup(filename);
> +info->g_module = g_module;
> +if (args) {
> +info->args = g_strdup(args);
> +}
> +
> +g_module_symbol(g_module, "plugin_init", (gpointer*)&info->init);
> +
> +/* Get the instrumentation callbacks */
> +g_module_symbol(g_module, "plugin_needs_before_insn",
> +(gpointer*)&info->needs_before_insn);
> +g_module_symbol(g_module, "plugin_before_insn",
> +(gpointer*)&info->before_insn);
> +
> +QLIST_INSERT_HEAD(&qemu_plugins, info, next);
> +
> +return;
> +}
> +
> +void qemu_plugins_init(void)
> +{
> +QemuPluginInfo *info;
> +QLIST_FOREACH(info, &qemu_plugins, next) {
> +if (info->init) {
> +info->init(info->args);
> +}
> +}
> +}
> diff --git a/qemu-options.hx b/qemu-options.hx
> index c0d3951..d171544 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3950,6 +3950,16 @@ Dump json-encoded vmstate information for current
> machine type to file
> in @var{file}
> ETEXI
>
> +#ifdef CONFIG_PLUGINS
> +DEF("plugin", HAS_ARG, QEMU_OPTION_plugin, \
> + "-plugin file=[,args=] load plugin with
> \n", QEMU_ARCH_ALL)
> +STEXI
> +@item -plugin file=@var{file}[,args=@var{args}]
> +@findex -plugin
> +Load @var{file} plugin passing @var{args} arguments.
> +ETEXI
> +#endif
> +
> STEXI
> @end table
> ETEXI
> diff --git a/vl.c b/vl.c
> index 0603171..05420bf 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -129,6 +129,7 @@ int main(int argc, char **argv)
> #include "qapi/qapi-commands-run-state.h"
> #include "qapi/qmp/qerror.h"
> #include "sysemu/iothread.h"
> +#include "qemu/plugins.h"
>
> #define MAX_VIRTIO_CONSOLES 1
>
> @@ -3925,6 +3926,11 @@ int main(int argc, char **argv, char **envp)
> exit(1);
> }
> break;
> +#ifdef CONFIG_PLUGINS
> +case QEMU_OPTION_plugin:
> +qemu_plugin_parse_cmd_args(optarg);
> +break;
> +#endif
> case QEMU_OPTION_nodefconfig:
> case QEMU_OPTION_nouserconfig:
> /* Nothing to be parsed here. Especially, do not error out
> below. */
> @@ -4470,6 +4476,8 @@ int main(int argc, char **argv, char **envp)
> }
> parse_numa_opts(current_machine);
>
> +qemu_plugins_init();
> +
> /* do monitor/qmp handling at preconfig state if requested */
> main_loop();
>
--
Alex Bennée
Re: [Qemu-devel] [RFC PATCH v2 0/7] QEMU binary instrumentation prototype
Pavel Dovgalyuk writes: > The following series implements dynamic binary instrumentation upon > QEMU. OK I've done a pass through the patches, final comments bellow. > > For the current patches the plugins should provide the following > callbacks: > - "needs" callback to check whether the specific instruction >should be instrumented by this plugin > - "run" callback which called before executing the instuction I think with a little re-jigging we could reduce this to a single run call which can then decide if it is going to deal with things in this case. > Our instrumentation subsystem exploits TCG helper mechanism to embed > callbacks into the translation blocks. These callbacks may be inserted > before the specific instructions. > > The aim of submission of this series at that early stage is to get > the feedback which will guide the development process. We are faced > the following questions: > 1. Does every plugins should have its own callback embedded into the TB > (which will cause TB extra growth in case of multiple plugins), > or the instrumentation layer's callback should invoke the plugins > that wanted to instrument that specific instruction? The common case is having one plugin and I'd rather give the flexibility of what helper to call and how much information to give it to the plugin and deal with the potential to have multiple helper calls per instruction for the complex cases. Now I can see arguments against it from an interface complexity point of view but I think plugins should get access to the TCG functions so they can generate their own op sequences if need be and not have to call a helper at all if they don't need to. > 2. How the plugins should function? Will they work as a binary dynamic > libraries or a script on some interpreted language? I think plain C dynamic libraries with optional TCG ops would be a sensible starting point. For the sort of analysis that potentially involves lots of operations adding an interpreted language would be: - a bike-shedding operation (everyone has a favourite) - slower than C > 3. Should the plugins reuse QEMU configuration script results? > Now there is no possibility for using platform-specific macros > generated by QEMU configure. It depends but I'm of the opinion that you build your plugins for the QEMU you have and not expect them to run with any other version. Defining some sort of cross-version API stability is just making a rod for our own back. > 4. Maybe QEMU module infrastructure should be extended to support > plugins too? Pass - I'm not very knowledgeable about what the existing module stuff does. > 5. How the GDB-related CPU inspection interface may be used better? > We should pass a register code to read the value. These codes > are not described in any of the files. Maybe a function for > accessing register by name should be added? Personally it feels a bit hacky at the moment. Perhaps we should be looking at exposing tcg_global to the plugin interface. They are already registered with names and it would be more efficient to load from TCG code into the helper. > > > v2 changes: > - added a subsystem for the plugins > - added QEMU side API for plugins > - added sample plugins for simple tracing > > --- > > Pavel Dovgalyuk (7): > tcg: add headers for non-target helpers > Add plugin support > plugins: provide helper functions for plugins > tcg: add instrumenting module > plugins: add plugin template > plugin: add instruction execution logger > plugins: add syscall logging plugin sample > > > Makefile.target |1 > accel/tcg/translator.c|5 + > configure | 14 > include/exec/helper-register.h| 53 +++ > include/qemu/instrument.h |7 ++ > include/qemu/plugins.h|8 ++ > plugins/exec-log/Makefile | 19 + > plugins/exec-log/exec-log.c | 18 + > plugins/helper.h |1 > plugins/include/plugins.h | 18 + > plugins/plugins.c | 132 > + > plugins/qemulib.c | 31 + > plugins/syscall-log/Makefile | 19 + > plugins/syscall-log/syscall-log.c | 44 > plugins/template/Makefile | 19 + > plugins/template/template.c | 19 + > qemu-options.hx | 10 +++ > tcg/tcg.c | 12 +++ > tcg/tcg.h |3 + > vl.c |8 ++ > 20 files changed, 440 insertions(+), 1 deletion(-) > create mode 100644 include/exec/helper-register.h > create mode 100644 include/qemu/instrument.h > create mode 100644 include/qemu/plugins.h > create mode 100644 plugins/exec-log/Makefile > create mode 100644 plugins/exec-log/exec-log.c > create mode 100644 plugins/helper
Re: [Qemu-devel] [PATCH 1/6] qht: remove unused map param from qht_remove__locked
Emilio G. Cota writes:
> Signed-off-by: Emilio G. Cota
Reviewed-by: Alex Bennée
> ---
> util/qht.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/util/qht.c b/util/qht.c
> index c138777a9c..7b57b50a24 100644
> --- a/util/qht.c
> +++ b/util/qht.c
> @@ -665,8 +665,7 @@ static inline void qht_bucket_remove_entry(struct
> qht_bucket *orig, int pos)
>
> /* call with b->lock held */
> static inline
> -bool qht_remove__locked(struct qht_map *map, struct qht_bucket *head,
> -const void *p, uint32_t hash)
> +bool qht_remove__locked(struct qht_bucket *head, const void *p, uint32_t
> hash)
> {
> struct qht_bucket *b = head;
> int i;
> @@ -701,7 +700,7 @@ bool qht_remove(struct qht *ht, const void *p, uint32_t
> hash)
> qht_debug_assert(p);
>
> b = qht_bucket_lock__no_stale(ht, hash, &map);
> -ret = qht_remove__locked(map, b, p, hash);
> +ret = qht_remove__locked(b, p, hash);
> qht_bucket_debug__locked(b);
> qemu_spin_unlock(&b->lock);
> return ret;
--
Alex Bennée
Re: [Qemu-devel] [PATCH 2/6] qht: add qht_iter_remove
Emilio G. Cota writes:
> This currently has no users, but the use case is so common that I
> think we must support it.
>
> Note that without the appended we cannot safely remove a set of
> elements; a 2-step approach (i.e. qht_iter first, keep track of
> the to-be-deleted elements, and then a bunch of qht_remove calls)
> would be racy, since between the iteration and the removals other
> threads might insert additional elements.
>
> Signed-off-by: Emilio G. Cota
> ---
> include/qemu/qht.h | 19
> util/qht.c | 74 +-
> 2 files changed, 85 insertions(+), 8 deletions(-)
>
> diff --git a/include/qemu/qht.h b/include/qemu/qht.h
> index 1fb9116fa0..91bc9b00cf 100644
> --- a/include/qemu/qht.h
> +++ b/include/qemu/qht.h
> @@ -44,6 +44,8 @@ struct qht_stats {
>
> typedef bool (*qht_lookup_func_t)(const void *obj, const void *userp);
> typedef void (*qht_iter_func_t)(struct qht *ht, void *p, uint32_t h, void
> *up);
> +typedef bool (*qht_iter_bool_func_t)(struct qht *ht, void *p, uint32_t h,
> + void *up);
>
> #define QHT_MODE_AUTO_RESIZE 0x1 /* auto-resize when heavily loaded */
>
> @@ -178,9 +180,26 @@ bool qht_resize(struct qht *ht, size_t n_elems);
> *
> * Each time it is called, user-provided @func is passed a pointer-hash pair,
> * plus @userp.
> + *
> + * Note: @ht cannot be accessed from @func
I'm confused by this comment. If @ht cannot be accessed (or shouldn't be
accessed) from @func then why are we passing it?
> + * See also: qht_iter_remove()
> */
> void qht_iter(struct qht *ht, qht_iter_func_t func, void *userp);
>
> +/**
> + * qht_iter_remove - Iterate over a QHT, optionally removing entries
> + * @ht: QHT to be iterated over
> + * @func: function to be called for each entry in QHT
> + * @userp: additional pointer to be passed to @func
> + *
> + * Each time it is called, user-provided @func is passed a pointer-hash pair,
> + * plus @userp. If @func returns true, the pointer-hash pair is removed.
> + *
> + * Note: @ht cannot be accessed from @func
> + * See also: qht_iter()
> + */
> +void qht_iter_remove(struct qht *ht, qht_iter_bool_func_t func, void *userp);
> +
> /**
> * qht_statistics_init - Gather statistics from a QHT
> * @ht: QHT to gather statistics from
> diff --git a/util/qht.c b/util/qht.c
> index 7b57b50a24..caec53dd0e 100644
> --- a/util/qht.c
> +++ b/util/qht.c
> @@ -89,6 +89,19 @@
> #define QHT_BUCKET_ENTRIES 4
> #endif
>
> +enum qht_iter_type {
> +QHT_ITER_VOID,/* do nothing; use retvoid */
> +QHT_ITER_RM, /* remove element if retbool returns true */
> +};
> +
> +struct qht_iter {
> +union {
> +qht_iter_func_t retvoid;
> +qht_iter_bool_func_t retbool;
> +} f;
> +enum qht_iter_type type;
> +};
> +
> /*
> * Note: reading partially-updated pointers in @pointers could lead to
> * segfaults. We thus access them with atomic_read/set; this guarantees
> @@ -706,9 +719,10 @@ bool qht_remove(struct qht *ht, const void *p, uint32_t
> hash)
> return ret;
> }
>
> -static inline void qht_bucket_iter(struct qht *ht, struct qht_bucket *b,
> - qht_iter_func_t func, void *userp)
> +static inline void qht_bucket_iter(struct qht *ht, struct qht_bucket *head,
> + const struct qht_iter *iter, void *userp)
> {
> +struct qht_bucket *b = head;
> int i;
>
> do {
> @@ -716,7 +730,25 @@ static inline void qht_bucket_iter(struct qht *ht,
> struct qht_bucket *b,
> if (b->pointers[i] == NULL) {
> return;
> }
> -func(ht, b->pointers[i], b->hashes[i], userp);
> +switch (iter->type) {
> +case QHT_ITER_VOID:
> +iter->f.retvoid(ht, b->pointers[i], b->hashes[i], userp);
> +break;
> +case QHT_ITER_RM:
> +if (iter->f.retbool(ht, b->pointers[i], b->hashes[i],
> userp)) {
> +/* replace i with the last valid element in the bucket */
> +seqlock_write_begin(&head->sequence);
> +qht_bucket_remove_entry(b, i);
> +seqlock_write_end(&head->sequence);
> +qht_bucket_debug__locked(b);
> +/* reevaluate i, since it just got replaced */
> +i--;
> +continue;
> +}
> +break;
> +default:
> +g_assert_not_reached();
> +}
> }
> b = b->next;
> } while (b);
> @@ -724,26 +756,48 @@ static inline void qht_bucket_iter(struct qht *ht,
> struct qht_bucket *b,
>
> /* call with all of the map's locks held */
> static inline void qht_map_iter__all_locked(struct qht *ht, struct qht_map
> *map,
> -qht_iter_func_t func, void
> *userp)
> +
Re: [Qemu-devel] [PATCH 3/6] test-qht: test qht_iter_remove
Emilio G. Cota writes:
> Signed-off-by: Emilio G. Cota
Reviewed-by: Alex Bennée
> ---
> tests/test-qht.c | 50 ++--
> 1 file changed, 48 insertions(+), 2 deletions(-)
>
> diff --git a/tests/test-qht.c b/tests/test-qht.c
> index dda6a067be..283fb3db39 100644
> --- a/tests/test-qht.c
> +++ b/tests/test-qht.c
> @@ -108,6 +108,49 @@ static void iter_check(unsigned int count)
> g_assert_cmpuint(curr, ==, count);
> }
>
> +static void sum_func(struct qht *ht, void *p, uint32_t hash, void *userp)
> +{
> +uint32_t *sum = userp;
> +uint32_t a = *(uint32_t *)p;
> +
> +*sum += a;
> +}
> +
> +static void iter_sum_check(unsigned int expected)
> +{
> +unsigned int sum = 0;
> +
> +qht_iter(&ht, sum_func, &sum);
> +g_assert_cmpuint(sum, ==, expected);
> +}
> +
> +static bool rm_mod_func(struct qht *ht, void *p, uint32_t hash, void *userp)
> +{
> +uint32_t a = *(uint32_t *)p;
> +unsigned int mod = *(unsigned int *)userp;
> +
> +return a % mod == 0;
> +}
> +
> +static void iter_rm_mod(unsigned int mod)
> +{
> +qht_iter_remove(&ht, rm_mod_func, &mod);
> +}
> +
> +static void iter_rm_mod_check(unsigned int mod)
> +{
> +unsigned int expected = 0;
> +unsigned int i;
> +
> +for (i = 0; i < N; i++) {
> +if (i % mod == 0) {
> +continue;
> +}
> +expected += i;
> +}
> +iter_sum_check(expected);
> +}
> +
> static void qht_do_test(unsigned int mode, size_t init_entries)
> {
> /* under KVM we might fetch stats from an uninitialized qht */
> @@ -138,8 +181,11 @@ static void qht_do_test(unsigned int mode, size_t
> init_entries)
> insert(10, 150);
> check_n(N);
>
> -rm(1, 2);
> -check_n(N - 1);
> +qht_reset(&ht);
> +insert(0, N);
> +iter_rm_mod(10);
> +iter_rm_mod_check(10);
> +check_n(N * 9 / 10);
> qht_reset_size(&ht, 0);
> check_n(0);
> check(0, N, false);
--
Alex Bennée
Re: [Qemu-devel] [PATCH 4/6] test-qht: test removal of non-existent entries
Emilio G. Cota writes:
> This improves qht.c code coverage from 89.44% to 90.00%.
>
> Signed-off-by: Emilio G. Cota
Reviewed-by: Alex Bennée
> ---
> tests/test-qht.c | 26 --
> 1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/tests/test-qht.c b/tests/test-qht.c
> index 283fb3db39..05b1d6807a 100644
> --- a/tests/test-qht.c
> +++ b/tests/test-qht.c
> @@ -41,7 +41,7 @@ static void insert(int a, int b)
> }
> }
>
> -static void rm(int init, int end)
> +static void do_rm(int init, int end, bool exist)
> {
> int i;
>
> @@ -49,10 +49,24 @@ static void rm(int init, int end)
> uint32_t hash;
>
> hash = arr[i];
> -g_assert_true(qht_remove(&ht, &arr[i], hash));
> +if (exist) {
> +g_assert_true(qht_remove(&ht, &arr[i], hash));
> +} else {
> +g_assert_false(qht_remove(&ht, &arr[i], hash));
> +}
> }
> }
>
> +static void rm(int init, int end)
> +{
> +do_rm(init, end, true);
> +}
> +
> +static void rm_nonexist(int init, int end)
> +{
> +do_rm(init, end, false);
> +}
> +
> static void check(int a, int b, bool expected)
> {
> struct qht_stats stats;
> @@ -157,8 +171,15 @@ static void qht_do_test(unsigned int mode, size_t
> init_entries)
> check_n(0);
>
> qht_init(&ht, is_equal, 0, mode);
> +rm_nonexist(0, 4);
> +insert(0, 4);
> +rm_nonexist(5, 6);
> +insert(4, 6);
> +rm_nonexist(7, 8);
> +iter_rm_mod(1);
>
> check_n(0);
> +rm_nonexist(0, 10);
> insert(0, N);
> check(0, N, true);
> check_n(N);
> @@ -183,6 +204,7 @@ static void qht_do_test(unsigned int mode, size_t
> init_entries)
>
> qht_reset(&ht);
> insert(0, N);
> +rm_nonexist(N, N + 32);
> iter_rm_mod(10);
> iter_rm_mod_check(10);
> check_n(N * 9 / 10);
--
Alex Bennée
Re: [Qemu-devel] [PATCH 5/6] test-qht: test deletion of the last entry in a bucket
Emilio G. Cota writes: > This improves coverage by one (!) LoC in qht.c, bringing the > coverage rate up from 90.00% to 90.28%. > > Signed-off-by: Emilio G. Cota Reviewed-by: Alex Bennée > --- > tests/test-qht.c | 13 - > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/tests/test-qht.c b/tests/test-qht.c > index 05b1d6807a..77666e8c5f 100644 > --- a/tests/test-qht.c > +++ b/tests/test-qht.c > @@ -172,9 +172,20 @@ static void qht_do_test(unsigned int mode, size_t > init_entries) > > qht_init(&ht, is_equal, 0, mode); > rm_nonexist(0, 4); > +/* > + * Test that we successfully delete the last element in a bucket. > + * This is a hard-to-reach code path when resizing is on, but without > + * resizing we can easily hit it if init_entries <= 1. > + * Given that the number of elements per bucket can be 4 or 6 depending > on > + * the host's pointer size, test the removal of the 4th and 6th elements. > + */ > insert(0, 4); > rm_nonexist(5, 6); > -insert(4, 6); > +rm(3, 4); > +check_n(3); > +insert(3, 6); > +rm(5, 6); > +check_n(5); > rm_nonexist(7, 8); > iter_rm_mod(1); -- Alex Bennée
