* [Qemu-devel] [PULL 01/10] memory: allow destroying a non-empty MemoryRegion
2015-10-02 17:20 [Qemu-devel] [PULL 00/10] Fix device introspection regressions Markus Armbruster
@ 2015-10-02 17:20 ` Markus Armbruster
2015-10-02 17:20 ` [Qemu-devel] [PULL 02/10] hw: do not pass NULL to memory_region_init from instance_init Markus Armbruster
` (9 subsequent siblings)
10 siblings, 0 replies; 21+ messages in thread
From: Markus Armbruster @ 2015-10-02 17:20 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini
From: Paolo Bonzini <pbonzini@redhat.com>
This is legal; the MemoryRegion will simply unreference all the
existing subregions and possibly bring them down with it as well.
However, it requires a bit of care to avoid an infinite loop.
Finalizing a memory region cannot trigger an address space update,
but memory_region_del_subregion errs on the side of caution and
might trigger a spurious update: avoid that by resetting mr->enabled
first.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1443689999-12182-2-git-send-email-armbru@redhat.com>
---
memory.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/memory.c b/memory.c
index ef87363..73d28ba 100644
--- a/memory.c
+++ b/memory.c
@@ -1304,7 +1304,22 @@ static void memory_region_finalize(Object *obj)
{
MemoryRegion *mr = MEMORY_REGION(obj);
- assert(QTAILQ_EMPTY(&mr->subregions));
+ assert(!mr->container);
+
+ /* We know the region is not visible in any address space (it
+ * does not have a container and cannot be a root either because
+ * it has no references, so we can blindly clear mr->enabled.
+ * memory_region_set_enabled instead could trigger a transaction
+ * and cause an infinite loop.
+ */
+ mr->enabled = false;
+ memory_region_transaction_begin();
+ while (!QTAILQ_EMPTY(&mr->subregions)) {
+ MemoryRegion *subregion = QTAILQ_FIRST(&mr->subregions);
+ memory_region_del_subregion(mr, subregion);
+ }
+ memory_region_transaction_commit();
+
mr->destructor(mr);
memory_region_clear_coalescing(mr);
g_free((char *)mr->name);
--
2.4.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PULL 02/10] hw: do not pass NULL to memory_region_init from instance_init
2015-10-02 17:20 [Qemu-devel] [PULL 00/10] Fix device introspection regressions Markus Armbruster
2015-10-02 17:20 ` [Qemu-devel] [PULL 01/10] memory: allow destroying a non-empty MemoryRegion Markus Armbruster
@ 2015-10-02 17:20 ` Markus Armbruster
2015-10-02 17:20 ` [Qemu-devel] [PULL 03/10] macio: move DBDMA_init from instance_init to realize Markus Armbruster
` (8 subsequent siblings)
10 siblings, 0 replies; 21+ messages in thread
From: Markus Armbruster @ 2015-10-02 17:20 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini
From: Paolo Bonzini <pbonzini@redhat.com>
This causes the region to outlive the object, because it attaches the
region to /machine. This is not nice for the "realize" method, but
much worse for "instance_init" because it can cause dangling pointers
after a simple object_new/object_unref pair.
Reported-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Tested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1443689999-12182-3-git-send-email-armbru@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
hw/arm/pxa2xx.c | 2 +-
hw/display/cg3.c | 4 ++--
hw/display/tcx.c | 2 +-
hw/misc/arm_integrator_debug.c | 2 +-
hw/misc/macio/cuda.c | 2 +-
hw/misc/macio/macio.c | 6 +++---
hw/pcmcia/pxa2xx.c | 6 +++---
7 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c
index 164260a..79d22d9 100644
--- a/hw/arm/pxa2xx.c
+++ b/hw/arm/pxa2xx.c
@@ -1958,7 +1958,7 @@ static void pxa2xx_fir_instance_init(Object *obj)
PXA2xxFIrState *s = PXA2XX_FIR(obj);
SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
- memory_region_init_io(&s->iomem, NULL, &pxa2xx_fir_ops, s,
+ memory_region_init_io(&s->iomem, obj, &pxa2xx_fir_ops, s,
"pxa2xx-fir", 0x1000);
sysbus_init_mmio(sbd, &s->iomem);
sysbus_init_irq(sbd, &s->irq);
diff --git a/hw/display/cg3.c b/hw/display/cg3.c
index d2a0d97..e309fbe 100644
--- a/hw/display/cg3.c
+++ b/hw/display/cg3.c
@@ -280,12 +280,12 @@ static void cg3_initfn(Object *obj)
SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
CG3State *s = CG3(obj);
- memory_region_init_ram(&s->rom, NULL, "cg3.prom", FCODE_MAX_ROM_SIZE,
+ memory_region_init_ram(&s->rom, obj, "cg3.prom", FCODE_MAX_ROM_SIZE,
&error_fatal);
memory_region_set_readonly(&s->rom, true);
sysbus_init_mmio(sbd, &s->rom);
- memory_region_init_io(&s->reg, NULL, &cg3_reg_ops, s, "cg3.reg",
+ memory_region_init_io(&s->reg, obj, &cg3_reg_ops, s, "cg3.reg",
CG3_REG_SIZE);
sysbus_init_mmio(sbd, &s->reg);
}
diff --git a/hw/display/tcx.c b/hw/display/tcx.c
index 4635800..bf119bc 100644
--- a/hw/display/tcx.c
+++ b/hw/display/tcx.c
@@ -944,7 +944,7 @@ static void tcx_initfn(Object *obj)
SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
TCXState *s = TCX(obj);
- memory_region_init_ram(&s->rom, NULL, "tcx.prom", FCODE_MAX_ROM_SIZE,
+ memory_region_init_ram(&s->rom, OBJECT(s), "tcx.prom", FCODE_MAX_ROM_SIZE,
&error_fatal);
memory_region_set_readonly(&s->rom, true);
sysbus_init_mmio(sbd, &s->rom);
diff --git a/hw/misc/arm_integrator_debug.c b/hw/misc/arm_integrator_debug.c
index 99b720f..6d9dd74 100644
--- a/hw/misc/arm_integrator_debug.c
+++ b/hw/misc/arm_integrator_debug.c
@@ -79,7 +79,7 @@ static void intdbg_control_init(Object *obj)
SysBusDevice *sd = SYS_BUS_DEVICE(obj);
IntegratorDebugState *s = INTEGRATOR_DEBUG(obj);
- memory_region_init_io(&s->iomem, NULL, &intdbg_control_ops,
+ memory_region_init_io(&s->iomem, obj, &intdbg_control_ops,
NULL, "dbg-leds", 0x1000000);
sysbus_init_mmio(sd, &s->iomem);
}
diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
index f3984e3..5d7043e 100644
--- a/hw/misc/macio/cuda.c
+++ b/hw/misc/macio/cuda.c
@@ -713,7 +713,7 @@ static void cuda_initfn(Object *obj)
CUDAState *s = CUDA(obj);
int i;
- memory_region_init_io(&s->mem, NULL, &cuda_ops, s, "cuda", 0x2000);
+ memory_region_init_io(&s->mem, obj, &cuda_ops, s, "cuda", 0x2000);
sysbus_init_mmio(d, &s->mem);
sysbus_init_irq(d, &s->irq);
diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index e3c0242..2548d96 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -105,10 +105,10 @@ static void macio_escc_legacy_setup(MacIOState *macio_state)
0xF0, 0xE0,
};
- memory_region_init(escc_legacy, NULL, "escc-legacy", 256);
+ memory_region_init(escc_legacy, OBJECT(macio_state), "escc-legacy", 256);
for (i = 0; i < ARRAY_SIZE(maps); i += 2) {
MemoryRegion *port = g_new(MemoryRegion, 1);
- memory_region_init_alias(port, NULL, "escc-legacy-port",
+ memory_region_init_alias(port, OBJECT(macio_state), "escc-legacy-port",
macio_state->escc_mem, maps[i+1], 0x2);
memory_region_add_subregion(escc_legacy, maps[i], port);
}
@@ -330,7 +330,7 @@ static void macio_instance_init(Object *obj)
MacIOState *s = MACIO(obj);
MemoryRegion *dbdma_mem;
- memory_region_init(&s->bar, NULL, "macio", 0x80000);
+ memory_region_init(&s->bar, obj, "macio", 0x80000);
object_initialize(&s->cuda, sizeof(s->cuda), TYPE_CUDA);
qdev_set_parent_bus(DEVICE(&s->cuda), sysbus_get_default());
diff --git a/hw/pcmcia/pxa2xx.c b/hw/pcmcia/pxa2xx.c
index a7e1877..812716e 100644
--- a/hw/pcmcia/pxa2xx.c
+++ b/hw/pcmcia/pxa2xx.c
@@ -163,7 +163,7 @@ static void pxa2xx_pcmcia_initfn(Object *obj)
sysbus_init_mmio(sbd, &s->container_mem);
/* Socket I/O Memory Space */
- memory_region_init_io(&s->iomem, NULL, &pxa2xx_pcmcia_io_ops, s,
+ memory_region_init_io(&s->iomem, obj, &pxa2xx_pcmcia_io_ops, s,
"pxa2xx-pcmcia-io", 0x04000000);
memory_region_add_subregion(&s->container_mem, 0x00000000,
&s->iomem);
@@ -171,13 +171,13 @@ static void pxa2xx_pcmcia_initfn(Object *obj)
/* Then next 64 MB is reserved */
/* Socket Attribute Memory Space */
- memory_region_init_io(&s->attr_iomem, NULL, &pxa2xx_pcmcia_attr_ops, s,
+ memory_region_init_io(&s->attr_iomem, obj, &pxa2xx_pcmcia_attr_ops, s,
"pxa2xx-pcmcia-attribute", 0x04000000);
memory_region_add_subregion(&s->container_mem, 0x08000000,
&s->attr_iomem);
/* Socket Common Memory Space */
- memory_region_init_io(&s->common_iomem, NULL, &pxa2xx_pcmcia_common_ops, s,
+ memory_region_init_io(&s->common_iomem, obj, &pxa2xx_pcmcia_common_ops, s,
"pxa2xx-pcmcia-common", 0x04000000);
memory_region_add_subregion(&s->container_mem, 0x0c000000,
&s->common_iomem);
--
2.4.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PULL 03/10] macio: move DBDMA_init from instance_init to realize
2015-10-02 17:20 [Qemu-devel] [PULL 00/10] Fix device introspection regressions Markus Armbruster
2015-10-02 17:20 ` [Qemu-devel] [PULL 01/10] memory: allow destroying a non-empty MemoryRegion Markus Armbruster
2015-10-02 17:20 ` [Qemu-devel] [PULL 02/10] hw: do not pass NULL to memory_region_init from instance_init Markus Armbruster
@ 2015-10-02 17:20 ` Markus Armbruster
2015-10-02 17:20 ` [Qemu-devel] [PULL 04/10] tests: Fix how qom-test is run Markus Armbruster
` (7 subsequent siblings)
10 siblings, 0 replies; 21+ messages in thread
From: Markus Armbruster @ 2015-10-02 17:20 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini
From: Paolo Bonzini <pbonzini@redhat.com>
DBDMA_init is not idempotent, and calling it from instance_init
breaks a simple object_new/object_unref pair. Work around this,
pending qdev-ification of DBDMA, by moving the call to realize.
Reported-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1443689999-12182-4-git-send-email-armbru@redhat.com>
---
hw/misc/macio/macio.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index 2548d96..c661f86 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -131,6 +131,10 @@ static void macio_common_realize(PCIDevice *d, Error **errp)
MacIOState *s = MACIO(d);
SysBusDevice *sysbus_dev;
Error *err = NULL;
+ MemoryRegion *dbdma_mem;
+
+ s->dbdma = DBDMA_init(&dbdma_mem);
+ memory_region_add_subregion(&s->bar, 0x08000, dbdma_mem);
object_property_set_bool(OBJECT(&s->cuda), true, "realized", &err);
if (err) {
@@ -328,16 +332,12 @@ static void macio_newworld_init(Object *obj)
static void macio_instance_init(Object *obj)
{
MacIOState *s = MACIO(obj);
- MemoryRegion *dbdma_mem;
memory_region_init(&s->bar, obj, "macio", 0x80000);
object_initialize(&s->cuda, sizeof(s->cuda), TYPE_CUDA);
qdev_set_parent_bus(DEVICE(&s->cuda), sysbus_get_default());
object_property_add_child(obj, "cuda", OBJECT(&s->cuda), NULL);
-
- s->dbdma = DBDMA_init(&dbdma_mem);
- memory_region_add_subregion(&s->bar, 0x08000, dbdma_mem);
}
static const VMStateDescription vmstate_macio_oldworld = {
--
2.4.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PULL 04/10] tests: Fix how qom-test is run
2015-10-02 17:20 [Qemu-devel] [PULL 00/10] Fix device introspection regressions Markus Armbruster
` (2 preceding siblings ...)
2015-10-02 17:20 ` [Qemu-devel] [PULL 03/10] macio: move DBDMA_init from instance_init to realize Markus Armbruster
@ 2015-10-02 17:20 ` Markus Armbruster
2015-10-02 17:20 ` [Qemu-devel] [PULL 05/10] libqtest: Clean up unused QTestState member sigact_old Markus Armbruster
` (6 subsequent siblings)
10 siblings, 0 replies; 21+ messages in thread
From: Markus Armbruster @ 2015-10-02 17:20 UTC (permalink / raw)
To: qemu-devel
We want to run qom-test for every architecture, without having to
manually add it to every architecture's list of tests. Commit 3687d53
accomplished this by adding it to every architecture's list
automatically.
However, some architectures inherit their tests from others, like this:
check-qtest-x86_64-y = $(check-qtest-i386-y)
check-qtest-microblazeel-y = $(check-qtest-microblaze-y)
check-qtest-xtensaeb-y = $(check-qtest-xtensa-y)
For such architectures, we ended up running the (slow!) test twice.
Commit 2b8419c attempted to avoid this by adding the test only when
it's not already present. Works only as long as we consider adding
the test to the architectures on the left hand side *after* the ones
on the right hand side: x86_64 after i386, microblazeel after
microblaze, xtensaeb after xtensa.
Turns out we consider them in $(SYSEMU_TARGET_LIST) order. Defined as
SYSEMU_TARGET_LIST := $(subst -softmmu.mak,,$(notdir \
$(wildcard $(SRC_PATH)/default-configs/*-softmmu.mak)))
On my machine, this results in the oder xtensa, x86_64, microblazeel,
microblaze, i386. Consequently, qom-test runs twice for microblazeel
and x86_64.
Replace this complex and flawed machinery with a much simpler one: add
generic tests (currently just qom-test) to check-qtest-generic-y
instead of check-qtest-$(target)-y for every target, then run
$(check-qtest-generic-y) for every target.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Andreas Färber <afaerber@suse.de>
Message-Id: <1443689999-12182-5-git-send-email-armbru@redhat.com>
---
tests/Makefile | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/tests/Makefile b/tests/Makefile
index 4063639..9380e14 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -86,6 +86,8 @@ check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
# All QTests for now are POSIX-only, but the dependencies are
# really in libqtest, not in the testcases themselves.
+check-qtest-generic-y =
+
gcov-files-ipack-y += hw/ipack/ipack.c
check-qtest-ipack-y += tests/ipoctal232-test$(EXESUF)
gcov-files-ipack-y += hw/char/ipoctal232.c
@@ -216,10 +218,7 @@ gcov-files-ppc64-y += ppc64-softmmu/hw/ppc/spapr_pci.c
check-qtest-microblazeel-y = $(check-qtest-microblaze-y)
check-qtest-xtensaeb-y = $(check-qtest-xtensa-y)
-# qom-test works for all sysemu architectures:
-$(foreach target,$(SYSEMU_TARGET_LIST), \
- $(if $(findstring tests/qom-test$(EXESUF), $(check-qtest-$(target)-y)),, \
- $(eval check-qtest-$(target)-y += tests/qom-test$(EXESUF))))
+check-qtest-generic-y += tests/qom-test$(EXESUF)
check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
comments.json empty.json enum-empty.json enum-missing-data.json \
@@ -446,8 +445,11 @@ CFLAGS += $(TEST_CFLAGS)
TARGETS=$(patsubst %-softmmu,%, $(filter %-softmmu,$(TARGET_DIRS)))
ifeq ($(CONFIG_POSIX),y)
-QTEST_TARGETS=$(foreach TARGET,$(TARGETS), $(if $(check-qtest-$(TARGET)-y), $(TARGET),))
+QTEST_TARGETS = $(TARGETS)
check-qtest-y=$(foreach TARGET,$(TARGETS), $(check-qtest-$(TARGET)-y))
+check-qtest-y += $(check-qtest-generic-y)
+else
+QTEST_TARGETS =
endif
qtest-obj-y = tests/libqtest.o $(test-util-obj-y)
@@ -485,7 +487,7 @@ $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y)
$(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \
QTEST_QEMU_IMG=qemu-img$(EXESUF) \
MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
- gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y),"GTESTER $@")
+ gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y) $(check-qtest-generic-y),"GTESTER $@")
$(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y); do \
echo Gcov report for $$f:;\
$(GCOV) $(GCOV_OPTIONS) $$f -o `dirname $$f`; \
--
2.4.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PULL 05/10] libqtest: Clean up unused QTestState member sigact_old
2015-10-02 17:20 [Qemu-devel] [PULL 00/10] Fix device introspection regressions Markus Armbruster
` (3 preceding siblings ...)
2015-10-02 17:20 ` [Qemu-devel] [PULL 04/10] tests: Fix how qom-test is run Markus Armbruster
@ 2015-10-02 17:20 ` Markus Armbruster
2015-10-02 17:20 ` [Qemu-devel] [PULL 06/10] libqtest: New hmp() & friends Markus Armbruster
` (5 subsequent siblings)
10 siblings, 0 replies; 21+ messages in thread
From: Markus Armbruster @ 2015-10-02 17:20 UTC (permalink / raw)
To: qemu-devel
Unused since commit d766825.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443689999-12182-6-git-send-email-armbru@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
tests/libqtest.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/tests/libqtest.c b/tests/libqtest.c
index e5188e0..8dede56 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -46,7 +46,6 @@ struct QTestState
bool irq_level[MAX_IRQ];
GString *rx;
pid_t qemu_pid; /* our child QEMU process */
- struct sigaction sigact_old; /* restored on exit */
};
static GList *qtest_instances;
--
2.4.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PULL 06/10] libqtest: New hmp() & friends
2015-10-02 17:20 [Qemu-devel] [PULL 00/10] Fix device introspection regressions Markus Armbruster
` (4 preceding siblings ...)
2015-10-02 17:20 ` [Qemu-devel] [PULL 05/10] libqtest: Clean up unused QTestState member sigact_old Markus Armbruster
@ 2015-10-02 17:20 ` Markus Armbruster
2015-10-02 17:20 ` [Qemu-devel] [PULL 07/10] device-introspect-test: New, covering device introspection Markus Armbruster
` (4 subsequent siblings)
10 siblings, 0 replies; 21+ messages in thread
From: Markus Armbruster @ 2015-10-02 17:20 UTC (permalink / raw)
To: qemu-devel
New convenience function hmp() to facilitate use of
human-monitor-command in tests. Use it to simplify its existing uses.
To blend into existing libqtest code, also add qtest_hmpv() and
qtest_hmp(). That, and the egregiously verbose GTK-Doc comment format
make this patch look bigger than it is.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-Id: <1443689999-12182-7-git-send-email-armbru@redhat.com>
---
tests/drive_del-test.c | 22 ++++++----------------
tests/ide-test.c | 8 ++------
tests/libqtest.c | 37 +++++++++++++++++++++++++++++++++++++
tests/libqtest.h | 33 +++++++++++++++++++++++++++++++++
4 files changed, 78 insertions(+), 22 deletions(-)
diff --git a/tests/drive_del-test.c b/tests/drive_del-test.c
index 8951f6f..3390946 100644
--- a/tests/drive_del-test.c
+++ b/tests/drive_del-test.c
@@ -16,28 +16,18 @@
static void drive_add(void)
{
- QDict *response;
+ char *resp = hmp("drive_add 0 if=none,id=drive0");
- response = qmp("{'execute': 'human-monitor-command',"
- " 'arguments': {"
- " 'command-line': 'drive_add 0 if=none,id=drive0'"
- "}}");
- g_assert(response);
- g_assert_cmpstr(qdict_get_try_str(response, "return"), ==, "OK\r\n");
- QDECREF(response);
+ g_assert_cmpstr(resp, ==, "OK\r\n");
+ g_free(resp);
}
static void drive_del(void)
{
- QDict *response;
+ char *resp = hmp("drive_del drive0");
- response = qmp("{'execute': 'human-monitor-command',"
- " 'arguments': {"
- " 'command-line': 'drive_del drive0'"
- "}}");
- g_assert(response);
- g_assert_cmpstr(qdict_get_try_str(response, "return"), ==, "");
- QDECREF(response);
+ g_assert_cmpstr(resp, ==, "");
+ g_free(resp);
}
static void device_del(void)
diff --git a/tests/ide-test.c b/tests/ide-test.c
index 5594738..6173dfa 100644
--- a/tests/ide-test.c
+++ b/tests/ide-test.c
@@ -510,9 +510,7 @@ static void test_flush(void)
tmp_path);
/* Delay the completion of the flush request until we explicitly do it */
- qmp_discard_response("{'execute':'human-monitor-command', 'arguments': {"
- " 'command-line':"
- " 'qemu-io ide0-hd0 \"break flush_to_os A\"'} }");
+ g_free(hmp("qemu-io ide0-hd0 \"break flush_to_os A\""));
/* FLUSH CACHE command on device 0*/
outb(IDE_BASE + reg_device, 0);
@@ -524,9 +522,7 @@ static void test_flush(void)
assert_bit_clear(data, DF | ERR | DRQ);
/* Complete the command */
- qmp_discard_response("{'execute':'human-monitor-command', 'arguments': {"
- " 'command-line':"
- " 'qemu-io ide0-hd0 \"resume A\"'} }");
+ g_free(hmp("qemu-io ide0-hd0 \"resume A\""));
/* Check registers */
data = inb(IDE_BASE + reg_device);
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 8dede56..2a396ba 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -483,6 +483,33 @@ void qtest_qmp_eventwait(QTestState *s, const char *event)
}
}
+char *qtest_hmpv(QTestState *s, const char *fmt, va_list ap)
+{
+ char *cmd;
+ QDict *resp;
+ char *ret;
+
+ cmd = g_strdup_vprintf(fmt, ap);
+ resp = qtest_qmp(s, "{'execute': 'human-monitor-command',"
+ " 'arguments': {'command-line': %s}}",
+ cmd);
+ ret = g_strdup(qdict_get_try_str(resp, "return"));
+ g_assert(ret);
+ QDECREF(resp);
+ g_free(cmd);
+ return ret;
+}
+
+char *qtest_hmp(QTestState *s, const char *fmt, ...)
+{
+ va_list ap;
+ char *ret;
+
+ va_start(ap, fmt);
+ ret = qtest_hmpv(s, fmt, ap);
+ va_end(ap);
+ return ret;
+}
const char *qtest_get_arch(void)
{
@@ -774,6 +801,16 @@ void qmp_discard_response(const char *fmt, ...)
qtest_qmpv_discard_response(global_qtest, fmt, ap);
va_end(ap);
}
+char *hmp(const char *fmt, ...)
+{
+ va_list ap;
+ char *ret;
+
+ va_start(ap, fmt);
+ ret = qtest_hmpv(global_qtest, fmt, ap);
+ va_end(ap);
+ return ret;
+}
bool qtest_big_endian(void)
{
diff --git a/tests/libqtest.h b/tests/libqtest.h
index ec42031..55bccbf 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -120,6 +120,29 @@ QDict *qtest_qmp_receive(QTestState *s);
void qtest_qmp_eventwait(QTestState *s, const char *event);
/**
+ * qtest_hmpv:
+ * @s: #QTestState instance to operate on.
+ * @fmt...: HMP command to send to QEMU
+ *
+ * Send HMP command to QEMU via QMP's human-monitor-command.
+ *
+ * Returns: the command's output. The caller should g_free() it.
+ */
+char *qtest_hmp(QTestState *s, const char *fmt, ...);
+
+/**
+ * qtest_hmpv:
+ * @s: #QTestState instance to operate on.
+ * @fmt: HMP command to send to QEMU
+ * @ap: HMP command arguments
+ *
+ * Send HMP command to QEMU via QMP's human-monitor-command.
+ *
+ * Returns: the command's output. The caller should g_free() it.
+ */
+char *qtest_hmpv(QTestState *s, const char *fmt, va_list ap);
+
+/**
* qtest_get_irq:
* @s: #QTestState instance to operate on.
* @num: Interrupt to observe.
@@ -499,6 +522,16 @@ static inline void qmp_eventwait(const char *event)
}
/**
+ * hmp:
+ * @fmt...: HMP command to send to QEMU
+ *
+ * Send HMP command to QEMU via QMP's human-monitor-command.
+ *
+ * Returns: the command's output. The caller should g_free() it.
+ */
+char *hmp(const char *fmt, ...);
+
+/**
* get_irq:
* @num: Interrupt to observe.
*
--
2.4.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PULL 07/10] device-introspect-test: New, covering device introspection
2015-10-02 17:20 [Qemu-devel] [PULL 00/10] Fix device introspection regressions Markus Armbruster
` (5 preceding siblings ...)
2015-10-02 17:20 ` [Qemu-devel] [PULL 06/10] libqtest: New hmp() & friends Markus Armbruster
@ 2015-10-02 17:20 ` Markus Armbruster
2015-10-02 17:20 ` [Qemu-devel] [PULL 08/10] qmp: Fix device-list-properties not to crash for abstract device Markus Armbruster
` (3 subsequent siblings)
10 siblings, 0 replies; 21+ messages in thread
From: Markus Armbruster @ 2015-10-02 17:20 UTC (permalink / raw)
To: qemu-devel
The test doesn't check that the output makes any sense, only that QEMU
survives. Useful since we've had an astounding number of crash bugs
around there.
In fact, we have a bunch of them right now: several devices crash or
hang, and some leave dangling pointers behind. The test skips testing
the broken parts. The next commits will fix them up, and drop the
skipping.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1443689999-12182-8-git-send-email-armbru@redhat.com>
---
tests/Makefile | 8 ++-
tests/device-introspect-test.c | 158 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 163 insertions(+), 3 deletions(-)
create mode 100644 tests/device-introspect-test.c
diff --git a/tests/Makefile b/tests/Makefile
index 9380e14..2bf7ba1 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -86,7 +86,8 @@ check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
# All QTests for now are POSIX-only, but the dependencies are
# really in libqtest, not in the testcases themselves.
-check-qtest-generic-y =
+check-qtest-generic-y = tests/device-introspect-test$(EXESUF)
+gcov-files-generic-y = qdev-monitor.c qmp.c
gcov-files-ipack-y += hw/ipack/ipack.c
check-qtest-ipack-y += tests/ipoctal232-test$(EXESUF)
@@ -381,6 +382,7 @@ libqos-imx-obj-y = $(libqos-obj-y) tests/libqos/i2c-imx.o
libqos-usb-obj-y = $(libqos-pc-obj-y) tests/libqos/usb.o
libqos-virtio-obj-y = $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o tests/libqos/virtio-mmio.o tests/libqos/malloc-generic.o
+tests/device-introspect-test$(EXESUF): tests/device-introspect-test.o
tests/rtc-test$(EXESUF): tests/rtc-test.o
tests/m48t59-test$(EXESUF): tests/m48t59-test.o
tests/endianness-test$(EXESUF): tests/endianness-test.o
@@ -488,7 +490,7 @@ $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y)
QTEST_QEMU_IMG=qemu-img$(EXESUF) \
MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y) $(check-qtest-generic-y),"GTESTER $@")
- $(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y); do \
+ $(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y) $(gcov-files-generic-y); do \
echo Gcov report for $$f:;\
$(GCOV) $(GCOV_OPTIONS) $$f -o `dirname $$f`; \
done,)
@@ -499,7 +501,7 @@ $(patsubst %, check-%, $(check-unit-y)): check-%: %
$(call quiet-command, \
MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
gtester $(GTESTER_OPTIONS) -m=$(SPEED) $*,"GTESTER $*")
- $(if $(CONFIG_GCOV),@for f in $(gcov-files-$(subst tests/,,$*)-y); do \
+ $(if $(CONFIG_GCOV),@for f in $(gcov-files-$(subst tests/,,$*)-y) $(gcov-files-generic-y); do \
echo Gcov report for $$f:;\
$(GCOV) $(GCOV_OPTIONS) $$f -o `dirname $$f`; \
done,)
diff --git a/tests/device-introspect-test.c b/tests/device-introspect-test.c
new file mode 100644
index 0000000..b08120d
--- /dev/null
+++ b/tests/device-introspect-test.c
@@ -0,0 +1,158 @@
+/*
+ * Device introspection test cases
+ *
+ * Copyright (c) 2015 Red Hat Inc.
+ *
+ * Authors:
+ * Markus Armbruster <armbru@redhat.com>,
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+/*
+ * Covers QMP device-list-properties and HMP device_add help. We
+ * currently don't check that their output makes sense, only that QEMU
+ * survives. Useful since we've had an astounding number of crash
+ * bugs around here.
+ */
+
+#include <glib.h>
+#include <stdarg.h>
+#include "qemu-common.h"
+#include "qapi/qmp/qstring.h"
+#include "libqtest.h"
+
+const char common_args[] = "-nodefaults -machine none";
+
+static QList *device_type_list(bool abstract)
+{
+ QDict *resp;
+ QList *ret;
+
+ resp = qmp("{'execute': 'qom-list-types',"
+ " 'arguments': {'implements': 'device', 'abstract': %i}}",
+ abstract);
+ g_assert(qdict_haskey(resp, "return"));
+ ret = qdict_get_qlist(resp, "return");
+ QINCREF(ret);
+ QDECREF(resp);
+ return ret;
+}
+
+static void test_one_device(const char *type)
+{
+ QDict *resp;
+ char *help, *qom_tree;
+
+ /*
+ * Skip this part for the abstract device test case, because
+ * device-list-properties crashes for such devices.
+ * FIXME fix it not to crash
+ */
+ if (strcmp(type, "device")) {
+ resp = qmp("{'execute': 'device-list-properties',"
+ " 'arguments': {'typename': %s}}",
+ type);
+ QDECREF(resp);
+ }
+
+ help = hmp("device_add \"%s,help\"", type);
+ g_free(help);
+
+ /*
+ * Some devices leave dangling pointers in QOM behind.
+ * "info qom-tree" has a good chance at crashing then
+ */
+ qom_tree = hmp("info qom-tree");
+ g_free(qom_tree);
+}
+
+static void test_device_intro_list(void)
+{
+ QList *types;
+ char *help;
+
+ qtest_start(common_args);
+
+ types = device_type_list(true);
+ QDECREF(types);
+
+ help = hmp("device_add help");
+ g_free(help);
+
+ qtest_end();
+}
+
+static void test_device_intro_none(void)
+{
+ qtest_start(common_args);
+ test_one_device("nonexistent");
+ qtest_end();
+}
+
+static void test_device_intro_abstract(void)
+{
+ qtest_start(common_args);
+ test_one_device("device");
+ qtest_end();
+}
+
+static bool blacklisted(const char *type)
+{
+ static const char *blacklist[] = {
+ /* hang in object_unref(): */
+ "realview_pci", "versatile_pci", "s390-sclp-event-facility", "sclp",
+ /* create a CPU, thus use after free (see below): */
+ "allwinner-a10", "digic", "fsl,imx25", "fsl,imx31", "xlnx,zynqmp",
+ };
+ size_t len = strlen(type);
+ int i;
+
+ if (len >= 4 && !strcmp(type + len - 4, "-cpu")) {
+ /* use after free: cpu_exec_init() saves CPUState in cpus */
+ return true;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(blacklist); i++) {
+ if (!strcmp(blacklist[i], type)) {
+ return true;
+ }
+ }
+ return false;
+}
+
+static void test_device_intro_concrete(void)
+{
+ QList *types;
+ QListEntry *entry;
+ const char *type;
+
+ qtest_start(common_args);
+ types = device_type_list(false);
+
+ QLIST_FOREACH_ENTRY(types, entry) {
+ type = qdict_get_try_str(qobject_to_qdict(qlist_entry_obj(entry)),
+ "name");
+ g_assert(type);
+ if (blacklisted(type)) {
+ continue; /* FIXME broken device, skip */
+ }
+ test_one_device(type);
+ }
+
+ QDECREF(types);
+ qtest_end();
+}
+
+int main(int argc, char **argv)
+{
+ g_test_init(&argc, &argv, NULL);
+
+ qtest_add_func("device/introspect/list", test_device_intro_list);
+ qtest_add_func("device/introspect/none", test_device_intro_none);
+ qtest_add_func("device/introspect/abstract", test_device_intro_abstract);
+ qtest_add_func("device/introspect/concrete", test_device_intro_concrete);
+
+ return g_test_run();
+}
--
2.4.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PULL 08/10] qmp: Fix device-list-properties not to crash for abstract device
2015-10-02 17:20 [Qemu-devel] [PULL 00/10] Fix device introspection regressions Markus Armbruster
` (6 preceding siblings ...)
2015-10-02 17:20 ` [Qemu-devel] [PULL 07/10] device-introspect-test: New, covering device introspection Markus Armbruster
@ 2015-10-02 17:20 ` Markus Armbruster
2015-10-02 17:20 ` [Qemu-devel] [PULL 09/10] qdev: Protect device-list-properties against broken devices Markus Armbruster
` (2 subsequent siblings)
10 siblings, 0 replies; 21+ messages in thread
From: Markus Armbruster @ 2015-10-02 17:20 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable
Broken in commit f4eb32b "qmp: show QOM properties in
device-list-properties", v2.1.
Cc: qemu-stable@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Andreas Färber <afaerber@suse.de>
Message-Id: <1443689999-12182-9-git-send-email-armbru@redhat.com>
---
qmp.c | 6 ++++++
tests/device-introspect-test.c | 15 ++++-----------
2 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/qmp.c b/qmp.c
index 057a7cb..1413de4 100644
--- a/qmp.c
+++ b/qmp.c
@@ -515,6 +515,12 @@ DevicePropertyInfoList *qmp_device_list_properties(const char *typename,
return NULL;
}
+ if (object_class_is_abstract(klass)) {
+ error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "name",
+ "non-abstract device type");
+ return NULL;
+ }
+
obj = object_new(typename);
QTAILQ_FOREACH(prop, &obj->properties, node) {
diff --git a/tests/device-introspect-test.c b/tests/device-introspect-test.c
index b08120d..9cd27f1 100644
--- a/tests/device-introspect-test.c
+++ b/tests/device-introspect-test.c
@@ -45,17 +45,10 @@ static void test_one_device(const char *type)
QDict *resp;
char *help, *qom_tree;
- /*
- * Skip this part for the abstract device test case, because
- * device-list-properties crashes for such devices.
- * FIXME fix it not to crash
- */
- if (strcmp(type, "device")) {
- resp = qmp("{'execute': 'device-list-properties',"
- " 'arguments': {'typename': %s}}",
- type);
- QDECREF(resp);
- }
+ resp = qmp("{'execute': 'device-list-properties',"
+ " 'arguments': {'typename': %s}}",
+ type);
+ QDECREF(resp);
help = hmp("device_add \"%s,help\"", type);
g_free(help);
--
2.4.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PULL 09/10] qdev: Protect device-list-properties against broken devices
2015-10-02 17:20 [Qemu-devel] [PULL 00/10] Fix device introspection regressions Markus Armbruster
` (7 preceding siblings ...)
2015-10-02 17:20 ` [Qemu-devel] [PULL 08/10] qmp: Fix device-list-properties not to crash for abstract device Markus Armbruster
@ 2015-10-02 17:20 ` Markus Armbruster
2015-10-02 17:20 ` [Qemu-devel] [PULL 10/10] Revert "qdev: Use qdev_get_device_class() for -device <type>, help" Markus Armbruster
2015-10-04 21:24 ` [Qemu-devel] [PULL 00/10] Fix device introspection regressions Peter Maydell
10 siblings, 0 replies; 21+ messages in thread
From: Markus Armbruster @ 2015-10-02 17:20 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Richard Henderson, Eduardo Habkost, Max Filippov,
Bastian Koppelmann, Anthony Green, Mark Cave-Ayland,
Alexander Graf, qemu-stable, Blue Swirl, Christian Borntraeger,
Michael Walle, qemu-ppc, Paolo Bonzini, Cornelia Huck,
Edgar E. Iglesias, Guan Xuetao, Leon Alrae, Andreas Färber,
Aurelien Jarno, Jia Liu
Several devices don't survive object_unref(object_new(T)): they crash
or hang during cleanup, or they leave dangling pointers behind.
This breaks at least device-list-properties, because
qmp_device_list_properties() needs to create a device to find its
properties. Broken in commit f4eb32b "qmp: show QOM properties in
device-list-properties", v2.1. Example reproducer:
$ qemu-system-aarch64 -nodefaults -display none -machine none -S -qmp stdio
{"QMP": {"version": {"qemu": {"micro": 50, "minor": 4, "major": 2}, "package": ""}, "capabilities": []}}
{ "execute": "qmp_capabilities" }
{"return": {}}
{ "execute": "device-list-properties", "arguments": { "typename": "pxa2xx-pcmcia" } }
qemu-system-aarch64: /home/armbru/work/qemu/memory.c:1307: memory_region_finalize: Assertion `((&mr->subregions)->tqh_first == ((void *)0))' failed.
Aborted (core dumped)
[Exit 134 (SIGABRT)]
Unfortunately, I can't fix the problems in these devices right now.
Instead, add DeviceClass member cannot_destroy_with_object_finalize_yet
to mark them:
* Hang during cleanup (didn't debug, so I can't say why):
"realview_pci", "versatile_pci", "s390-sclp-event-facility", "sclp"
* Dangling pointer in cpus: most CPUs, plus "allwinner-a10", "digic",
"fsl,imx25", "fsl,imx31", "xlnx,zynqmp", because they create such
CPUs
* Assert kvm_enabled(): "host-x86_64-cpu", host-i386-cpu",
"host-powerpc64-cpu", "host-embedded-powerpc-cpu",
"host-powerpc-cpu" (the powerpc ones can't currently reach the
assertion, because the CPUs are only registered when KVM is enabled,
but the assertion is arguably in the wrong place all the same)
Make qmp_device_list_properties() fail cleanly when the device is so
marked. This improves device-list-properties from "crashes, hangs or
leaves dangling pointers behind" to "fails". Not a complete fix, just
a better-than-nothing work-around. In the above reproducer,
device-list-properties now fails with "Can't list properties of device
'pxa2xx-pcmcia'".
This also protects -device FOO,help, which uses the same machinery
since commit ef52358 "qdev-monitor: include QOM properties in -device
FOO, help output", v2.2. Example reproducer:
$ qemu-system-aarch64 -machine none -device pxa2xx-pcmcia,help
Before:
qemu-system-aarch64: .../memory.c:1307: memory_region_finalize: Assertion `((&mr->subregions)->tqh_first == ((void *)0))' failed.
After:
Can't list properties of device 'pxa2xx-pcmcia'
Cc: "Andreas Färber" <afaerber@suse.de>
Cc: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
Cc: Alexander Graf <agraf@suse.de>
Cc: Anthony Green <green@moxielogic.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Cc: Blue Swirl <blauwirbel@gmail.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Guan Xuetao <gxt@mprc.pku.edu.cn>
Cc: Jia Liu <proljc@gmail.com>
Cc: Leon Alrae <leon.alrae@imgtec.com>
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Cc: Michael Walle <michael@walle.cc>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Richard Henderson <rth@twiddle.net>
Cc: qemu-ppc@nongnu.org
Cc: qemu-stable@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Message-Id: <1443689999-12182-10-git-send-email-armbru@redhat.com>
---
hw/arm/allwinner-a10.c | 6 ++++++
hw/arm/digic.c | 6 ++++++
hw/arm/fsl-imx25.c | 6 ++++++
hw/arm/fsl-imx31.c | 6 ++++++
hw/arm/xlnx-zynqmp.c | 6 ++++++
hw/pci-host/versatile.c | 11 +++++++++++
hw/s390x/event-facility.c | 3 +++
hw/s390x/sclp.c | 3 +++
include/hw/qdev-core.h | 13 +++++++++++++
qmp.c | 5 +++++
target-alpha/cpu.c | 7 +++++++
target-arm/cpu.c | 11 +++++++++++
target-cris/cpu.c | 7 +++++++
target-i386/cpu.c | 8 ++++++++
target-lm32/cpu.c | 7 +++++++
target-m68k/cpu.c | 7 +++++++
target-microblaze/cpu.c | 6 ++++++
target-mips/cpu.c | 7 +++++++
target-moxie/cpu.c | 7 +++++++
target-openrisc/cpu.c | 7 +++++++
target-ppc/kvm.c | 4 ++++
target-s390x/cpu.c | 7 +++++++
target-sh4/cpu.c | 7 +++++++
target-sparc/cpu.c | 7 +++++++
target-tilegx/cpu.c | 7 +++++++
target-tricore/cpu.c | 6 ++++++
target-unicore32/cpu.c | 7 +++++++
target-xtensa/cpu.c | 7 +++++++
tests/device-introspect-test.c | 27 ---------------------------
29 files changed, 191 insertions(+), 27 deletions(-)
diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
index ff249af..43dc0a1 100644
--- a/hw/arm/allwinner-a10.c
+++ b/hw/arm/allwinner-a10.c
@@ -103,6 +103,12 @@ static void aw_a10_class_init(ObjectClass *oc, void *data)
DeviceClass *dc = DEVICE_CLASS(oc);
dc->realize = aw_a10_realize;
+
+ /*
+ * Reason: creates an ARM CPU, thus use after free(), see
+ * arm_cpu_class_init()
+ */
+ dc->cannot_destroy_with_object_finalize_yet = true;
}
static const TypeInfo aw_a10_type_info = {
diff --git a/hw/arm/digic.c b/hw/arm/digic.c
index ec8c330..90f8190 100644
--- a/hw/arm/digic.c
+++ b/hw/arm/digic.c
@@ -97,6 +97,12 @@ static void digic_class_init(ObjectClass *oc, void *data)
DeviceClass *dc = DEVICE_CLASS(oc);
dc->realize = digic_realize;
+
+ /*
+ * Reason: creates an ARM CPU, thus use after free(), see
+ * arm_cpu_class_init()
+ */
+ dc->cannot_destroy_with_object_finalize_yet = true;
}
static const TypeInfo digic_type_info = {
diff --git a/hw/arm/fsl-imx25.c b/hw/arm/fsl-imx25.c
index 86fde42..e1cadac 100644
--- a/hw/arm/fsl-imx25.c
+++ b/hw/arm/fsl-imx25.c
@@ -284,6 +284,12 @@ static void fsl_imx25_class_init(ObjectClass *oc, void *data)
DeviceClass *dc = DEVICE_CLASS(oc);
dc->realize = fsl_imx25_realize;
+
+ /*
+ * Reason: creates an ARM CPU, thus use after free(), see
+ * arm_cpu_class_init()
+ */
+ dc->cannot_destroy_with_object_finalize_yet = true;
}
static const TypeInfo fsl_imx25_type_info = {
diff --git a/hw/arm/fsl-imx31.c b/hw/arm/fsl-imx31.c
index 8e1ed48..53d4473 100644
--- a/hw/arm/fsl-imx31.c
+++ b/hw/arm/fsl-imx31.c
@@ -258,6 +258,12 @@ static void fsl_imx31_class_init(ObjectClass *oc, void *data)
DeviceClass *dc = DEVICE_CLASS(oc);
dc->realize = fsl_imx31_realize;
+
+ /*
+ * Reason: creates an ARM CPU, thus use after free(), see
+ * arm_cpu_class_init()
+ */
+ dc->cannot_destroy_with_object_finalize_yet = true;
}
static const TypeInfo fsl_imx31_type_info = {
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index a9097f9..b36ca3d 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -271,6 +271,12 @@ static void xlnx_zynqmp_class_init(ObjectClass *oc, void *data)
dc->props = xlnx_zynqmp_props;
dc->realize = xlnx_zynqmp_realize;
+
+ /*
+ * Reason: creates an ARM CPU, thus use after free(), see
+ * arm_cpu_class_init()
+ */
+ dc->cannot_destroy_with_object_finalize_yet = true;
}
static const TypeInfo xlnx_zynqmp_type_info = {
diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c
index 6d23553..7172b90 100644
--- a/hw/pci-host/versatile.c
+++ b/hw/pci-host/versatile.c
@@ -500,6 +500,8 @@ static void pci_vpb_class_init(ObjectClass *klass, void *data)
dc->reset = pci_vpb_reset;
dc->vmsd = &pci_vpb_vmstate;
dc->props = pci_vpb_properties;
+ /* Reason: object_unref() hangs */
+ dc->cannot_destroy_with_object_finalize_yet = true;
}
static const TypeInfo pci_vpb_info = {
@@ -521,10 +523,19 @@ static void pci_realview_init(Object *obj)
s->mem_win_size[2] = 0x08000000;
}
+static void pci_realview_class_init(ObjectClass *class, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(class);
+
+ /* Reason: object_unref() hangs */
+ dc->cannot_destroy_with_object_finalize_yet = true;
+}
+
static const TypeInfo pci_realview_info = {
.name = "realview_pci",
.parent = TYPE_VERSATILE_PCI,
.instance_init = pci_realview_init,
+ .class_init = pci_realview_class_init,
};
static void versatile_pci_register_types(void)
diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index ef2a051..d90711b 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -381,6 +381,9 @@ static void init_event_facility_class(ObjectClass *klass, void *data)
set_bit(DEVICE_CATEGORY_MISC, dc->categories);
k->command_handler = command_handler;
k->event_pending = event_pending;
+
+ /* Reason: object_unref() hangs */
+ dc->cannot_destroy_with_object_finalize_yet = true;
}
static const TypeInfo sclp_event_facility_info = {
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index a061b49..53048c1 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -563,6 +563,9 @@ static void sclp_class_init(ObjectClass *oc, void *data)
sc->read_cpu_info = sclp_read_cpu_info;
sc->execute = sclp_execute;
sc->service_interrupt = service_interrupt;
+
+ /* Reason: object_unref() hangs */
+ dc->cannot_destroy_with_object_finalize_yet = true;
}
static TypeInfo sclp_info = {
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 038b54d..8057aed 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -114,6 +114,19 @@ typedef struct DeviceClass {
* TODO remove once we're there
*/
bool cannot_instantiate_with_device_add_yet;
+ /*
+ * Does this device model survive object_unref(object_new(TNAME))?
+ * All device models should, and this flag shouldn't exist. Some
+ * devices crash in object_new(), some crash or hang in
+ * object_unref(). Makes introspecting properties with
+ * qmp_device_list_properties() dangerous. Bad, because it's used
+ * by -device FOO,help. This flag serves to protect that code.
+ * It should never be set without a comment explaining why it is
+ * set.
+ * TODO remove once we're there
+ */
+ bool cannot_destroy_with_object_finalize_yet;
+
bool hotpluggable;
/* callbacks */
diff --git a/qmp.c b/qmp.c
index 1413de4..d9ecede 100644
--- a/qmp.c
+++ b/qmp.c
@@ -521,6 +521,11 @@ DevicePropertyInfoList *qmp_device_list_properties(const char *typename,
return NULL;
}
+ if (DEVICE_CLASS(klass)->cannot_destroy_with_object_finalize_yet) {
+ error_setg(errp, "Can't list properties of device '%s'", typename);
+ return NULL;
+ }
+
obj = object_new(typename);
QTAILQ_FOREACH(prop, &obj->properties, node) {
diff --git a/target-alpha/cpu.c b/target-alpha/cpu.c
index 421d7e5..ff1926a 100644
--- a/target-alpha/cpu.c
+++ b/target-alpha/cpu.c
@@ -298,6 +298,13 @@ static void alpha_cpu_class_init(ObjectClass *oc, void *data)
dc->vmsd = &vmstate_alpha_cpu;
#endif
cc->gdb_num_core_regs = 67;
+
+ /*
+ * Reason: alpha_cpu_initfn() calls cpu_exec_init(), which saves
+ * the object in cpus -> dangling pointer after final
+ * object_unref().
+ */
+ dc->cannot_destroy_with_object_finalize_yet = true;
}
static const TypeInfo alpha_cpu_type_info = {
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index d7b4445..30739fc 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -1427,6 +1427,17 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
cc->debug_excp_handler = arm_debug_excp_handler;
cc->disas_set_info = arm_disas_set_info;
+
+ /*
+ * Reason: arm_cpu_initfn() calls cpu_exec_init(), which saves
+ * the object in cpus -> dangling pointer after final
+ * object_unref().
+ *
+ * Once this is fixed, the devices that create ARM CPUs should be
+ * updated not to set cannot_destroy_with_object_finalize_yet,
+ * unless they still screw up something else.
+ */
+ dc->cannot_destroy_with_object_finalize_yet = true;
}
static void cpu_register(const ARMCPUInfo *info)
diff --git a/target-cris/cpu.c b/target-cris/cpu.c
index d461e07..8eaf5a5 100644
--- a/target-cris/cpu.c
+++ b/target-cris/cpu.c
@@ -309,6 +309,13 @@ static void cris_cpu_class_init(ObjectClass *oc, void *data)
cc->gdb_stop_before_watchpoint = true;
cc->disas_set_info = cris_disas_set_info;
+
+ /*
+ * Reason: cris_cpu_initfn() calls cpu_exec_init(), which saves
+ * the object in cpus -> dangling pointer after final
+ * object_unref().
+ */
+ dc->cannot_destroy_with_object_finalize_yet = true;
}
static const TypeInfo cris_cpu_type_info = {
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index bd411b9..ccb97e2 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1449,6 +1449,8 @@ static void host_x86_cpu_class_init(ObjectClass *oc, void *data)
*/
dc->props = host_x86_cpu_properties;
+ /* Reason: host_x86_cpu_initfn() dies when !kvm_enabled() */
+ dc->cannot_destroy_with_object_finalize_yet = true;
}
static void host_x86_cpu_initfn(Object *obj)
@@ -3169,6 +3171,12 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
#endif
cc->cpu_exec_enter = x86_cpu_exec_enter;
cc->cpu_exec_exit = x86_cpu_exec_exit;
+
+ /*
+ * Reason: x86_cpu_initfn() calls cpu_exec_init(), which saves the
+ * object in cpus -> dangling pointer after final object_unref().
+ */
+ dc->cannot_destroy_with_object_finalize_yet = true;
}
static const TypeInfo x86_cpu_type_info = {
diff --git a/target-lm32/cpu.c b/target-lm32/cpu.c
index c2b77c6..d0ab278 100644
--- a/target-lm32/cpu.c
+++ b/target-lm32/cpu.c
@@ -275,6 +275,13 @@ static void lm32_cpu_class_init(ObjectClass *oc, void *data)
cc->gdb_num_core_regs = 32 + 7;
cc->gdb_stop_before_watchpoint = true;
cc->debug_excp_handler = lm32_debug_excp_handler;
+
+ /*
+ * Reason: lm32_cpu_initfn() calls cpu_exec_init(), which saves
+ * the object in cpus -> dangling pointer after final
+ * object_unref().
+ */
+ dc->cannot_destroy_with_object_finalize_yet = true;
}
static void lm32_register_cpu_type(const LM32CPUInfo *info)
diff --git a/target-m68k/cpu.c b/target-m68k/cpu.c
index 4f246da..97527ef 100644
--- a/target-m68k/cpu.c
+++ b/target-m68k/cpu.c
@@ -212,6 +212,13 @@ static void m68k_cpu_class_init(ObjectClass *c, void *data)
dc->vmsd = &vmstate_m68k_cpu;
cc->gdb_num_core_regs = 18;
cc->gdb_core_xml_file = "cf-core.xml";
+
+ /*
+ * Reason: m68k_cpu_initfn() calls cpu_exec_init(), which saves
+ * the object in cpus -> dangling pointer after final
+ * object_unref().
+ */
+ dc->cannot_destroy_with_object_finalize_yet = true;
}
static void register_cpu_type(const M68kCPUInfo *info)
diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c
index cbd84a2..52959e1 100644
--- a/target-microblaze/cpu.c
+++ b/target-microblaze/cpu.c
@@ -264,6 +264,12 @@ static void mb_cpu_class_init(ObjectClass *oc, void *data)
cc->gdb_num_core_regs = 32 + 5;
cc->disas_set_info = mb_disas_set_info;
+
+ /*
+ * Reason: mb_cpu_initfn() calls cpu_exec_init(), which saves the
+ * object in cpus -> dangling pointer after final object_unref().
+ */
+ dc->cannot_destroy_with_object_finalize_yet = true;
}
static const TypeInfo mb_cpu_type_info = {
diff --git a/target-mips/cpu.c b/target-mips/cpu.c
index 4027d0f..7fe1f04 100644
--- a/target-mips/cpu.c
+++ b/target-mips/cpu.c
@@ -153,6 +153,13 @@ static void mips_cpu_class_init(ObjectClass *c, void *data)
cc->gdb_num_core_regs = 73;
cc->gdb_stop_before_watchpoint = true;
+
+ /*
+ * Reason: mips_cpu_initfn() calls cpu_exec_init(), which saves
+ * the object in cpus -> dangling pointer after final
+ * object_unref().
+ */
+ dc->cannot_destroy_with_object_finalize_yet = true;
}
static const TypeInfo mips_cpu_type_info = {
diff --git a/target-moxie/cpu.c b/target-moxie/cpu.c
index 6b035aa..3af3779 100644
--- a/target-moxie/cpu.c
+++ b/target-moxie/cpu.c
@@ -114,6 +114,13 @@ static void moxie_cpu_class_init(ObjectClass *oc, void *data)
cc->get_phys_page_debug = moxie_cpu_get_phys_page_debug;
cc->vmsd = &vmstate_moxie_cpu;
#endif
+
+ /*
+ * Reason: moxie_cpu_initfn() calls cpu_exec_init(), which saves
+ * the object in cpus -> dangling pointer after final
+ * object_unref().
+ */
+ dc->cannot_destroy_with_object_finalize_yet = true;
}
static void moxielite_initfn(Object *obj)
diff --git a/target-openrisc/cpu.c b/target-openrisc/cpu.c
index d97f3c0..cc5e2d1 100644
--- a/target-openrisc/cpu.c
+++ b/target-openrisc/cpu.c
@@ -177,6 +177,13 @@ static void openrisc_cpu_class_init(ObjectClass *oc, void *data)
dc->vmsd = &vmstate_openrisc_cpu;
#endif
cc->gdb_num_core_regs = 32 + 3;
+
+ /*
+ * Reason: openrisc_cpu_initfn() calls cpu_exec_init(), which saves
+ * the object in cpus -> dangling pointer after final
+ * object_unref().
+ */
+ dc->cannot_destroy_with_object_finalize_yet = true;
}
static void cpu_register(const OpenRISCCPUInfo *info)
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index e641680..96e8264 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -2193,6 +2193,7 @@ static void kvmppc_host_cpu_initfn(Object *obj)
static void kvmppc_host_cpu_class_init(ObjectClass *oc, void *data)
{
+ DeviceClass *dc = DEVICE_CLASS(oc);
PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
uint32_t vmx = kvmppc_get_vmx();
uint32_t dfp = kvmppc_get_dfp();
@@ -2219,6 +2220,9 @@ static void kvmppc_host_cpu_class_init(ObjectClass *oc, void *data)
if (icache_size != -1) {
pcc->l1_icache_size = icache_size;
}
+
+ /* Reason: kvmppc_host_cpu_initfn() dies when !kvm_enabled() */
+ dc->cannot_destroy_with_object_finalize_yet = true;
}
bool kvmppc_has_cap_epr(void)
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index c3e21b4..ccfaa8a 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -353,6 +353,13 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
#endif
cc->gdb_num_core_regs = S390_NUM_CORE_REGS;
cc->gdb_core_xml_file = "s390x-core64.xml";
+
+ /*
+ * Reason: s390_cpu_initfn() calls cpu_exec_init(), which saves
+ * the object in cpus -> dangling pointer after final
+ * object_unref().
+ */
+ dc->cannot_destroy_with_object_finalize_yet = true;
}
static const TypeInfo s390_cpu_type_info = {
diff --git a/target-sh4/cpu.c b/target-sh4/cpu.c
index 5c65ab4..64e4467 100644
--- a/target-sh4/cpu.c
+++ b/target-sh4/cpu.c
@@ -290,6 +290,13 @@ static void superh_cpu_class_init(ObjectClass *oc, void *data)
#endif
dc->vmsd = &vmstate_sh_cpu;
cc->gdb_num_core_regs = 59;
+
+ /*
+ * Reason: superh_cpu_initfn() calls cpu_exec_init(), which saves
+ * the object in cpus -> dangling pointer after final
+ * object_unref().
+ */
+ dc->cannot_destroy_with_object_finalize_yet = true;
}
static const TypeInfo superh_cpu_type_info = {
diff --git a/target-sparc/cpu.c b/target-sparc/cpu.c
index 9528e3a..82bb72a 100644
--- a/target-sparc/cpu.c
+++ b/target-sparc/cpu.c
@@ -854,6 +854,13 @@ static void sparc_cpu_class_init(ObjectClass *oc, void *data)
#else
cc->gdb_num_core_regs = 72;
#endif
+
+ /*
+ * Reason: sparc_cpu_initfn() calls cpu_exec_init(), which saves
+ * the object in cpus -> dangling pointer after final
+ * object_unref().
+ */
+ dc->cannot_destroy_with_object_finalize_yet = true;
}
static const TypeInfo sparc_cpu_type_info = {
diff --git a/target-tilegx/cpu.c b/target-tilegx/cpu.c
index 78b73e4..cfcc0ac 100644
--- a/target-tilegx/cpu.c
+++ b/target-tilegx/cpu.c
@@ -154,6 +154,13 @@ static void tilegx_cpu_class_init(ObjectClass *oc, void *data)
cc->set_pc = tilegx_cpu_set_pc;
cc->handle_mmu_fault = tilegx_cpu_handle_mmu_fault;
cc->gdb_num_core_regs = 0;
+
+ /*
+ * Reason: tilegx_cpu_initfn() calls cpu_exec_init(), which saves
+ * the object in cpus -> dangling pointer after final
+ * object_unref().
+ */
+ dc->cannot_destroy_with_object_finalize_yet = true;
}
static const TypeInfo tilegx_cpu_type_info = {
diff --git a/target-tricore/cpu.c b/target-tricore/cpu.c
index 2029ef6..ed8b030 100644
--- a/target-tricore/cpu.c
+++ b/target-tricore/cpu.c
@@ -170,6 +170,12 @@ static void tricore_cpu_class_init(ObjectClass *c, void *data)
cc->set_pc = tricore_cpu_set_pc;
cc->synchronize_from_tb = tricore_cpu_synchronize_from_tb;
+ /*
+ * Reason: tricore_cpu_initfn() calls cpu_exec_init(), which saves
+ * the object in cpus -> dangling pointer after final
+ * object_unref().
+ */
+ dc->cannot_destroy_with_object_finalize_yet = true;
}
static void cpu_register(const TriCoreCPUInfo *info)
diff --git a/target-unicore32/cpu.c b/target-unicore32/cpu.c
index fc451a1..e5252eb 100644
--- a/target-unicore32/cpu.c
+++ b/target-unicore32/cpu.c
@@ -155,6 +155,13 @@ static void uc32_cpu_class_init(ObjectClass *oc, void *data)
cc->get_phys_page_debug = uc32_cpu_get_phys_page_debug;
#endif
dc->vmsd = &vmstate_uc32_cpu;
+
+ /*
+ * Reason: uc32_cpu_initfn() calls cpu_exec_init(), which saves
+ * the object in cpus -> dangling pointer after final
+ * object_unref().
+ */
+ dc->cannot_destroy_with_object_finalize_yet = true;
}
static void uc32_register_cpu_type(const UniCore32CPUInfo *info)
diff --git a/target-xtensa/cpu.c b/target-xtensa/cpu.c
index da8129d..4e49bee 100644
--- a/target-xtensa/cpu.c
+++ b/target-xtensa/cpu.c
@@ -155,6 +155,13 @@ static void xtensa_cpu_class_init(ObjectClass *oc, void *data)
#endif
cc->debug_excp_handler = xtensa_breakpoint_handler;
dc->vmsd = &vmstate_xtensa_cpu;
+
+ /*
+ * Reason: xtensa_cpu_initfn() calls cpu_exec_init(), which saves
+ * the object in cpus -> dangling pointer after final
+ * object_unref().
+ */
+ dc->cannot_destroy_with_object_finalize_yet = true;
}
static const TypeInfo xtensa_cpu_type_info = {
diff --git a/tests/device-introspect-test.c b/tests/device-introspect-test.c
index 9cd27f1..11d5fea 100644
--- a/tests/device-introspect-test.c
+++ b/tests/device-introspect-test.c
@@ -91,30 +91,6 @@ static void test_device_intro_abstract(void)
qtest_end();
}
-static bool blacklisted(const char *type)
-{
- static const char *blacklist[] = {
- /* hang in object_unref(): */
- "realview_pci", "versatile_pci", "s390-sclp-event-facility", "sclp",
- /* create a CPU, thus use after free (see below): */
- "allwinner-a10", "digic", "fsl,imx25", "fsl,imx31", "xlnx,zynqmp",
- };
- size_t len = strlen(type);
- int i;
-
- if (len >= 4 && !strcmp(type + len - 4, "-cpu")) {
- /* use after free: cpu_exec_init() saves CPUState in cpus */
- return true;
- }
-
- for (i = 0; i < ARRAY_SIZE(blacklist); i++) {
- if (!strcmp(blacklist[i], type)) {
- return true;
- }
- }
- return false;
-}
-
static void test_device_intro_concrete(void)
{
QList *types;
@@ -128,9 +104,6 @@ static void test_device_intro_concrete(void)
type = qdict_get_try_str(qobject_to_qdict(qlist_entry_obj(entry)),
"name");
g_assert(type);
- if (blacklisted(type)) {
- continue; /* FIXME broken device, skip */
- }
test_one_device(type);
}
--
2.4.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PULL 10/10] Revert "qdev: Use qdev_get_device_class() for -device <type>, help"
2015-10-02 17:20 [Qemu-devel] [PULL 00/10] Fix device introspection regressions Markus Armbruster
` (8 preceding siblings ...)
2015-10-02 17:20 ` [Qemu-devel] [PULL 09/10] qdev: Protect device-list-properties against broken devices Markus Armbruster
@ 2015-10-02 17:20 ` Markus Armbruster
2015-10-04 21:24 ` [Qemu-devel] [PULL 00/10] Fix device introspection regressions Peter Maydell
10 siblings, 0 replies; 21+ messages in thread
From: Markus Armbruster @ 2015-10-02 17:20 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable
This reverts commit 31bed5509dfcbdfc293154ce81086a4dbd7a80b6.
The reverted commit changed qdev_device_help() to reject abstract
devices and devices that have cannot_instantiate_with_device_add_yet
set, to fix crash bugs like -device x86_64-cpu,help.
Rejecting abstract devices makes sense: they're purely internal, and
the implementation of the help feature can't cope with them.
Rejecting non-pluggable devices makes less sense: even though you
can't use them with -device, the help may still be useful elsewhere,
for instance with -global. This is a regression: -device FOO,help
used to help even for FOO that aren't pluggable.
The previous two commits fixed the crash bug at a lower layer, so
reverting this one is now safe. Fixes the -device FOO,help
regression, except for the broken devices marked
cannot_even_create_with_object_new_yet. For those, the error message
is improved.
Example of a device where the regression is fixed:
$ qemu-system-x86_64 -device PIIX4_PM,help
PIIX4_PM.command_serr_enable=bool (on/off)
PIIX4_PM.multifunction=bool (on/off)
PIIX4_PM.rombar=uint32
PIIX4_PM.romfile=str
PIIX4_PM.addr=int32 (Slot and optional function number, example: 06.0 or 06)
PIIX4_PM.memory-hotplug-support=bool
PIIX4_PM.acpi-pci-hotplug-with-bridge-support=bool
PIIX4_PM.s4_val=uint8
PIIX4_PM.disable_s4=uint8
PIIX4_PM.disable_s3=uint8
PIIX4_PM.smb_io_base=uint32
Example of a device where it isn't fixed:
$ qemu-system-x86_64 -device host-x86_64-cpu,help
Can't list properties of device 'host-x86_64-cpu'
Both failed with "Parameter 'driver' expects pluggable device type"
before.
Cc: qemu-stable@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Message-Id: <1443689999-12182-11-git-send-email-armbru@redhat.com>
---
qdev-monitor.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/qdev-monitor.c b/qdev-monitor.c
index eb7aef2..1cadefb 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -237,9 +237,12 @@ int qdev_device_help(QemuOpts *opts)
return 0;
}
- qdev_get_device_class(&driver, &local_err);
- if (local_err) {
- goto error;
+ if (!object_class_by_name(driver)) {
+ const char *typename = find_typename_by_alias(driver);
+
+ if (typename) {
+ driver = typename;
+ }
}
prop_list = qmp_device_list_properties(driver, &local_err);
--
2.4.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PULL 00/10] Fix device introspection regressions
2015-10-02 17:20 [Qemu-devel] [PULL 00/10] Fix device introspection regressions Markus Armbruster
` (9 preceding siblings ...)
2015-10-02 17:20 ` [Qemu-devel] [PULL 10/10] Revert "qdev: Use qdev_get_device_class() for -device <type>, help" Markus Armbruster
@ 2015-10-04 21:24 ` Peter Maydell
2015-10-05 6:49 ` Markus Armbruster
10 siblings, 1 reply; 21+ messages in thread
From: Peter Maydell @ 2015-10-04 21:24 UTC (permalink / raw)
To: Markus Armbruster; +Cc: QEMU Developers
On 2 October 2015 at 18:20, Markus Armbruster <armbru@redhat.com> wrote:
> QMP command device-list-properties regressed in 2.1: it can crash or
> leave dangling pointers behind.
>
> -device FOO,help regressed in 2.2: it no longer works for
> non-pluggable devices. I tried to fix that some time ago[*], but my
> fix failed review. This is my second, more comprehensive try.
>
> PATCH 1-3 fix one class of bugs involved in the regressions, PATCH 4-5
> are libqtest preliminaries, PATCH 6 adds tests to demonstrate the
> remaining bugs, PATCH 7-9 fix them to a degree (see PATCH 8 for
> limitations), and PATCH 10 cleans up.
This ordering breaks bisection of 'make check', as I found out when
I tried to figure out which of the patches in this pull was causing
an OSX test failure. Please can you reorder them so that 'make check'
works at all points in the series?
> The following changes since commit ff770b07f34d28b79013a83989bd6c85f8f16b2f:
>
> Merge remote-tracking branch 'remotes/cody/tags/block-pull-request' into staging (2015-10-02 11:01:18 +0100)
>
> are available in the git repository at:
>
> git://repo.or.cz/qemu/armbru.git tags/pull-monitor-2015-10-02
>
> for you to fetch changes up to e927162a6fa2fa6144de9d1d11cc9448a2143671:
>
> Revert "qdev: Use qdev_get_device_class() for -device <type>,help" (2015-10-02 16:45:53 +0200)
>
> ----------------------------------------------------------------
> Fix device introspection regressions
>
> ----------------------------------------------------------------
'make check' failure on OSX:
/aarch64/device/introspect/list: OK
/aarch64/device/introspect/none: OK
/aarch64/device/introspect/abstract: OK
/aarch64/device/introspect/concrete: **
ERROR:/Users/pm215/src/qemu-for-merges/qom/object.c:333:void
object_initialize_with_type(void *, size_t, TypeImpl *): assertion
failed: (type != NULL)
Broken pipe
FAIL
I have no idea why this only failed on OSX...
Backtrace:
(gdb) bt
#0 0x00007fff9145e286 in __pthread_kill ()
#1 0x00007fff912529f9 in pthread_kill ()
#2 0x00007fff956db9b3 in abort ()
#3 0x0000000110d21c50 in g_assertion_message ()
#4 0x0000000110d21c95 in g_assertion_message_expr ()
#5 0x00000001102909ad in object_initialize_with_type (data=<value
temporarily unavailable, due to optimizations>, size=<value
temporarily unavailable, due to optimizations>, type=<value
temporarily unavailable, due to optimizations>) at
/Users/pm215/src/qemu-for-merges/qom/object.c:333
#6 0x000000011007513b in virtio_instance_init_common
(proxy_obj=0x7ffae2841000, data=0x7ffae2849120, vdev_size=6,
vdev_name=0x0) at
/Users/pm215/src/qemu-for-merges/hw/virtio/virtio.c:1468
#7 0x00000001102908ee in type_get_parent [inlined] () at
/Users/pm215/src/qemu-for-merges/qom/object.c:344
#8 type_get_by_name [inlined] () at
/Users/pm215/src/qemu-for-merges/qom/object.c:325
#9 type_table_lookup [inlined] () at
/Users/pm215/src/qemu-for-merges/qom/object.c:165
#10 type_table_get [inlined] () at
/Users/pm215/src/qemu-for-merges/qom/object.c:159
#11 object_post_init_with_type [inlined] () at
/Users/pm215/src/qemu-for-merges/qom/object.c:93
#12 0x00000001102908ee in object_initialize_with_type
(data=0x7ffae2841000, size=<value temporarily unavailable, due to
optimizations>, type=0x7ffae1547be0) at
/Users/pm215/src/qemu-for-merges/qom/object.c:345
#13 0x000000011029124b in object_new_with_type [inlined] () at
/Users/pm215/src/qemu-for-merges/qom/object.c:430
#14 0x000000011029124b in object_new (typename=<value temporarily
unavailable, due to optimizations>) at
/Users/pm215/src/qemu-for-merges/qom/object.c:440
#15 0x000000011013ec1c in qmp_device_list_properties
(typename=0x7ffae1608ac0 "virtio-tablet-pci", errp=<value temporarily
unavailable, due to optimizations>) at
/Users/pm215/src/qemu-for-merges/qmp.c:529
#16 0x0000000110136987 in qmp_marshal_device_list_properties
(args=<value temporarily unavailable, due to optimizations>,
ret=0x7fff4fc26198, errp=0x7fff4fc261a0) at qmp-marshal.c:1693
#17 0x000000011000f6c5 in handle_qmp_command (parser=<value
temporarily unavailable, due to optimizations>, tokens=<value
temporarily unavailable, due to optimizations>) at
/Users/pm215/src/qemu-for-merges/monitor.c:3860
#18 0x00000001103206ba in json_message_process_token (lexer=<value
temporarily unavailable, due to optimizations>, token=<value
temporarily unavailable, due to optimizations>, type=<value
temporarily unavailable, due to optimizations>, x=<value temporarily
unavailable, due to optimizations>, y=0) at
/Users/pm215/src/qemu-for-merges/qobject/json-streamer.c:87
#19 0x0000000110320367 in json_lexer_feed_char () at
/Users/pm215/src/qemu-for-merges/qobject/json-lexer.c:303
#20 0x00000001103202ad in json_lexer_feed (lexer=0x7ffae1600d70,
buffer=<value temporarily unavailable, due to optimizations>,
size=<value temporarily unavailable, due to optimizations>) at
/Users/pm215/src/qemu-for-merges/qobject/json-lexer.c:356
#21 0x000000011000eb00 in monitor_qmp_read (opaque=<value temporarily
unavailable, due to optimizations>, buf=0x7d <Address 0x7d out of
bounds>, size=<value temporarily unavailable, due to optimizations>)
at /Users/pm215/src/qemu-for-merges/monitor.c:3875
#22 0x0000000110126e5a in qemu_chr_be_write [inlined] () at
/Users/pm215/src/qemu-for-merges/qemu-char.c:305
#23 0x0000000110126e5a in tcp_chr_read (chan=<value temporarily
unavailable, due to optimizations>, cond=<value temporarily
unavailable, due to optimizations>, opaque=0x7ffae1576ac0) at
/Users/pm215/src/qemu-for-merges/qemu-char.c:2873
#24 0x0000000110d020bd in g_main_context_dispatch ()
#25 0x00000001102a19a2 in main_loop_wait (nonblocking=<value
temporarily unavailable, due to optimizations>) at
/Users/pm215/src/qemu-for-merges/main-loop.c:211
#26 0x000000011012f6da in qemu_main (argc=<value temporarily
unavailable, due to optimizations>, argv=<value temporarily
unavailable, due to optimizations>, envp=0x4fc262c000000000) at
/Users/pm215/src/qemu-for-merges/vl.c:1880
#27 0x00007fff905f75c9 in start ()
thanks
-- PMM
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PULL 00/10] Fix device introspection regressions
2015-10-04 21:24 ` [Qemu-devel] [PULL 00/10] Fix device introspection regressions Peter Maydell
@ 2015-10-05 6:49 ` Markus Armbruster
2015-10-05 11:55 ` Peter Maydell
0 siblings, 1 reply; 21+ messages in thread
From: Markus Armbruster @ 2015-10-05 6:49 UTC (permalink / raw)
To: Peter Maydell; +Cc: QEMU Developers
Peter Maydell <peter.maydell@linaro.org> writes:
> On 2 October 2015 at 18:20, Markus Armbruster <armbru@redhat.com> wrote:
>> QMP command device-list-properties regressed in 2.1: it can crash or
>> leave dangling pointers behind.
>>
>> -device FOO,help regressed in 2.2: it no longer works for
>> non-pluggable devices. I tried to fix that some time ago[*], but my
>> fix failed review. This is my second, more comprehensive try.
>>
>> PATCH 1-3 fix one class of bugs involved in the regressions, PATCH 4-5
>> are libqtest preliminaries, PATCH 6 adds tests to demonstrate the
>> remaining bugs, PATCH 7-9 fix them to a degree (see PATCH 8 for
>> limitations), and PATCH 10 cleans up.
>
> This ordering breaks bisection of 'make check', as I found out when
> I tried to figure out which of the patches in this pull was causing
> an OSX test failure. Please can you reorder them so that 'make check'
> works at all points in the series?
My ordering may be bad (and I'll recheck it, of course), or it may
temporarily expose a hidden bug. I better figure out what's going on
here.
>> The following changes since commit ff770b07f34d28b79013a83989bd6c85f8f16b2f:
>>
>> Merge remote-tracking branch 'remotes/cody/tags/block-pull-request' into staging (2015-10-02 11:01:18 +0100)
>>
>> are available in the git repository at:
>>
>> git://repo.or.cz/qemu/armbru.git tags/pull-monitor-2015-10-02
>>
>> for you to fetch changes up to e927162a6fa2fa6144de9d1d11cc9448a2143671:
>>
>> Revert "qdev: Use qdev_get_device_class() for -device <type>,help" (2015-10-02 16:45:53 +0200)
>>
>> ----------------------------------------------------------------
>> Fix device introspection regressions
>>
>> ----------------------------------------------------------------
>
> 'make check' failure on OSX:
>
> /aarch64/device/introspect/list: OK
> /aarch64/device/introspect/none: OK
> /aarch64/device/introspect/abstract: OK
> /aarch64/device/introspect/concrete: **
> ERROR:/Users/pm215/src/qemu-for-merges/qom/object.c:333:void
> object_initialize_with_type(void *, size_t, TypeImpl *): assertion
> failed: (type != NULL)
> Broken pipe
> FAIL
>
> I have no idea why this only failed on OSX...
Can you re-run this with valgrind spliced in?
I use something like
$ QTEST_QEMU_BINARY="valgrind --vgdb-error=1 --log-file=vg.log qemu-system-aarch64" QTEST_QEMU_IMG=qemu-img MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM % 255 + 1))} gtester -k --verbose -m=quick tests/device-introspect-test
> Backtrace:
[Confusing due to inlining and other optimizations; snipped for now...]
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PULL 00/10] Fix device introspection regressions
2015-10-05 6:49 ` Markus Armbruster
@ 2015-10-05 11:55 ` Peter Maydell
2015-10-05 17:11 ` Markus Armbruster
0 siblings, 1 reply; 21+ messages in thread
From: Peter Maydell @ 2015-10-05 11:55 UTC (permalink / raw)
To: Markus Armbruster; +Cc: QEMU Developers
On 5 October 2015 at 07:49, Markus Armbruster <armbru@redhat.com> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>
>> On 2 October 2015 at 18:20, Markus Armbruster <armbru@redhat.com> wrote:
>>> QMP command device-list-properties regressed in 2.1: it can crash or
>>> leave dangling pointers behind.
>>>
>>> -device FOO,help regressed in 2.2: it no longer works for
>>> non-pluggable devices. I tried to fix that some time ago[*], but my
>>> fix failed review. This is my second, more comprehensive try.
>>>
>>> PATCH 1-3 fix one class of bugs involved in the regressions, PATCH 4-5
>>> are libqtest preliminaries, PATCH 6 adds tests to demonstrate the
>>> remaining bugs, PATCH 7-9 fix them to a degree (see PATCH 8 for
>>> limitations), and PATCH 10 cleans up.
>>
>> This ordering breaks bisection of 'make check', as I found out when
>> I tried to figure out which of the patches in this pull was causing
>> an OSX test failure. Please can you reorder them so that 'make check'
>> works at all points in the series?
>
> My ordering may be bad (and I'll recheck it, of course), or it may
> temporarily expose a hidden bug. I better figure out what's going on
> here.
>
>>> The following changes since commit ff770b07f34d28b79013a83989bd6c85f8f16b2f:
>>>
>>> Merge remote-tracking branch 'remotes/cody/tags/block-pull-request' into staging (2015-10-02 11:01:18 +0100)
>>>
>>> are available in the git repository at:
>>>
>>> git://repo.or.cz/qemu/armbru.git tags/pull-monitor-2015-10-02
>>>
>>> for you to fetch changes up to e927162a6fa2fa6144de9d1d11cc9448a2143671:
>>>
>>> Revert "qdev: Use qdev_get_device_class() for -device <type>,help" (2015-10-02 16:45:53 +0200)
>>>
>>> ----------------------------------------------------------------
>>> Fix device introspection regressions
>>>
>>> ----------------------------------------------------------------
>>
>> 'make check' failure on OSX:
>>
>> /aarch64/device/introspect/list: OK
>> /aarch64/device/introspect/none: OK
>> /aarch64/device/introspect/abstract: OK
>> /aarch64/device/introspect/concrete: **
>> ERROR:/Users/pm215/src/qemu-for-merges/qom/object.c:333:void
>> object_initialize_with_type(void *, size_t, TypeImpl *): assertion
>> failed: (type != NULL)
>> Broken pipe
>> FAIL
>>
>> I have no idea why this only failed on OSX...
>
> Can you re-run this with valgrind spliced in?
Valgrind is not particularly helpful: it reports a couple of
irrelevancies and an unimplemented syscall, then just
reports the backtrace for the abort:
==26853== Memcheck, a memory error detector
==26853== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==26853== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==26853== Command: ./aarch64-softmmu/qemu-system-aarch64 -qtest
unix:/tmp/qtest-26555.sock,nowait -qtest-log /dev/null -qmp
unix:/tmp/qtest-26555.qmp,nowait -machine accel=qtest -display none
-nodefaults -machine none
==26853== Parent PID: 26555
==26853==
==26853== Syscall param __pthread_sigmask(set) points to uninitialised byte(s)
==26853== at 0x10434E2B6: __pthread_sigmask (in
/usr/lib/system/libsystem_kernel.dylib)
==26853== by 0x10446406D: pthread_sigmask (in
/usr/lib/system/libsystem_pthread.dylib)
==26853== by 0x100537022: qemu_thread_create (qemu-thread-posix.c:488)
==26853== by 0x100550ACB: rcu_init_complete (rcu.c:320)
==26853== by 0x100550B18: rcu_init (rcu.c:351)
==26853== by 0x7FFF5FC12D0A:
ImageLoaderMachO::doModInitFunctions(ImageLoader::LinkContext const&)
(in /usr/lib/dyld)
==26853== by 0x7FFF5FC12E97:
ImageLoaderMachO::doInitialization(ImageLoader::LinkContext const&)
(in /usr/lib/dyld)
==26853== by 0x7FFF5FC0F890:
ImageLoader::recursiveInitialization(ImageLoader::LinkContext const&,
unsigned int, ImageLoader::InitializerTimingList&,
ImageLoader::UninitedUpwards&) (in /usr/lib/dyld)
==26853== by 0x7FFF5FC0F717:
ImageLoader::processInitializers(ImageLoader::LinkContext const&,
unsigned int, ImageLoader::InitializerTimingList&,
ImageLoader::UninitedUpwards&) (in /usr/lib/dyld)
==26853== by 0x7FFF5FC0F988:
ImageLoader::runInitializers(ImageLoader::LinkContext const&,
ImageLoader::InitializerTimingList&) (in /usr/lib/dyld)
==26853== by 0x7FFF5FC02244: dyld::initializeMainExecutable() (in
/usr/lib/dyld)
==26853== by 0x7FFF5FC05C18: dyld::_main(macho_header const*,
unsigned long, int, char const**, char const**, char const**, unsigned
long*) (in /usr/lib/dyld)
==26853== Address 0x1056e0c80 is on thread 1's stack
==26853== in frame #2, created by qemu_thread_create (qemu-thread-posix.c:461)
==26853==
==26853== Syscall param __pthread_sigmask(set) points to uninitialised byte(s)
==26853== at 0x10434E2B6: __pthread_sigmask (in
/usr/lib/system/libsystem_kernel.dylib)
==26853== by 0x10446406D: pthread_sigmask (in
/usr/lib/system/libsystem_pthread.dylib)
==26853== by 0x100537022: qemu_thread_create (qemu-thread-posix.c:488)
==26853== by 0x10053C6EC: qemu_signalfd_compat (compatfd.c:91)
==26853== by 0x10053C604: qemu_signalfd (in
./aarch64-softmmu/qemu-system-aarch64)
==26853== by 0x100473403: qemu_signal_init (main-loop.c:95)
==26853== by 0x10047319B: qemu_init_main_loop (main-loop.c:149)
==26853== by 0x1001FFAC4: qemu_main (vl.c:4008)
==26853== by 0x100435C72: main (cocoa.m:1164)
==26853== Address 0x1056e2c00 is on thread 1's stack
==26853== in frame #2, created by qemu_thread_create (qemu-thread-posix.c:461)
==26853==
--26853-- WARNING: unhandled amd64-darwin syscall: unix:330
--26853-- You may be able to write your own handler.
--26853-- Read the file README_MISSING_SYSCALL_OR_IOCTL.
--26853-- Nevertheless we consider this a bug. Please report
--26853-- it at http://valgrind.org/support/bug_reports.html.
==26853==
==26853== Process terminating with default action of signal 6 (SIGABRT)
==26853== at 0x10434E2B6: __pthread_sigmask (in
/usr/lib/system/libsystem_kernel.dylib)
==26853== by 0x104262A40: __abort (in /usr/lib/system/libsystem_c.dylib)
==26853== by 0x1042629C1: abort (in /usr/lib/system/libsystem_c.dylib)
==26853== by 0x101725C4F: g_assertion_message (in
/sw/lib/libglib-2.0.0.dylib)
==26853== by 0x101725C94: g_assertion_message_expr (in
/sw/lib/libglib-2.0.0.dylib)
==26853== by 0x10045BBB1: object_initialize_with_type (object.c:333)
==26853== by 0x10045C111: object_initialize (object.c:352)
==26853== by 0x1000E7D73: virtio_instance_init_common (virtio.c:1468)
==26853== by 0x1003EFE46: virtio_tablet_initfn (virtio-pci.c:2133)
==26853== by 0x10045C065: object_init_with_type (object.c:314)
==26853== by 0x10045BCF1: object_initialize_with_type (object.c:344)
==26853== by 0x10045C2A8: object_new_with_type (object.c:430)
==26853==
==26853== HEAP SUMMARY:
==26853== in use at exit: 2,242,505 bytes in 6,524 blocks
==26853== total heap usage: 84,155 allocs, 77,631 frees, 30,884,613
bytes allocated
==26853==
==26853== LEAK SUMMARY:
==26853== definitely lost: 91,693 bytes in 67 blocks
==26853== indirectly lost: 26,750 bytes in 719 blocks
==26853== possibly lost: 402,956 bytes in 2,553 blocks
==26853== still reachable: 396,629 bytes in 1,837 blocks
==26853== suppressed: 1,324,477 bytes in 1,348 blocks
==26853== Rerun with --leak-check=full to see details of leaked memory
==26853==
==26853== For counts of detected and suppressed errors, rerun with: -v
==26853== Use --track-origins=yes to see where uninitialised values come from
==26853== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)
-- PMM
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PULL 00/10] Fix device introspection regressions
2015-10-05 11:55 ` Peter Maydell
@ 2015-10-05 17:11 ` Markus Armbruster
2015-10-05 18:46 ` Peter Maydell
0 siblings, 1 reply; 21+ messages in thread
From: Markus Armbruster @ 2015-10-05 17:11 UTC (permalink / raw)
To: Peter Maydell; +Cc: QEMU Developers
Peter Maydell <peter.maydell@linaro.org> writes:
> On 5 October 2015 at 07:49, Markus Armbruster <armbru@redhat.com> wrote:
>> Peter Maydell <peter.maydell@linaro.org> writes:
>>
>>> On 2 October 2015 at 18:20, Markus Armbruster <armbru@redhat.com> wrote:
>>>> QMP command device-list-properties regressed in 2.1: it can crash or
>>>> leave dangling pointers behind.
>>>>
>>>> -device FOO,help regressed in 2.2: it no longer works for
>>>> non-pluggable devices. I tried to fix that some time ago[*], but my
>>>> fix failed review. This is my second, more comprehensive try.
>>>>
>>>> PATCH 1-3 fix one class of bugs involved in the regressions, PATCH 4-5
>>>> are libqtest preliminaries, PATCH 6 adds tests to demonstrate the
>>>> remaining bugs, PATCH 7-9 fix them to a degree (see PATCH 8 for
>>>> limitations), and PATCH 10 cleans up.
>>>
>>> This ordering breaks bisection of 'make check', as I found out when
>>> I tried to figure out which of the patches in this pull was causing
>>> an OSX test failure. Please can you reorder them so that 'make check'
>>> works at all points in the series?
>>
>> My ordering may be bad (and I'll recheck it, of course), or it may
>> temporarily expose a hidden bug. I better figure out what's going on
>> here.
All commits pass make check for me.
>>>> The following changes since commit ff770b07f34d28b79013a83989bd6c85f8f16b2f:
>>>>
>>>> Merge remote-tracking branch
>>>> 'remotes/cody/tags/block-pull-request' into staging (2015-10-02
>>>> 11:01:18 +0100)
>>>>
>>>> are available in the git repository at:
>>>>
>>>> git://repo.or.cz/qemu/armbru.git tags/pull-monitor-2015-10-02
>>>>
>>>> for you to fetch changes up to e927162a6fa2fa6144de9d1d11cc9448a2143671:
>>>>
>>>> Revert "qdev: Use qdev_get_device_class() for -device
>>>> <type>,help" (2015-10-02 16:45:53 +0200)
>>>>
>>>> ----------------------------------------------------------------
>>>> Fix device introspection regressions
>>>>
>>>> ----------------------------------------------------------------
>>>
>>> 'make check' failure on OSX:
>>>
>>> /aarch64/device/introspect/list: OK
>>> /aarch64/device/introspect/none: OK
>>> /aarch64/device/introspect/abstract: OK
>>> /aarch64/device/introspect/concrete: **
>>> ERROR:/Users/pm215/src/qemu-for-merges/qom/object.c:333:void
>>> object_initialize_with_type(void *, size_t, TypeImpl *): assertion
>>> failed: (type != NULL)
>>> Broken pipe
>>> FAIL
>>>
>>> I have no idea why this only failed on OSX...
>>
>> Can you re-run this with valgrind spliced in?
>
> Valgrind is not particularly helpful: it reports a couple of
> irrelevancies and an unimplemented syscall, then just
> reports the backtrace for the abort:
[...]
Sigh. I'll stare at the original backtrace some more.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PULL 00/10] Fix device introspection regressions
2015-10-05 17:11 ` Markus Armbruster
@ 2015-10-05 18:46 ` Peter Maydell
2015-10-05 19:27 ` Paolo Bonzini
0 siblings, 1 reply; 21+ messages in thread
From: Peter Maydell @ 2015-10-05 18:46 UTC (permalink / raw)
To: Markus Armbruster; +Cc: QEMU Developers
On 5 October 2015 at 18:11, Markus Armbruster <armbru@redhat.com> wrote:
> Sigh. I'll stare at the original backtrace some more.
Here's one from a debug build (with some reasoning about
the cause underneath it).
As an aside, thanks to your valgrind example I figured out
how to get the test makefile to run tests under gdb:
QTEST_QEMU_BINARY="xterm -e gdb --tty $(tty) --args
aarch64-softmmu/qemu-system-aarch64" QTEST_QEMU_IMG=qemu-img gtester
-k --verbose -m=quick tests/device-introspect-test
(which will start a new xterm with a gdb in it for
debugging each test).
#0 0x00007fff9145e286 in __pthread_kill ()
#1 0x00007fff912529f9 in pthread_kill ()
#2 0x00007fff956db9b3 in abort ()
#3 0x000000010103ec50 in g_assertion_message ()
#4 0x000000010103ec95 in g_assertion_message_expr ()
#5 0x000000010045bbb2 in object_initialize_with_type
(data=0x102056520, size=344, type=0x0) at
/Users/pm215/src/qemu-for-merges/qom/object.c:333
#6 0x000000010045c112 in object_initialize (data=0x102056520,
size=344, typename=0x1005a7a96 "virtio-tablet-device") at
/Users/pm215/src/qemu-for-merges/qom/object.c:352
#7 0x00000001000e7d74 in virtio_instance_init_common
(proxy_obj=0x10204e400, data=0x102056520, vdev_size=344,
vdev_name=0x1005a7a96 "virtio-tablet-device") at
/Users/pm215/src/qemu-for-merges/hw/virtio/virtio.c:1468
#8 0x00000001003efe47 in virtio_tablet_initfn (obj=0x10204e400) at
/Users/pm215/src/qemu-for-merges/hw/virtio/virtio-pci.c:2133
#9 0x000000010045c066 in object_init_with_type (obj=0x10204e400,
ti=0x10154f420) at /Users/pm215/src/qemu-for-merges/qom/object.c:314
#10 0x000000010045bcf2 in object_initialize_with_type
(data=0x10204e400, size=33400, type=0x10154f420) at
/Users/pm215/src/qemu-for-merges/qom/object.c:344
#11 0x000000010045c2a9 in object_new_with_type (type=0x10154f420) at
/Users/pm215/src/qemu-for-merges/qom/object.c:430
#12 0x000000010045c2f2 in object_new (typename=0x101585cc0
"virtio-tablet-pci") at
/Users/pm215/src/qemu-for-merges/qom/object.c:440
#13 0x000000010021923f in qmp_device_list_properties
(typename=0x101585cc0 "virtio-tablet-pci", errp=0x7fff5fbfd850) at
/Users/pm215/src/qemu-for-merges/qmp.c:529
#14 0x000000010020e054 in qmp_marshal_device_list_properties
(args=0x102023200, ret=0x7fff5fbfd8c8, errp=0x7fff5fbfd8d0) at
qmp-marshal.c:1693
#15 0x0000000100047fe1 in handle_qmp_command (parser=0x10157f388,
tokens=0x1013069a0) at /Users/pm215/src/qemu-for-merges/monitor.c:3860
#16 0x000000010052fb16 in json_message_process_token
(lexer=0x10157f390, token=0x101585b40, type=JSON_OPERATOR, x=15754,
y=0) at /Users/pm215/src/qemu-for-merges/qobject/json-streamer.c:87
#17 0x000000010052f45a in json_lexer_feed_char (lexer=0x10157f390,
ch=125 '}', flush=false) at
/Users/pm215/src/qemu-for-merges/qobject/json-lexer.c:303
#18 0x000000010052f2e1 in json_lexer_feed (lexer=0x10157f390,
buffer=0x7fff5fbfdb00 "}6\035�q&", size=1) at
/Users/pm215/src/qemu-for-merges/qobject/json-lexer.c:356
#19 0x000000010052fbc7 in json_message_parser_feed
(parser=0x10157f388, buffer=0x7fff5fbfdb00 "}6\035�q&", size=1) at
/Users/pm215/src/qemu-for-merges/qobject/json-streamer.c:110
#20 0x0000000100047ce6 in monitor_qmp_read (opaque=0x10157f310,
buf=0x7fff5fbfdb00 "}6\035�q&", size=1) at
/Users/pm215/src/qemu-for-merges/monitor.c:3875
#21 0x00000001001f06d7 in qemu_chr_be_write (s=0x10157e310,
buf=0x7fff5fbfdb00 "}6\035�q&", len=1) at
/Users/pm215/src/qemu-for-merges/qemu-char.c:305
#22 0x00000001001f661a in tcp_chr_read (chan=0x10157e5a0,
cond=G_IO_IN, opaque=0x10157e310) at
/Users/pm215/src/qemu-for-merges/qemu-char.c:2873
#23 0x000000010101f0bd in g_main_context_dispatch ()
#24 0x0000000100473aaa in glib_pollfds_poll () at
/Users/pm215/src/qemu-for-merges/main-loop.c:211
#25 0x000000010047371b in os_host_main_loop_wait (timeout=0) at
/Users/pm215/src/qemu-for-merges/main-loop.c:256
#26 0x000000010047358d in main_loop_wait (nonblocking=1) at
/Users/pm215/src/qemu-for-merges/main-loop.c:504
#27 0x000000010020715c in main_loop () at
/Users/pm215/src/qemu-for-merges/vl.c:1880
#28 0x00000001002015bc in qemu_main (argc=14, argv=0x7fff5fbff5c0,
envp=0x7fff5fbff638) at /Users/pm215/src/qemu-for-merges/vl.c:4634
#29 0x0000000100435c73 in main (argc=14, argv=0x7fff5fbff5c0) at
/Users/pm215/src/qemu-for-merges/ui/cocoa.m:1164
I think the problem here is that the "virtio-tablet-pci" device
is in hw/virtio/virtio-pci.c, which gets built for all hosts,
but it uses the device "virtio-tablet-device", which is defined
in hw/input/virtio-input-hid.c, which is only built if CONFIG_LINUX
is defined. So on non-Linux hosts we get a virtio-tablet-pci
device which can't be created.
The easy fix is to have some suitable ifdeffery in virtio-pci.c,
similar to how we only register the virtio_9p_pci and virtio_scsi_pci
types if they've been configured into this build.
thanks
-- PMM
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PULL 00/10] Fix device introspection regressions
2015-10-05 18:46 ` Peter Maydell
@ 2015-10-05 19:27 ` Paolo Bonzini
2015-10-05 19:37 ` Peter Maydell
0 siblings, 1 reply; 21+ messages in thread
From: Paolo Bonzini @ 2015-10-05 19:27 UTC (permalink / raw)
To: Peter Maydell, Markus Armbruster; +Cc: QEMU Developers
On 05/10/2015 20:46, Peter Maydell wrote:
> The easy fix is to have some suitable ifdeffery in virtio-pci.c,
> similar to how we only register the virtio_9p_pci and virtio_scsi_pci
(vhost_scsi_pci)
> types if they've been configured into this build.
Hmm, actually there's no reason to limit
common-obj-$(CONFIG_VIRTIO) += virtio-input.o
common-obj-$(CONFIG_VIRTIO) += virtio-input-hid.o
to CONFIG_LINUX. Does it work to extract them out of the if (which is
only correct for virtio-input-host.o)?
That said, the ifdeffery _is_ needed for TYPE_VIRTIO_INPUT_HOST_PCI.
Paolo
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PULL 00/10] Fix device introspection regressions
2015-10-05 19:27 ` Paolo Bonzini
@ 2015-10-05 19:37 ` Peter Maydell
2015-10-06 5:46 ` Markus Armbruster
0 siblings, 1 reply; 21+ messages in thread
From: Peter Maydell @ 2015-10-05 19:37 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Markus Armbruster, QEMU Developers
On 5 October 2015 at 20:27, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 05/10/2015 20:46, Peter Maydell wrote:
>> The easy fix is to have some suitable ifdeffery in virtio-pci.c,
>> similar to how we only register the virtio_9p_pci and virtio_scsi_pci
>
> (vhost_scsi_pci)
>
>> types if they've been configured into this build.
>
> Hmm, actually there's no reason to limit
>
> common-obj-$(CONFIG_VIRTIO) += virtio-input.o
> common-obj-$(CONFIG_VIRTIO) += virtio-input-hid.o
>
> to CONFIG_LINUX. Does it work to extract them out of the if (which is
> only correct for virtio-input-host.o)?
Yes, though as you predicted we then fall over on virtio-input-host-pci.
If I bodge that one out in virtio-pci.c then the device-introspect-test
passes.
thanks
-- PMM
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PULL 00/10] Fix device introspection regressions
2015-10-05 19:37 ` Peter Maydell
@ 2015-10-06 5:46 ` Markus Armbruster
2015-10-06 8:05 ` Peter Maydell
0 siblings, 1 reply; 21+ messages in thread
From: Markus Armbruster @ 2015-10-06 5:46 UTC (permalink / raw)
To: Peter Maydell; +Cc: Paolo Bonzini, QEMU Developers
Peter Maydell <peter.maydell@linaro.org> writes:
> On 5 October 2015 at 20:27, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>>
>> On 05/10/2015 20:46, Peter Maydell wrote:
>>> The easy fix is to have some suitable ifdeffery in virtio-pci.c,
>>> similar to how we only register the virtio_9p_pci and virtio_scsi_pci
>>
>> (vhost_scsi_pci)
>>
>>> types if they've been configured into this build.
>>
>> Hmm, actually there's no reason to limit
>>
>> common-obj-$(CONFIG_VIRTIO) += virtio-input.o
>> common-obj-$(CONFIG_VIRTIO) += virtio-input-hid.o
>>
>> to CONFIG_LINUX. Does it work to extract them out of the if (which is
>> only correct for virtio-input-host.o)?
>
> Yes, though as you predicted we then fall over on virtio-input-host-pci.
> If I bodge that one out in virtio-pci.c then the device-introspect-test
> passes.
Does this do the trick? If yes, I'll fill out the commit message and
post it properly.
>From e5ebf2f0680fcf145455db048b82cf8e5d9d6b9f Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Tue, 6 Oct 2015 07:43:18 +0200
Subject: [PATCH] virtio-input: Fix device introspection on non-Linux hosts WIP
---
hw/input/Makefile.objs | 2 +-
hw/virtio/virtio-pci.c | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/input/Makefile.objs b/hw/input/Makefile.objs
index 624ba7e..7715d72 100644
--- a/hw/input/Makefile.objs
+++ b/hw/input/Makefile.objs
@@ -8,9 +8,9 @@ common-obj-$(CONFIG_STELLARIS_INPUT) += stellaris_input.o
common-obj-$(CONFIG_TSC2005) += tsc2005.o
common-obj-$(CONFIG_VMMOUSE) += vmmouse.o
-ifeq ($(CONFIG_LINUX),y)
common-obj-$(CONFIG_VIRTIO) += virtio-input.o
common-obj-$(CONFIG_VIRTIO) += virtio-input-hid.o
+ifeq ($(CONFIG_LINUX),y)
common-obj-$(CONFIG_VIRTIO) += virtio-input-host.o
endif
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index eda8205..23f131b 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -2228,7 +2228,9 @@ static void virtio_pci_register_types(void)
type_register_static(&virtio_keyboard_pci_info);
type_register_static(&virtio_mouse_pci_info);
type_register_static(&virtio_tablet_pci_info);
+#ifdef CONFIG_LINUX
type_register_static(&virtio_host_pci_info);
+#endif
type_register_static(&virtio_pci_bus_info);
type_register_static(&virtio_pci_info);
#ifdef CONFIG_VIRTFS
--
2.4.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PULL 00/10] Fix device introspection regressions
2015-10-06 5:46 ` Markus Armbruster
@ 2015-10-06 8:05 ` Peter Maydell
2015-10-06 8:45 ` Markus Armbruster
0 siblings, 1 reply; 21+ messages in thread
From: Peter Maydell @ 2015-10-06 8:05 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Paolo Bonzini, QEMU Developers
On 6 October 2015 at 06:46, Markus Armbruster <armbru@redhat.com> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>
>> On 5 October 2015 at 20:27, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>
>>>
>>> On 05/10/2015 20:46, Peter Maydell wrote:
>>>> The easy fix is to have some suitable ifdeffery in virtio-pci.c,
>>>> similar to how we only register the virtio_9p_pci and virtio_scsi_pci
>>>
>>> (vhost_scsi_pci)
>>>
>>>> types if they've been configured into this build.
>>>
>>> Hmm, actually there's no reason to limit
>>>
>>> common-obj-$(CONFIG_VIRTIO) += virtio-input.o
>>> common-obj-$(CONFIG_VIRTIO) += virtio-input-hid.o
>>>
>>> to CONFIG_LINUX. Does it work to extract them out of the if (which is
>>> only correct for virtio-input-host.o)?
>>
>> Yes, though as you predicted we then fall over on virtio-input-host-pci.
>> If I bodge that one out in virtio-pci.c then the device-introspect-test
>> passes.
>
> Does this do the trick? If yes, I'll fill out the commit message and
> post it properly.
This is what I tested, yes, but...
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index eda8205..23f131b 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -2228,7 +2228,9 @@ static void virtio_pci_register_types(void)
> type_register_static(&virtio_keyboard_pci_info);
> type_register_static(&virtio_mouse_pci_info);
> type_register_static(&virtio_tablet_pci_info);
> +#ifdef CONFIG_LINUX
> type_register_static(&virtio_host_pci_info);
> +#endif
> type_register_static(&virtio_pci_bus_info);
> type_register_static(&virtio_pci_info);
> #ifdef CONFIG_VIRTFS
...this will get you warnings about unused variables/data -- you
need to #ifdef out the definition of virtio_host_pci_info and
all the static functions it references too.
thanks
-- PMM
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PULL 00/10] Fix device introspection regressions
2015-10-06 8:05 ` Peter Maydell
@ 2015-10-06 8:45 ` Markus Armbruster
0 siblings, 0 replies; 21+ messages in thread
From: Markus Armbruster @ 2015-10-06 8:45 UTC (permalink / raw)
To: Peter Maydell; +Cc: Paolo Bonzini, QEMU Developers
Peter Maydell <peter.maydell@linaro.org> writes:
> On 6 October 2015 at 06:46, Markus Armbruster <armbru@redhat.com> wrote:
>> Peter Maydell <peter.maydell@linaro.org> writes:
>>
>>> On 5 October 2015 at 20:27, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>>
>>>>
>>>> On 05/10/2015 20:46, Peter Maydell wrote:
>>>>> The easy fix is to have some suitable ifdeffery in virtio-pci.c,
>>>>> similar to how we only register the virtio_9p_pci and virtio_scsi_pci
>>>>
>>>> (vhost_scsi_pci)
>>>>
>>>>> types if they've been configured into this build.
>>>>
>>>> Hmm, actually there's no reason to limit
>>>>
>>>> common-obj-$(CONFIG_VIRTIO) += virtio-input.o
>>>> common-obj-$(CONFIG_VIRTIO) += virtio-input-hid.o
>>>>
>>>> to CONFIG_LINUX. Does it work to extract them out of the if (which is
>>>> only correct for virtio-input-host.o)?
>>>
>>> Yes, though as you predicted we then fall over on virtio-input-host-pci.
>>> If I bodge that one out in virtio-pci.c then the device-introspect-test
>>> passes.
>>
>> Does this do the trick? If yes, I'll fill out the commit message and
>> post it properly.
>
> This is what I tested, yes, but...
>
>> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
>> index eda8205..23f131b 100644
>> --- a/hw/virtio/virtio-pci.c
>> +++ b/hw/virtio/virtio-pci.c
>> @@ -2228,7 +2228,9 @@ static void virtio_pci_register_types(void)
>> type_register_static(&virtio_keyboard_pci_info);
>> type_register_static(&virtio_mouse_pci_info);
>> type_register_static(&virtio_tablet_pci_info);
>> +#ifdef CONFIG_LINUX
>> type_register_static(&virtio_host_pci_info);
>> +#endif
>> type_register_static(&virtio_pci_bus_info);
>> type_register_static(&virtio_pci_info);
>> #ifdef CONFIG_VIRTFS
>
> ...this will get you warnings about unused variables/data -- you
> need to #ifdef out the definition of virtio_host_pci_info and
> all the static functions it references too.
I'll finish the job and post for real. Thanks!
^ permalink raw reply [flat|nested] 21+ messages in thread