All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefan Weil <sw@weilnetz.de>
To: Kevin Wolf <kwolf@redhat.com>, Anthony Liguori <anthony@codemonkey.ws>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PULL 54/61] blockdev: Remove IF_* check for read-only blockdev_init
Date: Tue, 15 Oct 2013 17:53:58 +0200	[thread overview]
Message-ID: <525D6516.3010005@weilnetz.de> (raw)
In-Reply-To: <1381503951-27985-55-git-send-email-kwolf@redhat.com>

Am 11.10.2013 17:05, schrieb Kevin Wolf:
> IF_NONE allows read-only, which makes forbidding it in this place
> for other types pretty much pointless.
>
> Instead, make sure that all devices for which the check would have
> errored out check in their init function that they don't get a read-only
> BlockDriverState. This catches even cases where IF_NONE and -device is
> used.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
>

This patch breaks current QEMU (SIGSEGV with ARM in several test scenarios):

$ git bisect bad
4f8a066b5fc254eeaabbbde56ba4f5b29cc68fdf is the first bad commit
commit 4f8a066b5fc254eeaabbbde56ba4f5b29cc68fdf
Author: Kevin Wolf <kwolf@redhat.com>
Date:   Fri Sep 13 15:51:47 2013 +0200

    blockdev: Remove IF_* check for read-only blockdev_init
[...]

See the gdb protocol below for more details (Linux x86_64 host, default
configuration).

I got a bug report from a Windows user, but the crash is not OS specific.

Regards,
Stefan


(gdb) r
Starting program: bin/arm-softmmu/qemu-system-arm -M versatilepb -L
pc-bios -kernel vmlinuz-2.6.32-5-versatile -initrd
initrd.img-2.6.32-5-versatile -sd debian_squeeze_armel_standard.qcow2
-append root=/dev/sda1
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".

Breakpoint 5, pl181_init (sbd=0x5555565d3020) at hw/sd/pl181.c:482
482     {
(gdb) i s
#0  pl181_init (sbd=0x5555565d3020) at hw/sd/pl181.c:482
#1  0x00005555556d10e8 in sysbus_device_init (dev=0x5555565d3020) at
hw/core/sysbus.c:143
#2  0x00005555556ce6d3 in device_realize (dev=0x5555565d3020,
err=0x7fffffffdb08) at hw/core/qdev.c:178
#3  0x00005555556d002a in device_set_realized (obj=0x5555565d3020,
value=true, err=0x7fffffffdc80) at hw/core/qdev.c:699
#4  0x0000555555849520 in property_set_bool (obj=0x5555565d3020,
v=0x5555565d54d0, opaque=0x5555565ca870, name=0x555555a3c186 "realized",
errp=0x7fffffffdc80) at qom/object.c:1315
#5  0x0000555555848065 in object_property_set (obj=0x5555565d3020,
v=0x5555565d54d0, name=0x555555a3c186 "realized", errp=0x7fffffffdc80)
at qom/object.c:803
#6  0x00005555558497ca in object_property_set_qobject
(obj=0x5555565d3020, value=0x5555565adc20, name=0x555555a3c186
"realized", errp=0x7fffffffdc80) at qom/qom-qobject.c:24
#7  0x0000555555848351 in object_property_set_bool (obj=0x5555565d3020,
value=true, name=0x555555a3c186 "realized", errp=0x7fffffffdc80) at
qom/object.c:866
#8  0x00005555556ce60f in qdev_init (dev=0x5555565d3020) at
hw/core/qdev.c:163
#9  0x00005555556ceb8e in qdev_init_nofail (dev=0x5555565d3020) at
hw/core/qdev.c:277
#10 0x00005555556d11c3 in sysbus_create_varargs (name=0x555555a87f74
"pl181", addr=268455936) at hw/core/sysbus.c:157
#11 0x0000555555901572 in versatile_init (args=0x7fffffffe2d0,
board_id=387) at hw/arm/versatilepb.c:284
#12 0x0000555555901835 in vpb_init (args=0x7fffffffe2d0) at
hw/arm/versatilepb.c:357
#13 0x000055555589a38b in main (argc=13, argv=0x7fffffffe508,
envp=0x7fffffffe578) at vl.c:4245
(gdb) c
Continuing.

Breakpoint 5, pl181_init (sbd=0x5555565deda0) at hw/sd/pl181.c:482
482     {
(gdb)
Continuing.

Program received signal SIGSEGV, Segmentation fault.
0x000055555560b25f in bdrv_is_read_only (bs=0x0) at block.c:2933
2933        return bs->read_only;
(gdb) i s
#0  0x000055555560b25f in bdrv_is_read_only (bs=0x0) at block.c:2933
#1  0x0000555555794220 in sd_init (bs=0x0, is_spi=false) at hw/sd/sd.c:497
#2  0x000055555579316e in pl181_init (sbd=0x5555565deda0) at
hw/sd/pl181.c:493
#3  0x00005555556d10e8 in sysbus_device_init (dev=0x5555565deda0) at
hw/core/sysbus.c:143
#4  0x00005555556ce6d3 in device_realize (dev=0x5555565deda0,
err=0x7fffffffdb08) at hw/core/qdev.c:178
#5  0x00005555556d002a in device_set_realized (obj=0x5555565deda0,
value=true, err=0x7fffffffdc80) at hw/core/qdev.c:699
#6  0x0000555555849520 in property_set_bool (obj=0x5555565deda0,
v=0x5555565e1250, opaque=0x5555565ca500, name=0x555555a3c186 "realized",
errp=0x7fffffffdc80) at qom/object.c:1315
#7  0x0000555555848065 in object_property_set (obj=0x5555565deda0,
v=0x5555565e1250, name=0x555555a3c186 "realized", errp=0x7fffffffdc80)
at qom/object.c:803
#8  0x00005555558497ca in object_property_set_qobject
(obj=0x5555565deda0, value=0x5555565ca5f0, name=0x555555a3c186
"realized", errp=0x7fffffffdc80) at qom/qom-qobject.c:24
#9  0x0000555555848351 in object_property_set_bool (obj=0x5555565deda0,
value=true, name=0x555555a3c186 "realized", errp=0x7fffffffdc80) at
qom/object.c:866
#10 0x00005555556ce60f in qdev_init (dev=0x5555565deda0) at
hw/core/qdev.c:163
#11 0x00005555556ceb8e in qdev_init_nofail (dev=0x5555565deda0) at
hw/core/qdev.c:277
#12 0x00005555556d11c3 in sysbus_create_varargs (name=0x555555a87f74
"pl181", addr=268480512) at hw/core/sysbus.c:157
#13 0x000055555590159f in versatile_init (args=0x7fffffffe2d0,
board_id=387) at hw/arm/versatilepb.c:285
#14 0x0000555555901835 in vpb_init (args=0x7fffffffe2d0) at
hw/arm/versatilepb.c:357
#15 0x000055555589a38b in main (argc=13, argv=0x7fffffffe508,
envp=0x7fffffffe578) at vl.c:4245

  reply	other threads:[~2013-10-15 15:54 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-11 15:04 [Qemu-devel] [PULL 00/61] Block patches Kevin Wolf
2013-10-11 15:04 ` [Qemu-devel] [PULL 01/61] blockjob: rename BlockJobType to BlockJobDriver Kevin Wolf
2013-10-11 15:04 ` [Qemu-devel] [PULL 02/61] qapi: Introduce enum BlockJobType Kevin Wolf
2013-10-11 15:04 ` [Qemu-devel] [PULL 03/61] qapi: make use of new BlockJobType Kevin Wolf
2013-10-11 15:04 ` [Qemu-devel] [PULL 04/61] qapi: Add ImageInfoSpecific type Kevin Wolf
2013-10-11 15:04 ` [Qemu-devel] [PULL 05/61] block: Add bdrv_get_specific_info Kevin Wolf
2013-10-11 15:04 ` [Qemu-devel] [PULL 06/61] block/qapi: Human-readable ImageInfoSpecific dump Kevin Wolf
2013-10-11 15:04 ` [Qemu-devel] [PULL 07/61] qcow2: Add support for ImageInfoSpecific Kevin Wolf
2013-10-11 15:04 ` [Qemu-devel] [PULL 08/61] qemu-iotests: Discard specific info in _img_info Kevin Wolf
2013-10-11 15:04 ` [Qemu-devel] [PULL 09/61] qemu-iotests: Additional info from qemu-img info Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 10/61] qcow2: Alignment of snapshot table entries Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 11/61] qcow2: Use pread for inactive L1 in overlap check Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 12/61] qcow2: Free preallocated zero clusters Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 13/61] qcow2: Always use error path on writing snapshots Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 14/61] qcow2: Free allocated snapshot table on error Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 15/61] qcow2: Assert against snapshot name/ID overflow Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 16/61] block/get_block_status: avoid redundant callouts on raw devices Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 17/61] block: Add BlockDriver.bdrv_check_ext_snapshot Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 18/61] qemu-iotests: Discard preallocated zero clusters Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 19/61] ahci: set ahci mode on reset Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 20/61] block: qemu-iotests for vhdx, read sample dynamic image Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 21/61] qcow2: Add missing space in error message Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 22/61] qcow2: Remove wrong metadata overlap check Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 23/61] qcow2: Fix snapshot restoration in snapshot_create Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 24/61] qcow2: Use better type for numerical snapshot ID Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 25/61] block: Improve driver whitelist checks Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 26/61] qcow2: Use negated overflow check mask Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 27/61] qcow2: Make overlap check mask variable Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 28/61] qcow2: Add overlap-check options Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 29/61] qcow2: Array assigning options to OL check bits Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 30/61] qcow2: Add more overlap check bitmask macros Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 31/61] qcow2: Evaluate overlap check options Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 32/61] block/raw_bsd: Employ error parameter Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 33/61] block/raw-win32: " Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 34/61] blkdebug: " Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 35/61] blkverify: " Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 36/61] qemu-iotests: move blank lines of output in case 059 Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 37/61] block/raw-posix: Employ error parameter Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 38/61] tests: build the helper program by default Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 39/61] build: add command check-clean Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 40/61] vmdk: convert error code to use errp Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 41/61] vmdk: refuse enabling zeroed grain with flat images Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 42/61] qapi-types/visit.py: Pass whole expr dict for structs Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 43/61] qapi-types/visit.py: Inheritance " Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 44/61] blockdev: Introduce DriveInfo.enable_auto_del Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 45/61] blockdev: 'blockdev-add' QMP command Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 46/61] blockdev: Separate ID generation from DriveInfo creation Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 47/61] blockdev: Pass QDict to blockdev_init() Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 48/61] blockdev: Move parsing of 'media' option to drive_init Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 49/61] blockdev: Move parsing of 'if' " Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 50/61] blockdev: Moving parsing of geometry options " Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 51/61] blockdev: Move parsing of 'boot' option " Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 52/61] blockdev: Move bus/unit/index processing " Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 53/61] blockdev: Move virtio-blk device creation " Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 54/61] blockdev: Remove IF_* check for read-only blockdev_init Kevin Wolf
2013-10-15 15:53   ` Stefan Weil [this message]
2013-10-15 15:59     ` Kevin Wolf
2013-10-15 16:02       ` Stefan Weil
2013-10-11 15:05 ` [Qemu-devel] [PULL 55/61] qemu-iotests: Check autodel behaviour for device_del Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 56/61] blockdev: Remove 'media' parameter from blockdev_init() Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 57/61] blockdev: Don't disable COR automatically with blockdev-add Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 58/61] blockdev: blockdev_init() error conversion Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 59/61] vmdk: Fix vmdk_parse_extents Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 60/61] qemu-io: Let "open" pass options to block driver Kevin Wolf
2013-10-11 15:05 ` [Qemu-devel] [PULL 61/61] qemu-iotests: Add test for inactive L2 overlap Kevin Wolf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=525D6516.3010005@weilnetz.de \
    --to=sw@weilnetz.de \
    --cc=anthony@codemonkey.ws \
    --cc=kwolf@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.