* [PATCH v2 00/16] Crazy shit around -global (pardon my french)
@ 2020-06-22 9:42 Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 01/16] iotests/172: Include "info block" in test output Markus Armbruster
` (17 more replies)
0 siblings, 18 replies; 24+ messages in thread
From: Markus Armbruster @ 2020-06-22 9:42 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, berrange, ehabkost, qemu-block, mreitz, pbonzini, jsnow
There are three ways to configure backends:
* -nic, -serial, -drive, ... (onboard devices)
* Set the property with -device, or, if you feel masochistic, with
-set device (pluggable devices)
* Set the property with -global (both)
The trouble is -global is terrible.
It gets applied in object_new(), which can't fail. We treat failure
to apply -global as fatal error, except when hot-plugging, where we
treat it as warning *boggle*. I'm not addressing that today.
Some code falls apart when you use both -global and the other way.
To make life more interesting, we gave -drive two roles: with
interface type other than none, it's for configuring onboard devices,
and with interface type none, it's for defining backends for use with
-device and such. Since we neglect to require interface type none for
the latter, you can use one -drive in both roles. This confuses the
code about as much as you, dear reader, probably are by now.
Because this still isn't interesting enough, there's yet another way
to configure backends, just for floppies: set the floppy controller's
property. Goes back to the time when floppy wasn't a separate device,
and involves some Bad Magic. Now -global can interact with itself!
Digging through all this took me an embarrassing amount of time.
Hair, too.
My patches reject some the silliest uses outright, and deprecate some
not so silly ones that have replacements.
Apply on top of my "[PATCH v2 00/58] qdev: Rework how we plug into the
parent bus".
Enjoy!
v2:
* Rebased; tests/qemu-iotests/172.out regenerated to resolve conflicts
* PATCH 10-12: check_non_null() renamed to check_prop_still_unset()
[Philippe]
Markus Armbruster (16):
iotests/172: Include "info block" in test output
iotests/172: Cover empty filename and multiple use of drives
iotests/172: Cover -global floppy.drive=...
fdc: Reject clash between -drive if=floppy and -global isa-fdc
fdc: Open-code fdctrl_init_isa()
fdc: Deprecate configuring floppies with -global isa-fdc
docs/qdev-device-use.txt: Update section "Default Devices"
blockdev: Deprecate -drive with bogus interface type
qdev: Eliminate get_pointer(), set_pointer()
qdev: Improve netdev property override error a bit
qdev: Reject drive property override
qdev: Reject chardev property override
qdev: Make qdev_prop_set_drive() match the other helpers
arm/aspeed: Drop aspeed_board_init_flashes() parameter @errp
sd/pxa2xx_mmci: Don't crash on pxa2xx_mmci_init() error
sd/milkymist-memcard: Fix error API violation
docs/qdev-device-use.txt | 17 +-
docs/system/deprecated.rst | 34 ++
include/hw/block/fdc.h | 2 +-
include/hw/qdev-properties.h | 18 +-
include/sysemu/blockdev.h | 2 +
blockdev.c | 27 +-
hw/arm/aspeed.c | 16 +-
hw/arm/cubieboard.c | 2 +-
hw/arm/exynos4210.c | 2 +-
hw/arm/imx25_pdk.c | 2 +-
hw/arm/mcimx6ul-evk.c | 2 +-
hw/arm/mcimx7d-sabre.c | 2 +-
hw/arm/msf2-som.c | 4 +-
hw/arm/nseries.c | 4 +-
hw/arm/orangepi.c | 2 +-
hw/arm/raspi.c | 2 +-
hw/arm/sabrelite.c | 6 +-
hw/arm/vexpress.c | 3 +-
hw/arm/xilinx_zynq.c | 7 +-
hw/arm/xlnx-versal-virt.c | 2 +-
hw/arm/xlnx-zcu102.c | 10 +-
hw/block/fdc.c | 82 ++--
hw/block/nand.c | 2 +-
hw/block/pflash_cfi01.c | 6 +-
hw/block/pflash_cfi02.c | 2 +-
hw/core/qdev-properties-system.c | 151 ++++---
hw/core/qdev-properties.c | 17 +
hw/i386/pc.c | 8 +-
hw/ide/qdev.c | 4 +-
hw/isa/isa-superio.c | 18 +-
hw/m68k/q800.c | 3 +-
hw/microblaze/petalogix_ml605_mmu.c | 5 +-
hw/ppc/pnv.c | 3 +-
hw/ppc/spapr.c | 4 +-
hw/scsi/scsi-bus.c | 2 +-
hw/sd/milkymist-memcard.c | 2 +-
hw/sd/pxa2xx_mmci.c | 15 +-
hw/sd/sd.c | 2 +-
hw/sd/ssi-sd.c | 3 +-
hw/sparc64/sun4u.c | 9 +-
hw/xtensa/xtfpga.c | 3 +-
softmmu/vl.c | 8 +
tests/qemu-iotests/172 | 27 +-
tests/qemu-iotests/172.out | 656 +++++++++++++++++++++++++---
44 files changed, 928 insertions(+), 270 deletions(-)
--
2.26.2
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 01/16] iotests/172: Include "info block" in test output
2020-06-22 9:42 [PATCH v2 00/16] Crazy shit around -global (pardon my french) Markus Armbruster
@ 2020-06-22 9:42 ` Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 02/16] iotests/172: Cover empty filename and multiple use of drives Markus Armbruster
` (16 subsequent siblings)
17 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2020-06-22 9:42 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, berrange, ehabkost, qemu-block, mreitz, pbonzini, jsnow
The additional output demonstrates we screw up when -global isa-fdc
clashes with -drive if=floppy or its sugared forms: according to "info
qtree", only the latter backend is attached, but according to "info
block", both are. For instance:
Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveA=none0
dev: isa-fdc, id ""
[...]
driveA = ""
driveB = ""
[...]
bus: floppy-bus.0
type floppy-bus
dev: floppy, id ""
unit = 0 (0x0)
---> drive = "floppy0"
[...]
floppy0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
---> Attached to: /machine/unattached/device[15]
Removable device: not locked, tray closed
Cache mode: writeback
none0 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
---> Attached to: /machine/unattached/device[14]
Cache mode: writeback
/machine/unattached/device[15] is floppy, and
/machine/unattached/device[14] is isa-fdc.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/qemu-iotests/172 | 5 +-
tests/qemu-iotests/172.out | 486 +++++++++++++++++++++++++++++++++++++
2 files changed, 489 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/172 b/tests/qemu-iotests/172
index 7195fb895a..19c2516cf8 100755
--- a/tests/qemu-iotests/172
+++ b/tests/qemu-iotests/172
@@ -69,9 +69,10 @@ check_floppy_qtree()
#
# Apply the sed filter to stdout only, but keep the stderr output and
# filter the qemu program name in it.
- echo "info qtree" |
+ printf "info qtree\ninfo block\n" |
(QEMU_OPTIONS="" do_run_qemu "$@" |
- sed -ne '/^ dev: isa-fdc/,/^ dev:/{x;p}' ) 2>&1 |
+ _filter_testdir |_filter_generated_node_ids | _filter_hmp |
+ sed -ne '/^ dev: isa-fdc/,/^ dev:/{x;p};/^[a-z][^ ]* (NODE_NAME):* /,/^(qemu)$/{p}') 2>&1 |
_filter_win32 | _filter_qemu
}
diff --git a/tests/qemu-iotests/172.out b/tests/qemu-iotests/172.out
index e782c5957e..a315866e17 100644
--- a/tests/qemu-iotests/172.out
+++ b/tests/qemu-iotests/172.out
@@ -62,6 +62,19 @@ Testing: -fda TEST_DIR/t.qcow2
write-cache = "auto"
share-rw = false
drive-type = "144"
+floppy0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/unattached/device[15]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[22]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
Testing: -fdb TEST_DIR/t.qcow2
@@ -100,6 +113,23 @@ Testing: -fdb TEST_DIR/t.qcow2
write-cache = "auto"
share-rw = false
drive-type = "288"
+floppy1 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/unattached/device[16]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[23]
+ Removable device: not locked, tray closed
+
+floppy0: [not inserted]
+ Attached to: /machine/unattached/device[15]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
Testing: -fda TEST_DIR/t.qcow2 -fdb TEST_DIR/t.qcow2.2
@@ -138,6 +168,24 @@ Testing: -fda TEST_DIR/t.qcow2 -fdb TEST_DIR/t.qcow2.2
write-cache = "auto"
share-rw = false
drive-type = "144"
+floppy0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/unattached/device[15]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+floppy1 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
+ Attached to: /machine/unattached/device[16]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[23]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
=== Using -drive options ===
@@ -168,6 +216,19 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2
write-cache = "auto"
share-rw = false
drive-type = "144"
+floppy0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/unattached/device[15]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[22]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
Testing: -drive if=floppy,file=TEST_DIR/t.qcow2,index=1
@@ -206,6 +267,23 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2,index=1
write-cache = "auto"
share-rw = false
drive-type = "288"
+floppy1 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/unattached/device[16]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[23]
+ Removable device: not locked, tray closed
+
+floppy0: [not inserted]
+ Attached to: /machine/unattached/device[15]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=floppy,file=TEST_DIR/t.qcow2.2,index=1
@@ -244,6 +322,24 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=floppy,file=TEST_DIR/t
write-cache = "auto"
share-rw = false
drive-type = "144"
+floppy0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/unattached/device[15]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+floppy1 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
+ Attached to: /machine/unattached/device[16]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[23]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
=== Using -drive if=none and -global ===
@@ -274,6 +370,19 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveA=none0
write-cache = "auto"
share-rw = false
drive-type = "144"
+none0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/unattached/device[15]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[22]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
Testing: -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveB=none0
@@ -301,6 +410,19 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveB=none0
write-cache = "auto"
share-rw = false
drive-type = "144"
+none0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/unattached/device[15]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[22]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveA=none0 -global isa-fdc.driveB=none1
@@ -339,6 +461,24 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco
write-cache = "auto"
share-rw = false
drive-type = "144"
+none0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/unattached/device[15]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+none1 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
+ Attached to: /machine/unattached/device[16]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[23]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
=== Using -drive if=none and -device ===
@@ -369,6 +509,19 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0
write-cache = "auto"
share-rw = false
drive-type = "144"
+none0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/peripheral-anon/device[0]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[21]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,unit=1
@@ -396,6 +549,19 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,unit=1
write-cache = "auto"
share-rw = false
drive-type = "144"
+none0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/peripheral-anon/device[0]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[21]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device floppy,drive=none0 -device floppy,drive=none1,unit=1
@@ -434,6 +600,24 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco
write-cache = "auto"
share-rw = false
drive-type = "144"
+none0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/peripheral-anon/device[0]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+none1 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
+ Attached to: /machine/peripheral-anon/device[1]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[21]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
=== Mixing -fdX and -global ===
@@ -475,6 +659,24 @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global is
write-cache = "auto"
share-rw = false
drive-type = "144"
+floppy0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/unattached/device[15]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+none0 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
+ Attached to: /machine/unattached/device[16]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[23]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveA=none0
@@ -513,6 +715,24 @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global is
write-cache = "auto"
share-rw = false
drive-type = "144"
+floppy1 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/unattached/device[16]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+none0 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
+ Attached to: /machine/unattached/device[15]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[23]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveA=none0
@@ -540,6 +760,23 @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global is
write-cache = "auto"
share-rw = false
drive-type = "144"
+floppy0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/unattached/device[15]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+none0 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
+ Attached to: /machine/unattached/device[14]
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[22]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveB=none0
@@ -567,6 +804,23 @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global is
write-cache = "auto"
share-rw = false
drive-type = "144"
+floppy1 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/unattached/device[15]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+none0 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
+ Attached to: /machine/unattached/device[14]
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[22]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
=== Mixing -fdX and -device ===
@@ -608,6 +862,24 @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
write-cache = "auto"
share-rw = false
drive-type = "144"
+floppy0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/unattached/device[15]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+none0 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
+ Attached to: /machine/peripheral-anon/device[0]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[22]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device floppy,drive=none0,unit=1
@@ -646,6 +918,24 @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
write-cache = "auto"
share-rw = false
drive-type = "144"
+floppy0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/unattached/device[15]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+none0 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
+ Attached to: /machine/peripheral-anon/device[0]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[22]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device floppy,drive=none0
@@ -684,6 +974,24 @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
write-cache = "auto"
share-rw = false
drive-type = "144"
+floppy1 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/unattached/device[15]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+none0 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
+ Attached to: /machine/peripheral-anon/device[0]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[22]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device floppy,drive=none0,unit=0
@@ -722,6 +1030,24 @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
write-cache = "auto"
share-rw = false
drive-type = "144"
+floppy1 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/unattached/device[15]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+none0 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
+ Attached to: /machine/peripheral-anon/device[0]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[22]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device floppy,drive=none0,unit=0
QEMU_PROG: -device floppy,drive=none0,unit=0: Floppy unit 0 is in use
@@ -769,6 +1095,24 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.q
write-cache = "auto"
share-rw = false
drive-type = "144"
+floppy0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/unattached/device[15]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+none0 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
+ Attached to: /machine/peripheral-anon/device[0]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[22]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device floppy,drive=none0,unit=1
@@ -807,6 +1151,24 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.q
write-cache = "auto"
share-rw = false
drive-type = "144"
+floppy0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/unattached/device[15]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+none0 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
+ Attached to: /machine/peripheral-anon/device[0]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[22]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device floppy,drive=none0,unit=0
QEMU_PROG: -device floppy,drive=none0,unit=0: Floppy unit 0 is in use
@@ -851,6 +1213,24 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco
write-cache = "auto"
share-rw = false
drive-type = "144"
+none0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/unattached/device[15]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+none1 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
+ Attached to: /machine/peripheral-anon/device[0]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[22]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveA=none0 -device floppy,drive=none1,unit=1
@@ -889,6 +1269,24 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco
write-cache = "auto"
share-rw = false
drive-type = "144"
+none0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/unattached/device[15]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+none1 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
+ Attached to: /machine/peripheral-anon/device[0]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[22]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveB=none0 -device floppy,drive=none1
@@ -927,6 +1325,24 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco
write-cache = "auto"
share-rw = false
drive-type = "144"
+none0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/unattached/device[15]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+none1 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
+ Attached to: /machine/peripheral-anon/device[0]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[22]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveB=none0 -device floppy,drive=none1,unit=0
@@ -965,6 +1381,24 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco
write-cache = "auto"
share-rw = false
drive-type = "144"
+none0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/unattached/device[15]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+none1 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
+ Attached to: /machine/peripheral-anon/device[0]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[22]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveA=none0 -device floppy,drive=none1,unit=0
QEMU_PROG: -device floppy,drive=none1,unit=0: Floppy unit 0 is in use
@@ -1118,6 +1552,19 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,drive-t
write-cache = "auto"
share-rw = false
drive-type = "120"
+none0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/peripheral-anon/device[0]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[21]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,drive-type=288
@@ -1145,6 +1592,19 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,drive-t
write-cache = "auto"
share-rw = false
drive-type = "288"
+none0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/peripheral-anon/device[0]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[21]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
=== Try passing different block sizes ===
@@ -1175,6 +1635,19 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,logical
write-cache = "auto"
share-rw = false
drive-type = "144"
+none0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/peripheral-anon/device[0]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[21]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,physical_block_size=512
@@ -1202,6 +1675,19 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,physica
write-cache = "auto"
share-rw = false
drive-type = "144"
+none0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/peripheral-anon/device[0]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[21]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,logical_block_size=4096
QEMU_PROG: -device floppy,drive=none0,logical_block_size=4096: logical_block_size > physical_block_size not supported
--
2.26.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 02/16] iotests/172: Cover empty filename and multiple use of drives
2020-06-22 9:42 [PATCH v2 00/16] Crazy shit around -global (pardon my french) Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 01/16] iotests/172: Include "info block" in test output Markus Armbruster
@ 2020-06-22 9:42 ` Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 03/16] iotests/172: Cover -global floppy.drive= Markus Armbruster
` (15 subsequent siblings)
17 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2020-06-22 9:42 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, berrange, ehabkost, qemu-block, mreitz, pbonzini, jsnow
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/qemu-iotests/172 | 12 +++++++++
tests/qemu-iotests/172.out | 50 ++++++++++++++++++++++++++++++++++++++
2 files changed, 62 insertions(+)
diff --git a/tests/qemu-iotests/172 b/tests/qemu-iotests/172
index 19c2516cf8..714c7527b4 100755
--- a/tests/qemu-iotests/172
+++ b/tests/qemu-iotests/172
@@ -111,6 +111,7 @@ echo === Using -fda/-fdb options ===
check_floppy_qtree -fda "$TEST_IMG"
check_floppy_qtree -fdb "$TEST_IMG"
check_floppy_qtree -fda "$TEST_IMG" -fdb "$TEST_IMG.2"
+check_floppy_qtree -fdb ""
echo
@@ -198,6 +199,17 @@ check_floppy_qtree -drive if=none,file="$TEST_IMG" -drive if=none,file="$TEST_IM
check_floppy_qtree -drive if=none,file="$TEST_IMG" -drive if=none,file="$TEST_IMG.2" \
-global isa-fdc.driveB=none0 -device floppy,drive=none1,unit=1
+echo
+echo
+echo === Attempt to use drive twice ===
+
+# if=none
+check_floppy_qtree -drive if=none -device floppy,drive=none0 -device floppy -device floppy,drive=none0
+# if=floppy
+check_floppy_qtree -fda "" -device floppy,drive=floppy0
+# default if=floppy (not found, because it's created later)
+check_floppy_qtree -device floppy,drive=floppy0
+
echo
echo
echo === Too many floppy drives ===
diff --git a/tests/qemu-iotests/172.out b/tests/qemu-iotests/172.out
index a315866e17..0665cdcb51 100644
--- a/tests/qemu-iotests/172.out
+++ b/tests/qemu-iotests/172.out
@@ -187,6 +187,44 @@ sd0: [not inserted]
(qemu) quit
+Testing: -fdb
+
+ dev: isa-fdc, id ""
+ iobase = 1008 (0x3f0)
+ irq = 6 (0x6)
+ dma = 2 (0x2)
+ driveA = ""
+ driveB = ""
+ check_media_rate = true
+ fdtypeA = "auto"
+ fdtypeB = "auto"
+ fallback = "288"
+ isa irq 6
+ bus: floppy-bus.0
+ type floppy-bus
+ dev: floppy, id ""
+ unit = 1 (0x1)
+ drive = "floppy1"
+ logical_block_size = 512 (0x200)
+ physical_block_size = 512 (0x200)
+ min_io_size = 0 (0x0)
+ opt_io_size = 0 (0x0)
+ discard_granularity = 4294967295 (0xffffffff)
+ write-cache = "auto"
+ share-rw = false
+ drive-type = "288"
+ dev: floppy, id ""
+ unit = 0 (0x0)
+ drive = "floppy0"
+ logical_block_size = 512 (0x200)
+ physical_block_size = 512 (0x200)
+ min_io_size = 0 (0x0)
+ opt_io_size = 0 (0x0)
+ discard_granularity = 4294967295 (0xffffffff)
+ write-cache = "auto"
+ share-rw = false
+ drive-type = "288"
+
=== Using -drive options ===
@@ -1407,6 +1445,18 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco
QEMU_PROG: -device floppy,drive=none1,unit=1: Floppy unit 1 is in use
+=== Attempt to use drive twice ===
+
+Testing: -drive if=none -device floppy,drive=none0 -device floppy -device floppy,drive=none0
+QEMU_PROG: -device floppy,drive=none0: Drive 'none0' is already in use by another device
+
+Testing: -fda -device floppy,drive=floppy0
+QEMU_PROG: -device floppy,drive=floppy0: Drive 'floppy0' is already in use because it has been automatically connected to another device (did you need 'if=none' in the drive options?)
+
+Testing: -device floppy,drive=floppy0
+QEMU_PROG: -device floppy,drive=floppy0: Property 'floppy.drive' can't find value 'floppy0'
+
+
=== Too many floppy drives ===
Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -drive if=none,file=TEST_DIR/t.qcow2.3 -global isa-fdc.driveB=none0 -device floppy,drive=none1
--
2.26.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 03/16] iotests/172: Cover -global floppy.drive=...
2020-06-22 9:42 [PATCH v2 00/16] Crazy shit around -global (pardon my french) Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 01/16] iotests/172: Include "info block" in test output Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 02/16] iotests/172: Cover empty filename and multiple use of drives Markus Armbruster
@ 2020-06-22 9:42 ` Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 04/16] fdc: Reject clash between -drive if=floppy and -global isa-fdc Markus Armbruster
` (14 subsequent siblings)
17 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2020-06-22 9:42 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, berrange, ehabkost, qemu-block, mreitz, pbonzini, jsnow
Use of -global to set a default backend for non-singleton devices is a
bad idea. But as long as we permit it, we better test it.
Test output demonstrates we screw up when -global floppy clashes with
-fda or with -device floppy: according to "info qtree", only the
latter backend is attached, but according to "info block", both are.
Here's the clash with -device:
Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global floppy.drive=none0 -device floppy,drive=none1,unit=0
dev: isa-fdc, id ""
[...]
driveA = ""
driveB = ""
[...]
bus: floppy-bus.0
type floppy-bus
dev: floppy, id ""
unit = 0 (0x0)
---> drive = "none1"
[...]
none0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
---> Attached to: /machine/peripheral-anon/device[0]
Cache mode: writeback
none1 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
---> Attached to: /machine/peripheral-anon/device[0]
Removable device: not locked, tray closed
Cache mode: writeback
/machine/peripheral-anon/device[0] is the floppy created with -device.
Test output further demonstrates the "Drive 'FOO' is already in use
because it has been automatically connected to another device" error
message can be misleading. With '-fda "" -global
floppy.drive=floppy0', it's in use because -global reuses -fda's
backend. There is no other device involved.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/qemu-iotests/172 | 7 ++
tests/qemu-iotests/172.out | 134 +++++++++++++++++++++++++++++++++++++
2 files changed, 141 insertions(+)
diff --git a/tests/qemu-iotests/172 b/tests/qemu-iotests/172
index 714c7527b4..18056bcef7 100755
--- a/tests/qemu-iotests/172
+++ b/tests/qemu-iotests/172
@@ -151,6 +151,7 @@ check_floppy_qtree -fdb "$TEST_IMG" -drive if=none,file="$TEST_IMG.2" -global is
# Conflicting (-fdX wins)
check_floppy_qtree -fda "$TEST_IMG" -drive if=none,file="$TEST_IMG.2" -global isa-fdc.driveA=none0
check_floppy_qtree -fdb "$TEST_IMG" -drive if=none,file="$TEST_IMG.2" -global isa-fdc.driveB=none0
+check_floppy_qtree -fda "$TEST_IMG" -drive if=none,file="$TEST_IMG.2" -global floppy.drive=none0
echo
echo
@@ -192,12 +193,16 @@ check_floppy_qtree -drive if=none,file="$TEST_IMG" -drive if=none,file="$TEST_IM
-global isa-fdc.driveB=none0 -device floppy,drive=none1
check_floppy_qtree -drive if=none,file="$TEST_IMG" -drive if=none,file="$TEST_IMG.2" \
-global isa-fdc.driveB=none0 -device floppy,drive=none1,unit=0
+check_floppy_qtree -drive if=none,file="$TEST_IMG" \
+ -global floppy.drive=none0 -device floppy,unit=0
# Conflicting
check_floppy_qtree -drive if=none,file="$TEST_IMG" -drive if=none,file="$TEST_IMG.2" \
-global isa-fdc.driveA=none0 -device floppy,drive=none1,unit=0
check_floppy_qtree -drive if=none,file="$TEST_IMG" -drive if=none,file="$TEST_IMG.2" \
-global isa-fdc.driveB=none0 -device floppy,drive=none1,unit=1
+check_floppy_qtree -drive if=none,file="$TEST_IMG" -drive if=none,file="$TEST_IMG.2" \
+ -global floppy.drive=none0 -device floppy,drive=none1,unit=0
echo
echo
@@ -205,8 +210,10 @@ echo === Attempt to use drive twice ===
# if=none
check_floppy_qtree -drive if=none -device floppy,drive=none0 -device floppy -device floppy,drive=none0
+check_floppy_qtree -drive if=none -global floppy.drive=none0 -device floppy -device floppy
# if=floppy
check_floppy_qtree -fda "" -device floppy,drive=floppy0
+check_floppy_qtree -fda "" -global floppy.drive=floppy0
# default if=floppy (not found, because it's created later)
check_floppy_qtree -device floppy,drive=floppy0
diff --git a/tests/qemu-iotests/172.out b/tests/qemu-iotests/172.out
index 0665cdcb51..68e7a5ea5f 100644
--- a/tests/qemu-iotests/172.out
+++ b/tests/qemu-iotests/172.out
@@ -860,6 +860,50 @@ sd0: [not inserted]
(qemu) quit
+Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global floppy.drive=none0
+
+ dev: isa-fdc, id ""
+ iobase = 1008 (0x3f0)
+ irq = 6 (0x6)
+ dma = 2 (0x2)
+ driveA = ""
+ driveB = ""
+ check_media_rate = true
+ fdtypeA = "auto"
+ fdtypeB = "auto"
+ fallback = "288"
+ isa irq 6
+ bus: floppy-bus.0
+ type floppy-bus
+ dev: floppy, id ""
+ unit = 0 (0x0)
+ drive = "floppy0"
+ logical_block_size = 512 (0x200)
+ physical_block_size = 512 (0x200)
+ min_io_size = 0 (0x0)
+ opt_io_size = 0 (0x0)
+ discard_granularity = 4294967295 (0xffffffff)
+ write-cache = "auto"
+ share-rw = false
+ drive-type = "144"
+floppy0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/unattached/device[15]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+none0 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
+ Attached to: /machine/unattached/device[15]
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[22]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
+
=== Mixing -fdX and -device ===
@@ -1438,21 +1482,111 @@ sd0: [not inserted]
(qemu) quit
+Testing: -drive if=none,file=TEST_DIR/t.qcow2 -global floppy.drive=none0 -device floppy,unit=0
+
+ dev: isa-fdc, id ""
+ iobase = 1008 (0x3f0)
+ irq = 6 (0x6)
+ dma = 2 (0x2)
+ driveA = ""
+ driveB = ""
+ check_media_rate = true
+ fdtypeA = "auto"
+ fdtypeB = "auto"
+ fallback = "288"
+ isa irq 6
+ bus: floppy-bus.0
+ type floppy-bus
+ dev: floppy, id ""
+ unit = 0 (0x0)
+ drive = "none0"
+ logical_block_size = 512 (0x200)
+ physical_block_size = 512 (0x200)
+ min_io_size = 0 (0x0)
+ opt_io_size = 0 (0x0)
+ discard_granularity = 4294967295 (0xffffffff)
+ write-cache = "auto"
+ share-rw = false
+ drive-type = "144"
+none0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/peripheral-anon/device[0]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[21]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
+
Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveA=none0 -device floppy,drive=none1,unit=0
QEMU_PROG: -device floppy,drive=none1,unit=0: Floppy unit 0 is in use
Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveB=none0 -device floppy,drive=none1,unit=1
QEMU_PROG: -device floppy,drive=none1,unit=1: Floppy unit 1 is in use
+Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global floppy.drive=none0 -device floppy,drive=none1,unit=0
+
+ dev: isa-fdc, id ""
+ iobase = 1008 (0x3f0)
+ irq = 6 (0x6)
+ dma = 2 (0x2)
+ driveA = ""
+ driveB = ""
+ check_media_rate = true
+ fdtypeA = "auto"
+ fdtypeB = "auto"
+ fallback = "288"
+ isa irq 6
+ bus: floppy-bus.0
+ type floppy-bus
+ dev: floppy, id ""
+ unit = 0 (0x0)
+ drive = "none1"
+ logical_block_size = 512 (0x200)
+ physical_block_size = 512 (0x200)
+ min_io_size = 0 (0x0)
+ opt_io_size = 0 (0x0)
+ discard_granularity = 4294967295 (0xffffffff)
+ write-cache = "auto"
+ share-rw = false
+ drive-type = "144"
+none0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+ Attached to: /machine/peripheral-anon/device[0]
+ Cache mode: writeback
+
+none1 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
+ Attached to: /machine/peripheral-anon/device[0]
+ Removable device: not locked, tray closed
+ Cache mode: writeback
+
+ide1-cd0: [not inserted]
+ Attached to: /machine/unattached/device[21]
+ Removable device: not locked, tray closed
+
+sd0: [not inserted]
+ Removable device: not locked, tray closed
+(qemu) quit
+
+
=== Attempt to use drive twice ===
Testing: -drive if=none -device floppy,drive=none0 -device floppy -device floppy,drive=none0
QEMU_PROG: -device floppy,drive=none0: Drive 'none0' is already in use by another device
+Testing: -drive if=none -global floppy.drive=none0 -device floppy -device floppy
+QEMU_PROG: -device floppy: can't apply global floppy.drive=none0: Drive 'none0' is already in use by another device
+
Testing: -fda -device floppy,drive=floppy0
QEMU_PROG: -device floppy,drive=floppy0: Drive 'floppy0' is already in use because it has been automatically connected to another device (did you need 'if=none' in the drive options?)
+Testing: -fda -global floppy.drive=floppy0
+QEMU_PROG: can't apply global floppy.drive=floppy0: Drive 'floppy0' is already in use because it has been automatically connected to another device (did you need 'if=none' in the drive options?)
+
Testing: -device floppy,drive=floppy0
QEMU_PROG: -device floppy,drive=floppy0: Property 'floppy.drive' can't find value 'floppy0'
--
2.26.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 04/16] fdc: Reject clash between -drive if=floppy and -global isa-fdc
2020-06-22 9:42 [PATCH v2 00/16] Crazy shit around -global (pardon my french) Markus Armbruster
` (2 preceding siblings ...)
2020-06-22 9:42 ` [PATCH v2 03/16] iotests/172: Cover -global floppy.drive= Markus Armbruster
@ 2020-06-22 9:42 ` Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 05/16] fdc: Open-code fdctrl_init_isa() Markus Armbruster
` (13 subsequent siblings)
17 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2020-06-22 9:42 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, berrange, ehabkost, qemu-block, mreitz, pbonzini, jsnow
The floppy controller devices desugar their drive properties into
floppy devices (since commit a92bd191a4 "fdc: Move qdev properties to
FloppyDrive", v2.8.0). This involves some bad magic in
fdctrl_connect_drives(), and exists for backward compatibility.
The functions for boards to create floppy controller devices
fdctrl_init_isa(), fdctrl_init_sysbus(), and sun4m_fdctrl_init()
desugar -drive if=floppy to these floppy controller drive properties.
If you use both -drive if=floppy (or its -fda / -fdb sugar) and
-global isa-fdc for the same floppy device, -global silently loses the
conflict, and both backends involved end up with the floppy device
frontend attached, as demonstrated by iotest 172 (see commit before
previous). This is wrong.
Desugar -drive if=floppy straight to floppy devices instead, with
helper fdctrl_init_drives(). The conflict now gets rejected cleanly:
first, fdctrl_connect_drives() creates the floppy for the controller's
property, then fdctrl_init_drives() attempts to create the floppy for
-drive if=floppy, but fails because the unit is already in use.
Output of iotest 172 changes in three ways:
1. The clash gets rejected.
2. In one test case, "info qtree" has the floppy devices swapped, and
"info block" has their QOM paths swapped. This is because the
floppy device for -fda now gets created after the one for -global
isa-fdc.driveB.
3. The error message for -global floppy.drive=floppy0 changes. Before
the patch, we set isa-fdc.driveA to -fda's block backend, then
create the floppy device for it, then move the backend from
isa-fdc.driveA to floppy.drive. Floppy creation fails when
applying -global floppy.drive=floppy0, because floppy0 is still
attached to isa-fdc. After the patch, we create the floppy for
-fda, then set its drive property to floppy0. Now floppy creation
succeeds, but setting the drive property fails, because -global
already set it. Yes, this is exasperatingly complicated.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
include/hw/block/fdc.h | 1 +
hw/block/fdc.c | 51 ++++++------
hw/isa/isa-superio.c | 18 ++--
hw/sparc64/sun4u.c | 9 +-
tests/qemu-iotests/172 | 3 +-
tests/qemu-iotests/172.out | 164 ++++++++++---------------------------
6 files changed, 79 insertions(+), 167 deletions(-)
diff --git a/include/hw/block/fdc.h b/include/hw/block/fdc.h
index c15ff4c623..8855d3476c 100644
--- a/include/hw/block/fdc.h
+++ b/include/hw/block/fdc.h
@@ -9,6 +9,7 @@
#define TYPE_ISA_FDC "isa-fdc"
+void isa_fdc_init_drives(ISADevice *fdc, DriveInfo **fds);
ISADevice *fdctrl_init_isa(ISABus *bus, DriveInfo **fds);
void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
hwaddr mmio_base, DriveInfo **fds);
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index be0674e4aa..2650dcb0df 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -2497,6 +2497,29 @@ static void fdctrl_result_timer(void *opaque)
}
/* Init functions */
+
+static void fdctrl_init_drives(FloppyBus *bus, DriveInfo **fds)
+{
+ DeviceState *dev;
+ int i;
+
+ for (i = 0; i < MAX_FD; i++) {
+ if (fds[i]) {
+ dev = qdev_new("floppy");
+ qdev_prop_set_uint32(dev, "unit", i);
+ qdev_prop_set_enum(dev, "drive-type", FLOPPY_DRIVE_TYPE_AUTO);
+ qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(fds[i]),
+ &error_fatal);
+ qdev_realize_and_unref(dev, &bus->bus, &error_fatal);
+ }
+ }
+}
+
+void isa_fdc_init_drives(ISADevice *fdc, DriveInfo **fds)
+{
+ fdctrl_init_drives(&ISA_FDC(fdc)->state.bus, fds);
+}
+
static void fdctrl_connect_drives(FDCtrl *fdctrl, DeviceState *fdc_dev,
Error **errp)
{
@@ -2544,25 +2567,15 @@ static void fdctrl_connect_drives(FDCtrl *fdctrl, DeviceState *fdc_dev,
ISADevice *fdctrl_init_isa(ISABus *bus, DriveInfo **fds)
{
- DeviceState *dev;
ISADevice *isadev;
isadev = isa_try_new(TYPE_ISA_FDC);
if (!isadev) {
return NULL;
}
- dev = DEVICE(isadev);
-
- if (fds[0]) {
- qdev_prop_set_drive(dev, "driveA", blk_by_legacy_dinfo(fds[0]),
- &error_fatal);
- }
- if (fds[1]) {
- qdev_prop_set_drive(dev, "driveB", blk_by_legacy_dinfo(fds[1]),
- &error_fatal);
- }
isa_realize_and_unref(isadev, bus, &error_fatal);
+ isa_fdc_init_drives(isadev, fds);
return isadev;
}
@@ -2578,18 +2591,12 @@ void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
sys = SYSBUS_FDC(dev);
fdctrl = &sys->state;
fdctrl->dma_chann = dma_chann; /* FIXME */
- if (fds[0]) {
- qdev_prop_set_drive(dev, "driveA", blk_by_legacy_dinfo(fds[0]),
- &error_fatal);
- }
- if (fds[1]) {
- qdev_prop_set_drive(dev, "driveB", blk_by_legacy_dinfo(fds[1]),
- &error_fatal);
- }
sbd = SYS_BUS_DEVICE(dev);
sysbus_realize_and_unref(sbd, &error_fatal);
sysbus_connect_irq(sbd, 0, irq);
sysbus_mmio_map(sbd, 0, mmio_base);
+
+ fdctrl_init_drives(&sys->state.bus, fds);
}
void sun4m_fdctrl_init(qemu_irq irq, hwaddr io_base,
@@ -2599,15 +2606,13 @@ void sun4m_fdctrl_init(qemu_irq irq, hwaddr io_base,
FDCtrlSysBus *sys;
dev = qdev_new("SUNW,fdtwo");
- if (fds[0]) {
- qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(fds[0]),
- &error_fatal);
- }
sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
sys = SYSBUS_FDC(dev);
sysbus_connect_irq(SYS_BUS_DEVICE(sys), 0, irq);
sysbus_mmio_map(SYS_BUS_DEVICE(sys), 0, io_base);
*fdc_tc = qdev_get_gpio_in(dev, 0);
+
+ fdctrl_init_drives(&sys->state.bus, fds);
}
static void fdctrl_realize_common(DeviceState *dev, FDCtrl *fdctrl,
diff --git a/hw/isa/isa-superio.c b/hw/isa/isa-superio.c
index d3d58f9f16..e2e47d8fd9 100644
--- a/hw/isa/isa-superio.c
+++ b/hw/isa/isa-superio.c
@@ -17,6 +17,7 @@
#include "sysemu/sysemu.h"
#include "sysemu/blockdev.h"
#include "chardev/char.h"
+#include "hw/block/fdc.h"
#include "hw/isa/superio.h"
#include "hw/qdev-properties.h"
#include "hw/input/i8042.h"
@@ -31,7 +32,7 @@ static void isa_superio_realize(DeviceState *dev, Error **errp)
ISADevice *isa;
DeviceState *d;
Chardev *chr;
- DriveInfo *drive;
+ DriveInfo *fd[MAX_FD];
char *name;
int i;
@@ -115,7 +116,7 @@ static void isa_superio_realize(DeviceState *dev, Error **errp)
/* Floppy disc */
if (!k->floppy.is_enabled || k->floppy.is_enabled(sio, 0)) {
- isa = isa_new("isa-fdc");
+ isa = isa_new(TYPE_ISA_FDC);
d = DEVICE(isa);
if (k->floppy.get_iobase) {
qdev_prop_set_uint32(d, "iobase", k->floppy.get_iobase(sio, 0));
@@ -124,19 +125,12 @@ static void isa_superio_realize(DeviceState *dev, Error **errp)
qdev_prop_set_uint32(d, "irq", k->floppy.get_irq(sio, 0));
}
/* FIXME use a qdev drive property instead of drive_get() */
- drive = drive_get(IF_FLOPPY, 0, 0);
- if (drive != NULL) {
- qdev_prop_set_drive(d, "driveA", blk_by_legacy_dinfo(drive),
- &error_fatal);
- }
- /* FIXME use a qdev drive property instead of drive_get() */
- drive = drive_get(IF_FLOPPY, 0, 1);
- if (drive != NULL) {
- qdev_prop_set_drive(d, "driveB", blk_by_legacy_dinfo(drive),
- &error_fatal);
+ for (i = 0; i < MAX_FD; i++) {
+ fd[i] = drive_get(IF_FLOPPY, 0, i);
}
object_property_add_child(OBJECT(sio), "isa-fdc", OBJECT(isa));
isa_realize_and_unref(isa, bus, &error_fatal);
+ isa_fdc_init_drives(isa, fd);
sio->floppy = isa;
trace_superio_create_floppy(0,
k->floppy.get_iobase ?
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 97e6d3a025..9c8655cffc 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -341,16 +341,9 @@ static void ebus_realize(PCIDevice *pci_dev, Error **errp)
}
isa_dev = isa_new(TYPE_ISA_FDC);
dev = DEVICE(isa_dev);
- if (fd[0]) {
- qdev_prop_set_drive(dev, "driveA", blk_by_legacy_dinfo(fd[0]),
- &error_abort);
- }
- if (fd[1]) {
- qdev_prop_set_drive(dev, "driveB", blk_by_legacy_dinfo(fd[1]),
- &error_abort);
- }
qdev_prop_set_uint32(dev, "dma", -1);
isa_realize_and_unref(isa_dev, s->isa_bus, &error_fatal);
+ isa_fdc_init_drives(isa_dev, fd);
/* Power */
dev = qdev_new(TYPE_SUN4U_POWER);
diff --git a/tests/qemu-iotests/172 b/tests/qemu-iotests/172
index 18056bcef7..3abfa72948 100755
--- a/tests/qemu-iotests/172
+++ b/tests/qemu-iotests/172
@@ -148,9 +148,10 @@ echo === Mixing -fdX and -global ===
check_floppy_qtree -fda "$TEST_IMG" -drive if=none,file="$TEST_IMG.2" -global isa-fdc.driveB=none0
check_floppy_qtree -fdb "$TEST_IMG" -drive if=none,file="$TEST_IMG.2" -global isa-fdc.driveA=none0
-# Conflicting (-fdX wins)
+# Conflicting
check_floppy_qtree -fda "$TEST_IMG" -drive if=none,file="$TEST_IMG.2" -global isa-fdc.driveA=none0
check_floppy_qtree -fdb "$TEST_IMG" -drive if=none,file="$TEST_IMG.2" -global isa-fdc.driveB=none0
+# Conflicting, -fdX wins
check_floppy_qtree -fda "$TEST_IMG" -drive if=none,file="$TEST_IMG.2" -global floppy.drive=none0
echo
diff --git a/tests/qemu-iotests/172.out b/tests/qemu-iotests/172.out
index 68e7a5ea5f..340dbd39cb 100644
--- a/tests/qemu-iotests/172.out
+++ b/tests/qemu-iotests/172.out
@@ -205,22 +205,22 @@ Testing: -fdb
dev: floppy, id ""
unit = 1 (0x1)
drive = "floppy1"
- logical_block_size = 512 (0x200)
- physical_block_size = 512 (0x200)
- min_io_size = 0 (0x0)
- opt_io_size = 0 (0x0)
- discard_granularity = 4294967295 (0xffffffff)
+ logical_block_size = 512 (512 B)
+ physical_block_size = 512 (512 B)
+ min_io_size = 0 (0 B)
+ opt_io_size = 0 (0 B)
+ discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
drive-type = "288"
dev: floppy, id ""
unit = 0 (0x0)
drive = "floppy0"
- logical_block_size = 512 (0x200)
- physical_block_size = 512 (0x200)
- min_io_size = 0 (0x0)
- opt_io_size = 0 (0x0)
- discard_granularity = 4294967295 (0xffffffff)
+ logical_block_size = 512 (512 B)
+ physical_block_size = 512 (512 B)
+ min_io_size = 0 (0 B)
+ opt_io_size = 0 (0 B)
+ discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
drive-type = "288"
@@ -675,17 +675,6 @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global is
isa irq 6
bus: floppy-bus.0
type floppy-bus
- dev: floppy, id ""
- unit = 1 (0x1)
- drive = "none0"
- logical_block_size = 512 (512 B)
- physical_block_size = 512 (512 B)
- min_io_size = 0 (0 B)
- opt_io_size = 0 (0 B)
- discard_granularity = 4294967295 (4 GiB)
- write-cache = "auto"
- share-rw = false
- drive-type = "144"
dev: floppy, id ""
unit = 0 (0x0)
drive = "floppy0"
@@ -697,13 +686,24 @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global is
write-cache = "auto"
share-rw = false
drive-type = "144"
+ dev: floppy, id ""
+ unit = 1 (0x1)
+ drive = "none0"
+ logical_block_size = 512 (512 B)
+ physical_block_size = 512 (512 B)
+ min_io_size = 0 (0 B)
+ opt_io_size = 0 (0 B)
+ discard_granularity = 4294967295 (4 GiB)
+ write-cache = "auto"
+ share-rw = false
+ drive-type = "144"
floppy0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
- Attached to: /machine/unattached/device[15]
+ Attached to: /machine/unattached/device[16]
Removable device: not locked, tray closed
Cache mode: writeback
none0 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
- Attached to: /machine/unattached/device[16]
+ Attached to: /machine/unattached/device[15]
Removable device: not locked, tray closed
Cache mode: writeback
@@ -773,92 +773,10 @@ sd0: [not inserted]
Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveA=none0
-
- dev: isa-fdc, id ""
- iobase = 1008 (0x3f0)
- irq = 6 (0x6)
- dma = 2 (0x2)
- driveA = ""
- driveB = ""
- check_media_rate = true
- fdtypeA = "auto"
- fdtypeB = "auto"
- fallback = "288"
- isa irq 6
- bus: floppy-bus.0
- type floppy-bus
- dev: floppy, id ""
- unit = 0 (0x0)
- drive = "floppy0"
- logical_block_size = 512 (512 B)
- physical_block_size = 512 (512 B)
- min_io_size = 0 (0 B)
- opt_io_size = 0 (0 B)
- discard_granularity = 4294967295 (4 GiB)
- write-cache = "auto"
- share-rw = false
- drive-type = "144"
-floppy0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
- Attached to: /machine/unattached/device[15]
- Removable device: not locked, tray closed
- Cache mode: writeback
-
-none0 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
- Attached to: /machine/unattached/device[14]
- Cache mode: writeback
-
-ide1-cd0: [not inserted]
- Attached to: /machine/unattached/device[22]
- Removable device: not locked, tray closed
-
-sd0: [not inserted]
- Removable device: not locked, tray closed
-(qemu) quit
-
+QEMU_PROG: Floppy unit 0 is in use
Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveB=none0
-
- dev: isa-fdc, id ""
- iobase = 1008 (0x3f0)
- irq = 6 (0x6)
- dma = 2 (0x2)
- driveA = ""
- driveB = ""
- check_media_rate = true
- fdtypeA = "auto"
- fdtypeB = "auto"
- fallback = "288"
- isa irq 6
- bus: floppy-bus.0
- type floppy-bus
- dev: floppy, id ""
- unit = 1 (0x1)
- drive = "floppy1"
- logical_block_size = 512 (512 B)
- physical_block_size = 512 (512 B)
- min_io_size = 0 (0 B)
- opt_io_size = 0 (0 B)
- discard_granularity = 4294967295 (4 GiB)
- write-cache = "auto"
- share-rw = false
- drive-type = "144"
-floppy1 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
- Attached to: /machine/unattached/device[15]
- Removable device: not locked, tray closed
- Cache mode: writeback
-
-none0 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
- Attached to: /machine/unattached/device[14]
- Cache mode: writeback
-
-ide1-cd0: [not inserted]
- Attached to: /machine/unattached/device[22]
- Removable device: not locked, tray closed
-
-sd0: [not inserted]
- Removable device: not locked, tray closed
-(qemu) quit
-
+QEMU_PROG: Floppy unit 1 is in use
Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global floppy.drive=none0
@@ -878,11 +796,11 @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global fl
dev: floppy, id ""
unit = 0 (0x0)
drive = "floppy0"
- logical_block_size = 512 (0x200)
- physical_block_size = 512 (0x200)
- min_io_size = 0 (0x0)
- opt_io_size = 0 (0x0)
- discard_granularity = 4294967295 (0xffffffff)
+ logical_block_size = 512 (512 B)
+ physical_block_size = 512 (512 B)
+ min_io_size = 0 (0 B)
+ opt_io_size = 0 (0 B)
+ discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
drive-type = "144"
@@ -1500,11 +1418,11 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -global floppy.drive=none0 -device
dev: floppy, id ""
unit = 0 (0x0)
drive = "none0"
- logical_block_size = 512 (0x200)
- physical_block_size = 512 (0x200)
- min_io_size = 0 (0x0)
- opt_io_size = 0 (0x0)
- discard_granularity = 4294967295 (0xffffffff)
+ logical_block_size = 512 (512 B)
+ physical_block_size = 512 (512 B)
+ min_io_size = 0 (0 B)
+ opt_io_size = 0 (0 B)
+ discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
drive-type = "144"
@@ -1546,11 +1464,11 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco
dev: floppy, id ""
unit = 0 (0x0)
drive = "none1"
- logical_block_size = 512 (0x200)
- physical_block_size = 512 (0x200)
- min_io_size = 0 (0x0)
- opt_io_size = 0 (0x0)
- discard_granularity = 4294967295 (0xffffffff)
+ logical_block_size = 512 (512 B)
+ physical_block_size = 512 (512 B)
+ min_io_size = 0 (0 B)
+ opt_io_size = 0 (0 B)
+ discard_granularity = 4294967295 (4 GiB)
write-cache = "auto"
share-rw = false
drive-type = "144"
@@ -1585,7 +1503,7 @@ Testing: -fda -device floppy,drive=floppy0
QEMU_PROG: -device floppy,drive=floppy0: Drive 'floppy0' is already in use because it has been automatically connected to another device (did you need 'if=none' in the drive options?)
Testing: -fda -global floppy.drive=floppy0
-QEMU_PROG: can't apply global floppy.drive=floppy0: Drive 'floppy0' is already in use because it has been automatically connected to another device (did you need 'if=none' in the drive options?)
+QEMU_PROG: Drive 'floppy0' is already in use because it has been automatically connected to another device (did you need 'if=none' in the drive options?)
Testing: -device floppy,drive=floppy0
QEMU_PROG: -device floppy,drive=floppy0: Property 'floppy.drive' can't find value 'floppy0'
--
2.26.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 05/16] fdc: Open-code fdctrl_init_isa()
2020-06-22 9:42 [PATCH v2 00/16] Crazy shit around -global (pardon my french) Markus Armbruster
` (3 preceding siblings ...)
2020-06-22 9:42 ` [PATCH v2 04/16] fdc: Reject clash between -drive if=floppy and -global isa-fdc Markus Armbruster
@ 2020-06-22 9:42 ` Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 06/16] fdc: Deprecate configuring floppies with -global isa-fdc Markus Armbruster
` (12 subsequent siblings)
17 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2020-06-22 9:42 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, berrange, ehabkost, qemu-block,
Philippe Mathieu-Daudé,
mreitz, pbonzini, jsnow
Helper function fdctrl_init_isa() is less than helpful: one of three
places creating "isa-fdc" devices use it. Open-code it there, and
drop the function.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
include/hw/block/fdc.h | 1 -
hw/block/fdc.c | 14 --------------
hw/i386/pc.c | 8 ++++++--
3 files changed, 6 insertions(+), 17 deletions(-)
diff --git a/include/hw/block/fdc.h b/include/hw/block/fdc.h
index 8855d3476c..d232d3fa1e 100644
--- a/include/hw/block/fdc.h
+++ b/include/hw/block/fdc.h
@@ -10,7 +10,6 @@
#define TYPE_ISA_FDC "isa-fdc"
void isa_fdc_init_drives(ISADevice *fdc, DriveInfo **fds);
-ISADevice *fdctrl_init_isa(ISABus *bus, DriveInfo **fds);
void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
hwaddr mmio_base, DriveInfo **fds);
void sun4m_fdctrl_init(qemu_irq irq, hwaddr io_base,
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 2650dcb0df..d1f7722cff 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -2565,20 +2565,6 @@ static void fdctrl_connect_drives(FDCtrl *fdctrl, DeviceState *fdc_dev,
}
}
-ISADevice *fdctrl_init_isa(ISABus *bus, DriveInfo **fds)
-{
- ISADevice *isadev;
-
- isadev = isa_try_new(TYPE_ISA_FDC);
- if (!isadev) {
- return NULL;
- }
- isa_realize_and_unref(isadev, bus, &error_fatal);
-
- isa_fdc_init_drives(isadev, fds);
- return isadev;
-}
-
void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
hwaddr mmio_base, DriveInfo **fds)
{
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index d103b8c0ab..f670bcd6e6 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1142,7 +1142,7 @@ static void pc_superio_init(ISABus *isa_bus, bool create_fdctrl, bool no_vmport)
int i;
DriveInfo *fd[MAX_FD];
qemu_irq *a20_line;
- ISADevice *i8042, *port92, *vmmouse;
+ ISADevice *fdc, *i8042, *port92, *vmmouse;
serial_hds_isa_init(isa_bus, 0, MAX_ISA_SERIAL_PORTS);
parallel_hds_isa_init(isa_bus, MAX_PARALLEL_PORTS);
@@ -1152,7 +1152,11 @@ static void pc_superio_init(ISABus *isa_bus, bool create_fdctrl, bool no_vmport)
create_fdctrl |= !!fd[i];
}
if (create_fdctrl) {
- fdctrl_init_isa(isa_bus, fd);
+ fdc = isa_new(TYPE_ISA_FDC);
+ if (fdc) {
+ isa_realize_and_unref(fdc, isa_bus, &error_fatal);
+ isa_fdc_init_drives(fdc, fd);
+ }
}
i8042 = isa_create_simple(isa_bus, "i8042");
--
2.26.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 06/16] fdc: Deprecate configuring floppies with -global isa-fdc
2020-06-22 9:42 [PATCH v2 00/16] Crazy shit around -global (pardon my french) Markus Armbruster
` (4 preceding siblings ...)
2020-06-22 9:42 ` [PATCH v2 05/16] fdc: Open-code fdctrl_init_isa() Markus Armbruster
@ 2020-06-22 9:42 ` Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 07/16] docs/qdev-device-use.txt: Update section "Default Devices" Markus Armbruster
` (11 subsequent siblings)
17 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2020-06-22 9:42 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, berrange, ehabkost, qemu-block, mreitz, pbonzini, jsnow
Deprecate
-global isa-fdc.driveA=...
-global isa-fdc.driveB=...
in favour of
-device floppy,unit=0,drive=...
-device floppy,unit=1,drive=...
Same for the other floppy controller devices.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Acked-by: John Snow <jsnow@redhat.com>
---
docs/qdev-device-use.txt | 13 ++++---------
docs/system/deprecated.rst | 26 ++++++++++++++++++++++++++
hw/block/fdc.c | 17 +++++++++++++++++
tests/qemu-iotests/172.out | 30 ++++++++++++++++++++++++++++++
4 files changed, 77 insertions(+), 9 deletions(-)
diff --git a/docs/qdev-device-use.txt b/docs/qdev-device-use.txt
index cc53e97dcd..3d781be547 100644
--- a/docs/qdev-device-use.txt
+++ b/docs/qdev-device-use.txt
@@ -104,15 +104,10 @@ The -device argument differs in detail for each type of drive:
* if=floppy
- -global isa-fdc.driveA=DRIVE-ID
- -global isa-fdc.driveB=DRIVE-ID
+ -device floppy,unit=UNIT,drive=DRIVE-ID
- This is -global instead of -device, because the floppy controller is
- created automatically, and we want to configure that one, not create
- a second one (which isn't possible anyway).
-
- Without any -global isa-fdc,... you get an empty driveA and no
- driveB. You can use -nodefaults to suppress the default driveA, see
+ Without any -device floppy,... you get an empty unit 0 and no unit
+ 1. You can use -nodefaults to suppress the default unit 0, see
"Default Devices".
* if=virtio
@@ -385,7 +380,7 @@ some DEVNAMEs:
default device suppressing DEVNAMEs
CD-ROM ide-cd, ide-drive, ide-hd, scsi-cd, scsi-hd
- isa-fdc's driveA floppy, isa-fdc
+ floppy floppy, isa-fdc
parallel isa-parallel
serial isa-serial
VGA VGA, cirrus-vga, isa-vga, isa-cirrus-vga,
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 3a255591c3..6f8bf19d37 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -164,6 +164,32 @@ previously available ``-tb-size`` option.
Use ``-display sdl,show-cursor=on`` or
``-display gtk,show-cursor=on`` instead.
+``Configuring floppies with ``-global``
+'''''''''''''''''''''''''''''''''''''''
+
+Use ``-device floppy,...`` instead:
+::
+
+ -global isa-fdc.driveA=...
+ -global sysbus-fdc.driveA=...
+ -global SUNW,fdtwo.drive=...
+
+become
+::
+
+ -device floppy,unit=0,drive=...
+
+and
+::
+
+ -global isa-fdc.driveB=...
+ -global sysbus-fdc.driveB=...
+
+become
+::
+
+ -device floppy,unit=1,drive=...
+
QEMU Machine Protocol (QMP) commands
------------------------------------
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index d1f7722cff..7e143cbab0 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -2528,6 +2528,7 @@ static void fdctrl_connect_drives(FDCtrl *fdctrl, DeviceState *fdc_dev,
DeviceState *dev;
BlockBackend *blk;
Error *local_err = NULL;
+ const char *fdc_name, *drive_suffix;
for (i = 0; i < MAX_FD; i++) {
drive = &fdctrl->drives[i];
@@ -2542,10 +2543,26 @@ static void fdctrl_connect_drives(FDCtrl *fdctrl, DeviceState *fdc_dev,
continue;
}
+ fdc_name = object_get_typename(OBJECT(fdc_dev));
+ drive_suffix = !strcmp(fdc_name, "SUNW,fdtwo") ? "" : i ? "B" : "A";
+ warn_report("warning: property %s.drive%s is deprecated",
+ fdc_name, drive_suffix);
+ error_printf("Use -device floppy,unit=%d,drive=... instead.\n", i);
+
dev = qdev_new("floppy");
qdev_prop_set_uint32(dev, "unit", i);
qdev_prop_set_enum(dev, "drive-type", fdctrl->qdev_for_drives[i].type);
+ /*
+ * Hack alert: we move the backend from the floppy controller
+ * device to the floppy device. We first need to detach the
+ * controller, or else floppy_create()'s qdev_prop_set_drive()
+ * will die when it attaches floppy device. We also need to
+ * take another reference so that blk_detach_dev() doesn't
+ * free blk while we still need it.
+ *
+ * The hack is probably a bad idea.
+ */
blk_ref(blk);
blk_detach_dev(blk, fdc_dev);
fdctrl->qdev_for_drives[i].blk = NULL;
diff --git a/tests/qemu-iotests/172.out b/tests/qemu-iotests/172.out
index 340dbd39cb..778406e4bf 100644
--- a/tests/qemu-iotests/172.out
+++ b/tests/qemu-iotests/172.out
@@ -383,6 +383,8 @@ sd0: [not inserted]
=== Using -drive if=none and -global ===
Testing: -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveA=none0
+QEMU_PROG: warning: warning: property isa-fdc.driveA is deprecated
+Use -device floppy,unit=0,drive=... instead.
dev: isa-fdc, id ""
iobase = 1008 (0x3f0)
@@ -423,6 +425,8 @@ sd0: [not inserted]
Testing: -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveB=none0
+QEMU_PROG: warning: warning: property isa-fdc.driveB is deprecated
+Use -device floppy,unit=1,drive=... instead.
dev: isa-fdc, id ""
iobase = 1008 (0x3f0)
@@ -463,6 +467,10 @@ sd0: [not inserted]
Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveA=none0 -global isa-fdc.driveB=none1
+QEMU_PROG: warning: warning: property isa-fdc.driveA is deprecated
+Use -device floppy,unit=0,drive=... instead.
+QEMU_PROG: warning: warning: property isa-fdc.driveB is deprecated
+Use -device floppy,unit=1,drive=... instead.
dev: isa-fdc, id ""
iobase = 1008 (0x3f0)
@@ -661,6 +669,8 @@ sd0: [not inserted]
=== Mixing -fdX and -global ===
Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveB=none0
+QEMU_PROG: warning: warning: property isa-fdc.driveB is deprecated
+Use -device floppy,unit=1,drive=... instead.
dev: isa-fdc, id ""
iobase = 1008 (0x3f0)
@@ -717,6 +727,8 @@ sd0: [not inserted]
Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveA=none0
+QEMU_PROG: warning: warning: property isa-fdc.driveA is deprecated
+Use -device floppy,unit=0,drive=... instead.
dev: isa-fdc, id ""
iobase = 1008 (0x3f0)
@@ -773,9 +785,13 @@ sd0: [not inserted]
Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveA=none0
+QEMU_PROG: warning: warning: property isa-fdc.driveA is deprecated
+Use -device floppy,unit=0,drive=... instead.
QEMU_PROG: Floppy unit 0 is in use
Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveB=none0
+QEMU_PROG: warning: warning: property isa-fdc.driveB is deprecated
+Use -device floppy,unit=1,drive=... instead.
QEMU_PROG: Floppy unit 1 is in use
Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global floppy.drive=none0
@@ -1177,6 +1193,8 @@ QEMU_PROG: -device floppy,drive=none0,unit=0: Floppy unit 0 is in use
=== Mixing -global and -device ===
Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveA=none0 -device floppy,drive=none1
+QEMU_PROG: warning: warning: property isa-fdc.driveA is deprecated
+Use -device floppy,unit=0,drive=... instead.
dev: isa-fdc, id ""
iobase = 1008 (0x3f0)
@@ -1233,6 +1251,8 @@ sd0: [not inserted]
Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveA=none0 -device floppy,drive=none1,unit=1
+QEMU_PROG: warning: warning: property isa-fdc.driveA is deprecated
+Use -device floppy,unit=0,drive=... instead.
dev: isa-fdc, id ""
iobase = 1008 (0x3f0)
@@ -1289,6 +1309,8 @@ sd0: [not inserted]
Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveB=none0 -device floppy,drive=none1
+QEMU_PROG: warning: warning: property isa-fdc.driveB is deprecated
+Use -device floppy,unit=1,drive=... instead.
dev: isa-fdc, id ""
iobase = 1008 (0x3f0)
@@ -1345,6 +1367,8 @@ sd0: [not inserted]
Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveB=none0 -device floppy,drive=none1,unit=0
+QEMU_PROG: warning: warning: property isa-fdc.driveB is deprecated
+Use -device floppy,unit=1,drive=... instead.
dev: isa-fdc, id ""
iobase = 1008 (0x3f0)
@@ -1441,9 +1465,13 @@ sd0: [not inserted]
Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveA=none0 -device floppy,drive=none1,unit=0
+QEMU_PROG: warning: warning: property isa-fdc.driveA is deprecated
+Use -device floppy,unit=0,drive=... instead.
QEMU_PROG: -device floppy,drive=none1,unit=0: Floppy unit 0 is in use
Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveB=none0 -device floppy,drive=none1,unit=1
+QEMU_PROG: warning: warning: property isa-fdc.driveB is deprecated
+Use -device floppy,unit=1,drive=... instead.
QEMU_PROG: -device floppy,drive=none1,unit=1: Floppy unit 1 is in use
Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global floppy.drive=none0 -device floppy,drive=none1,unit=0
@@ -1512,6 +1540,8 @@ QEMU_PROG: -device floppy,drive=floppy0: Property 'floppy.drive' can't find valu
=== Too many floppy drives ===
Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -drive if=none,file=TEST_DIR/t.qcow2.3 -global isa-fdc.driveB=none0 -device floppy,drive=none1
+QEMU_PROG: warning: warning: property isa-fdc.driveB is deprecated
+Use -device floppy,unit=1,drive=... instead.
QEMU_PROG: -device floppy,drive=none1: Can't create floppy unit 2, bus supports only 2 units
--
2.26.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 07/16] docs/qdev-device-use.txt: Update section "Default Devices"
2020-06-22 9:42 [PATCH v2 00/16] Crazy shit around -global (pardon my french) Markus Armbruster
` (5 preceding siblings ...)
2020-06-22 9:42 ` [PATCH v2 06/16] fdc: Deprecate configuring floppies with -global isa-fdc Markus Armbruster
@ 2020-06-22 9:42 ` Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 08/16] blockdev: Deprecate -drive with bogus interface type Markus Armbruster
` (10 subsequent siblings)
17 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2020-06-22 9:42 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, berrange, ehabkost, qemu-block, mreitz, pbonzini, jsnow
Resynchronize the table of default device suppressions with vl.c's
default_list[].
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
docs/qdev-device-use.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/docs/qdev-device-use.txt b/docs/qdev-device-use.txt
index 3d781be547..4bbbcf561f 100644
--- a/docs/qdev-device-use.txt
+++ b/docs/qdev-device-use.txt
@@ -384,8 +384,8 @@ some DEVNAMEs:
parallel isa-parallel
serial isa-serial
VGA VGA, cirrus-vga, isa-vga, isa-cirrus-vga,
- vmware-svga, qxl-vga, virtio-vga
- virtioconsole virtio-serial-pci, virtio-serial
+ vmware-svga, qxl-vga, virtio-vga, ati-vga,
+ vhost-user-vga
The default NIC is connected to a default part created along with it.
It is *not* suppressed by configuring a NIC with -device (you may call
--
2.26.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 08/16] blockdev: Deprecate -drive with bogus interface type
2020-06-22 9:42 [PATCH v2 00/16] Crazy shit around -global (pardon my french) Markus Armbruster
` (6 preceding siblings ...)
2020-06-22 9:42 ` [PATCH v2 07/16] docs/qdev-device-use.txt: Update section "Default Devices" Markus Armbruster
@ 2020-06-22 9:42 ` Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 09/16] qdev: Eliminate get_pointer(), set_pointer() Markus Armbruster
` (9 subsequent siblings)
17 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2020-06-22 9:42 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, berrange, ehabkost, qemu-block, mreitz, pbonzini, jsnow
Drives with interface types other than if=none are for onboard
devices. Unfortunately, any such drives the board doesn't pick up can
still be used with -device, like this:
$ qemu-system-x86_64 -nodefaults -display none -S -drive if=floppy,id=bogus,unit=7 -device ide-cd,drive=bogus -monitor stdio
QEMU 5.0.50 monitor - type 'help' for more information
(qemu) info block
bogus: [not inserted]
Attached to: /machine/peripheral-anon/device[0]
Removable device: not locked, tray closed
(qemu) info qtree
bus: main-system-bus
type System
[...]
bus: ide.1
type IDE
dev: ide-cd, id ""
---> drive = "bogus"
[...]
unit = 0 (0x0)
[...]
This kind of abuse has always worked. Deprecate it:
qemu-system-x86_64: -drive if=floppy,id=bogus,unit=7: warning: bogus if=floppy is deprecated, use if=none
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
docs/system/deprecated.rst | 8 ++++++++
include/sysemu/blockdev.h | 2 ++
blockdev.c | 27 +++++++++++++++++++++++++--
softmmu/vl.c | 8 ++++++++
4 files changed, 43 insertions(+), 2 deletions(-)
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 6f8bf19d37..728233a8ff 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -190,6 +190,14 @@ become
-device floppy,unit=1,drive=...
+``-drive`` with bogus interface type
+''''''''''''''''''''''''''''''''''''
+
+Drives with interface types other than ``if=none`` are for onboard
+devices. It is possible to use drives the board doesn't pick up with
+-device. This usage is now deprecated. Use ``if=none`` instead.
+
+
QEMU Machine Protocol (QMP) commands
------------------------------------
diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index a86d99b3d8..3b5fcda08d 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -35,6 +35,7 @@ struct DriveInfo {
bool is_default; /* Added by default_drive() ? */
int media_cd;
QemuOpts *opts;
+ bool claimed_by_board;
QTAILQ_ENTRY(DriveInfo) next;
};
@@ -45,6 +46,7 @@ BlockBackend *blk_by_legacy_dinfo(DriveInfo *dinfo);
void override_max_devs(BlockInterfaceType type, int max_devs);
DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit);
+void drive_mark_claimed_by_board(void);
void drive_check_orphaned(void);
DriveInfo *drive_get_by_index(BlockInterfaceType type, int index);
int drive_get_max_bus(BlockInterfaceType type);
diff --git a/blockdev.c b/blockdev.c
index 72df193ca7..31d5eaf6bf 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -239,6 +239,19 @@ DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit)
return NULL;
}
+void drive_mark_claimed_by_board(void)
+{
+ BlockBackend *blk;
+ DriveInfo *dinfo;
+
+ for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {
+ dinfo = blk_legacy_dinfo(blk);
+ if (dinfo && blk_get_attached_dev(blk)) {
+ dinfo->claimed_by_board = true;
+ }
+ }
+}
+
void drive_check_orphaned(void)
{
BlockBackend *blk;
@@ -248,8 +261,10 @@ void drive_check_orphaned(void)
for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {
dinfo = blk_legacy_dinfo(blk);
- if (!blk_get_attached_dev(blk) && !dinfo->is_default &&
- dinfo->type != IF_NONE) {
+ if (dinfo->is_default || dinfo->type == IF_NONE) {
+ continue;
+ }
+ if (!blk_get_attached_dev(blk)) {
loc_push_none(&loc);
qemu_opts_loc_restore(dinfo->opts);
error_report("machine type does not support"
@@ -257,6 +272,14 @@ void drive_check_orphaned(void)
if_name[dinfo->type], dinfo->bus, dinfo->unit);
loc_pop(&loc);
orphans = true;
+ continue;
+ }
+ if (!dinfo->claimed_by_board && dinfo->type != IF_VIRTIO) {
+ loc_push_none(&loc);
+ qemu_opts_loc_restore(dinfo->opts);
+ warn_report("bogus if=%s is deprecated, use if=none",
+ if_name[dinfo->type]);
+ loc_pop(&loc);
}
}
diff --git a/softmmu/vl.c b/softmmu/vl.c
index f669c06ede..3e15ee2435 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -4347,6 +4347,14 @@ void qemu_init(int argc, char **argv, char **envp)
/* from here on runstate is RUN_STATE_PRELAUNCH */
machine_run_board_init(current_machine);
+ /*
+ * TODO To drop support for deprecated bogus if=..., move
+ * drive_check_orphaned() here, replacing this call. Also drop
+ * its deprecation warning, along with DriveInfo member
+ * @claimed_by_board.
+ */
+ drive_mark_claimed_by_board();
+
realtime_init();
soundhw_init();
--
2.26.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 09/16] qdev: Eliminate get_pointer(), set_pointer()
2020-06-22 9:42 [PATCH v2 00/16] Crazy shit around -global (pardon my french) Markus Armbruster
` (7 preceding siblings ...)
2020-06-22 9:42 ` [PATCH v2 08/16] blockdev: Deprecate -drive with bogus interface type Markus Armbruster
@ 2020-06-22 9:42 ` Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 10/16] qdev: Improve netdev property override error a bit Markus Armbruster
` (8 subsequent siblings)
17 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2020-06-22 9:42 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, berrange, ehabkost, qemu-block, mreitz, pbonzini, jsnow
We stopped using get_pointer() and set_pointer() for netdev in commit
23120b13c6 "net: don't use set/get_pointer() in set/get_netdev()"
(v2.3.0), and for chardev in commit becdfa00cf "char: replace PROP_CHR
with CharBackend" (v2.8.0). With only the drive user left, they're
not helpful anymore. Eliminate.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/core/qdev-properties-system.c | 95 ++++++++++++--------------------
1 file changed, 35 insertions(+), 60 deletions(-)
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 70bfd4809b..9aa80495ee 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -25,29 +25,45 @@
#include "sysemu/iothread.h"
#include "sysemu/tpm_backend.h"
-static void get_pointer(Object *obj, Visitor *v, Property *prop,
- char *(*print)(void *ptr),
- const char *name, Error **errp)
+/* --- drive --- */
+
+static void get_drive(Object *obj, Visitor *v, const char *name, void *opaque,
+ Error **errp)
{
DeviceState *dev = DEVICE(obj);
+ Property *prop = opaque;
void **ptr = qdev_get_prop_ptr(dev, prop);
+ const char *value;
char *p;
- p = *ptr ? print(*ptr) : g_strdup("");
+ if (*ptr) {
+ value = blk_name(*ptr);
+ if (!*value) {
+ BlockDriverState *bs = blk_bs(*ptr);
+ if (bs) {
+ value = bdrv_get_node_name(bs);
+ }
+ }
+ } else {
+ value = "";
+ }
+
+ p = g_strdup(value);
visit_type_str(v, name, &p, errp);
g_free(p);
}
-static void set_pointer(Object *obj, Visitor *v, Property *prop,
- void (*parse)(DeviceState *dev, const char *str,
- void **ptr, const char *propname,
- Error **errp),
- const char *name, Error **errp)
+static void set_drive_helper(Object *obj, Visitor *v, const char *name,
+ void *opaque, bool iothread, Error **errp)
{
DeviceState *dev = DEVICE(obj);
+ Property *prop = opaque;
Error *local_err = NULL;
void **ptr = qdev_get_prop_ptr(dev, prop);
char *str;
+ BlockBackend *blk;
+ bool blk_created = false;
+ int ret;
if (dev->realized) {
qdev_prop_set_after_realize(dev, name, errp);
@@ -59,23 +75,12 @@ static void set_pointer(Object *obj, Visitor *v, Property *prop,
error_propagate(errp, local_err);
return;
}
+
if (!*str) {
g_free(str);
*ptr = NULL;
return;
}
- parse(dev, str, ptr, prop->name, errp);
- g_free(str);
-}
-
-/* --- drive --- */
-
-static void do_parse_drive(DeviceState *dev, const char *str, void **ptr,
- const char *propname, bool iothread, Error **errp)
-{
- BlockBackend *blk;
- bool blk_created = false;
- int ret;
blk = blk_by_name(str);
if (!blk) {
@@ -101,7 +106,7 @@ static void do_parse_drive(DeviceState *dev, const char *str, void **ptr,
}
if (!blk) {
error_setg(errp, "Property '%s.%s' can't find value '%s'",
- object_get_typename(OBJECT(dev)), propname, str);
+ object_get_typename(OBJECT(dev)), prop->name, str);
goto fail;
}
if (blk_attach_dev(blk, dev) < 0) {
@@ -126,18 +131,20 @@ fail:
/* If we need to keep a reference, blk_attach_dev() took it */
blk_unref(blk);
}
+
+ g_free(str);
}
-static void parse_drive(DeviceState *dev, const char *str, void **ptr,
- const char *propname, Error **errp)
+static void set_drive(Object *obj, Visitor *v, const char *name, void *opaque,
+ Error **errp)
{
- do_parse_drive(dev, str, ptr, propname, false, errp);
+ set_drive_helper(obj, v, name, opaque, false, errp);
}
-static void parse_drive_iothread(DeviceState *dev, const char *str, void **ptr,
- const char *propname, Error **errp)
+static void set_drive_iothread(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
{
- do_parse_drive(dev, str, ptr, propname, true, errp);
+ set_drive_helper(obj, v, name, opaque, true, errp);
}
static void release_drive(Object *obj, const char *name, void *opaque)
@@ -156,38 +163,6 @@ static void release_drive(Object *obj, const char *name, void *opaque)
}
}
-static char *print_drive(void *ptr)
-{
- const char *name;
-
- name = blk_name(ptr);
- if (!*name) {
- BlockDriverState *bs = blk_bs(ptr);
- if (bs) {
- name = bdrv_get_node_name(bs);
- }
- }
- return g_strdup(name);
-}
-
-static void get_drive(Object *obj, Visitor *v, const char *name, void *opaque,
- Error **errp)
-{
- get_pointer(obj, v, opaque, print_drive, name, errp);
-}
-
-static void set_drive(Object *obj, Visitor *v, const char *name, void *opaque,
- Error **errp)
-{
- set_pointer(obj, v, opaque, parse_drive, name, errp);
-}
-
-static void set_drive_iothread(Object *obj, Visitor *v, const char *name,
- void *opaque, Error **errp)
-{
- set_pointer(obj, v, opaque, parse_drive_iothread, name, errp);
-}
-
const PropertyInfo qdev_prop_drive = {
.name = "str",
.description = "Node name or ID of a block device to use as a backend",
--
2.26.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 10/16] qdev: Improve netdev property override error a bit
2020-06-22 9:42 [PATCH v2 00/16] Crazy shit around -global (pardon my french) Markus Armbruster
` (8 preceding siblings ...)
2020-06-22 9:42 ` [PATCH v2 09/16] qdev: Eliminate get_pointer(), set_pointer() Markus Armbruster
@ 2020-06-22 9:42 ` Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 11/16] qdev: Reject drive property override Markus Armbruster
` (7 subsequent siblings)
17 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2020-06-22 9:42 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, berrange, ehabkost, qemu-block, Jason Wang, mreitz,
pbonzini, jsnow
qdev_prop_set_netdev() fails when the property already has a non-null
value. Seems to go back to commit 30c367ed44
"qdev-properties-system.c: Allow vlan or netdev for -device, not
both", v1.7.0. Board code doesn't expect failure, and crashes:
$ qemu-system-x86_64 --nodefaults -nic user -netdev user,id=nic0 -global e1000.netdev=nic0
Unexpected error in error_set_from_qdev_prop_error() at /work/armbru/qemu/hw/core/qdev-properties.c:1101:
qemu-system-x86_64: Property 'e1000.netdev' doesn't take value '__org.qemu.nic0
'
Aborted (core dumped)
-device and device_add handle the failure:
$ qemu-system-x86_64 -nodefaults -netdev user,id=net0 -netdev user,id=net1 -device e1000,netdev=net0,netdev=net1
qemu-system-x86_64: -device e1000,netdev=net0,netdev=net1: Property 'e1000.netdev' doesn't take value 'net1'
$ qemu-system-x86_64 -nodefaults -S -display none -monitor stdio -netdev user,id=net0 -netdev user,id=net1 -global e1000.netdev=net0
QEMU 5.0.50 monitor - type 'help' for more information
(qemu) qemu-system-x86_64: warning: netdev net0 has no peer
qemu-system-x86_64: warning: netdev net1 has no peer
device_add e1000,netdev=net1
Error: Property 'e1000.netdev' doesn't take value 'net1'
Perhaps netdev property override could be made to work. Perhaps it
should. I'm not the right guy to figure this out. What I can do is
improve the error message a bit:
(qemu) device_add e1000,netdev=net1
Error: -global e1000.netdev=... conflicts with netdev=net1
Cc: Jason Wang <jasowang@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
include/hw/qdev-properties.h | 2 ++
hw/core/qdev-properties-system.c | 30 +++++++++++++++++++++++++++---
hw/core/qdev-properties.c | 17 +++++++++++++++++
3 files changed, 46 insertions(+), 3 deletions(-)
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 5252bb6b1a..fc0b2f6b45 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -251,6 +251,8 @@ void qdev_prop_set_macaddr(DeviceState *dev, const char *name,
void qdev_prop_set_enum(DeviceState *dev, const char *name, int value);
void qdev_prop_register_global(GlobalProperty *prop);
+const GlobalProperty *qdev_find_global_prop(DeviceState *dev,
+ const char *name);
int qdev_prop_check_globals(void);
void qdev_prop_set_globals(DeviceState *dev);
void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev,
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 9aa80495ee..3f84309b7e 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -25,6 +25,28 @@
#include "sysemu/iothread.h"
#include "sysemu/tpm_backend.h"
+static bool check_prop_still_unset(DeviceState *dev, const char *name,
+ const void *old_val, const char *new_val,
+ Error **errp)
+{
+ const GlobalProperty *prop = qdev_find_global_prop(dev, name);
+
+ if (!old_val) {
+ return true;
+ }
+
+ if (prop) {
+ error_setg(errp, "-global %s.%s=... conflicts with %s=%s",
+ prop->driver, prop->property, name, new_val);
+ } else {
+ /* Error message is vague, but a better one would be hard */
+ error_setg(errp, "%s=%s conflicts, and override is not implemented",
+ name, new_val);
+ }
+ return false;
+}
+
+
/* --- drive --- */
static void get_drive(Object *obj, Visitor *v, const char *name, void *opaque,
@@ -299,14 +321,16 @@ static void set_netdev(Object *obj, Visitor *v, const char *name,
}
for (i = 0; i < queues; i++) {
-
if (peers[i]->peer) {
err = -EEXIST;
goto out;
}
- if (ncs[i]) {
- err = -EINVAL;
+ /*
+ * TODO Should this really be an error? If no, the old value
+ * needs to be released before we store the new one.
+ */
+ if (!check_prop_still_unset(dev, name, ncs[i], str, errp)) {
goto out;
}
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index ead35d7ffd..71f8aca7c6 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1246,6 +1246,23 @@ void qdev_prop_register_global(GlobalProperty *prop)
g_ptr_array_add(global_props(), prop);
}
+const GlobalProperty *qdev_find_global_prop(DeviceState *dev,
+ const char *name)
+{
+ GPtrArray *props = global_props();
+ const GlobalProperty *p;
+ int i;
+
+ for (i = 0; i < props->len; i++) {
+ p = g_ptr_array_index(props, i);
+ if (object_dynamic_cast(OBJECT(dev), p->driver)
+ && !strcmp(p->property, name)) {
+ return p;
+ }
+ }
+ return NULL;
+}
+
int qdev_prop_check_globals(void)
{
int i, ret = 0;
--
2.26.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 11/16] qdev: Reject drive property override
2020-06-22 9:42 [PATCH v2 00/16] Crazy shit around -global (pardon my french) Markus Armbruster
` (9 preceding siblings ...)
2020-06-22 9:42 ` [PATCH v2 10/16] qdev: Improve netdev property override error a bit Markus Armbruster
@ 2020-06-22 9:42 ` Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 12/16] qdev: Reject chardev " Markus Armbruster
` (6 subsequent siblings)
17 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2020-06-22 9:42 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, berrange, ehabkost, qemu-block, mreitz, pbonzini, jsnow
qdev_prop_set_drive() screws up when the property already has a
non-null value: it neglects to release the old value. Both the old
and the new backend become attached to the same device.
Example (taken from iotest 172): -fda ... -drive if=none,... -global
floppy.drive=none0.
Special case: attempting to use the same backend both times fails.
Example (also from iotest 172): -fda ... -global floppy.drive=floppy0.
Yet another example: -device with multiple drive=... (but not
device_add, which silently drops all but the last duplicate property).
Perhaps drive property override could be made to work. Perhaps it
should. I can't afford the time to figure this out now. What I can
do is reject usage that leaves backends in unhealthy states. For what
it's worth, we've long done the same for netdev properties.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/core/qdev-properties-system.c | 8 +++
tests/qemu-iotests/172.out | 88 ++------------------------------
2 files changed, 11 insertions(+), 85 deletions(-)
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 3f84309b7e..6b5fc59901 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -98,6 +98,14 @@ static void set_drive_helper(Object *obj, Visitor *v, const char *name,
return;
}
+ /*
+ * TODO Should this really be an error? If no, the old value
+ * needs to be released before we store the new one.
+ */
+ if (!check_prop_still_unset(dev, name, *ptr, str, errp)) {
+ return;
+ }
+
if (!*str) {
g_free(str);
*ptr = NULL;
diff --git a/tests/qemu-iotests/172.out b/tests/qemu-iotests/172.out
index 778406e4bf..cca2894af0 100644
--- a/tests/qemu-iotests/172.out
+++ b/tests/qemu-iotests/172.out
@@ -795,48 +795,7 @@ Use -device floppy,unit=1,drive=... instead.
QEMU_PROG: Floppy unit 1 is in use
Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global floppy.drive=none0
-
- dev: isa-fdc, id ""
- iobase = 1008 (0x3f0)
- irq = 6 (0x6)
- dma = 2 (0x2)
- driveA = ""
- driveB = ""
- check_media_rate = true
- fdtypeA = "auto"
- fdtypeB = "auto"
- fallback = "288"
- isa irq 6
- bus: floppy-bus.0
- type floppy-bus
- dev: floppy, id ""
- unit = 0 (0x0)
- drive = "floppy0"
- logical_block_size = 512 (512 B)
- physical_block_size = 512 (512 B)
- min_io_size = 0 (0 B)
- opt_io_size = 0 (0 B)
- discard_granularity = 4294967295 (4 GiB)
- write-cache = "auto"
- share-rw = false
- drive-type = "144"
-floppy0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
- Attached to: /machine/unattached/device[15]
- Removable device: not locked, tray closed
- Cache mode: writeback
-
-none0 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
- Attached to: /machine/unattached/device[15]
- Cache mode: writeback
-
-ide1-cd0: [not inserted]
- Attached to: /machine/unattached/device[22]
- Removable device: not locked, tray closed
-
-sd0: [not inserted]
- Removable device: not locked, tray closed
-(qemu) quit
-
+QEMU_PROG: -global floppy.drive=... conflicts with drive=floppy0
=== Mixing -fdX and -device ===
@@ -1475,48 +1434,7 @@ Use -device floppy,unit=1,drive=... instead.
QEMU_PROG: -device floppy,drive=none1,unit=1: Floppy unit 1 is in use
Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global floppy.drive=none0 -device floppy,drive=none1,unit=0
-
- dev: isa-fdc, id ""
- iobase = 1008 (0x3f0)
- irq = 6 (0x6)
- dma = 2 (0x2)
- driveA = ""
- driveB = ""
- check_media_rate = true
- fdtypeA = "auto"
- fdtypeB = "auto"
- fallback = "288"
- isa irq 6
- bus: floppy-bus.0
- type floppy-bus
- dev: floppy, id ""
- unit = 0 (0x0)
- drive = "none1"
- logical_block_size = 512 (512 B)
- physical_block_size = 512 (512 B)
- min_io_size = 0 (0 B)
- opt_io_size = 0 (0 B)
- discard_granularity = 4294967295 (4 GiB)
- write-cache = "auto"
- share-rw = false
- drive-type = "144"
-none0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
- Attached to: /machine/peripheral-anon/device[0]
- Cache mode: writeback
-
-none1 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
- Attached to: /machine/peripheral-anon/device[0]
- Removable device: not locked, tray closed
- Cache mode: writeback
-
-ide1-cd0: [not inserted]
- Attached to: /machine/unattached/device[21]
- Removable device: not locked, tray closed
-
-sd0: [not inserted]
- Removable device: not locked, tray closed
-(qemu) quit
-
+QEMU_PROG: -device floppy,drive=none1,unit=0: -global floppy.drive=... conflicts with drive=none1
=== Attempt to use drive twice ===
@@ -1531,7 +1449,7 @@ Testing: -fda -device floppy,drive=floppy0
QEMU_PROG: -device floppy,drive=floppy0: Drive 'floppy0' is already in use because it has been automatically connected to another device (did you need 'if=none' in the drive options?)
Testing: -fda -global floppy.drive=floppy0
-QEMU_PROG: Drive 'floppy0' is already in use because it has been automatically connected to another device (did you need 'if=none' in the drive options?)
+QEMU_PROG: -global floppy.drive=... conflicts with drive=floppy0
Testing: -device floppy,drive=floppy0
QEMU_PROG: -device floppy,drive=floppy0: Property 'floppy.drive' can't find value 'floppy0'
--
2.26.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 12/16] qdev: Reject chardev property override
2020-06-22 9:42 [PATCH v2 00/16] Crazy shit around -global (pardon my french) Markus Armbruster
` (10 preceding siblings ...)
2020-06-22 9:42 ` [PATCH v2 11/16] qdev: Reject drive property override Markus Armbruster
@ 2020-06-22 9:42 ` Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 13/16] qdev: Make qdev_prop_set_drive() match the other helpers Markus Armbruster
` (5 subsequent siblings)
17 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2020-06-22 9:42 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, berrange, ehabkost, qemu-block, mreitz,
Marc-André Lureau, pbonzini, jsnow
qdev_prop_set_chr() screws up when the property already has a non-null
value: it neglects to release the old value. Both the old and the new
backend become attached to the same device. Unlike for block devices
(see previous commit), this can't be observed from the monitor (I
think).
Example: -serial null -chardev null,id=chr0 -global isa-serial.chardev=chr0
Special case: attempting to use the same backend both times crashes:
$ qemu-system-x86_64 --nodefaults -serial null -global isa-serial.chardev=serial0
Unexpected error in qemu_chr_fe_init() at /work/armbru/qemu/chardev/char-fe.c:220:
qemu-system-x86_64: Device 'serial0' is in use
Aborted (core dumped)
Yet another example: -device with multiple chardev=... (but not
device_add, which silently drops all but the last duplicate property).
Perhaps chardev property override could be made to work. Perhaps it
should. I can't afford the time to figure this out now. What I can
do reject usage that leaves backends in unhealthy states. For what
it's worth, we've long done the same for netdev properties.
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/core/qdev-properties-system.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 6b5fc59901..2561fa09a8 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -244,6 +244,14 @@ static void set_chr(Object *obj, Visitor *v, const char *name, void *opaque,
return;
}
+ /*
+ * TODO Should this really be an error? If no, the old value
+ * needs to be released before we store the new one.
+ */
+ if (!check_prop_still_unset(dev, name, be->chr, str, errp)) {
+ return;
+ }
+
if (!*str) {
g_free(str);
be->chr = NULL;
--
2.26.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 13/16] qdev: Make qdev_prop_set_drive() match the other helpers
2020-06-22 9:42 [PATCH v2 00/16] Crazy shit around -global (pardon my french) Markus Armbruster
` (11 preceding siblings ...)
2020-06-22 9:42 ` [PATCH v2 12/16] qdev: Reject chardev " Markus Armbruster
@ 2020-06-22 9:42 ` Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 14/16] arm/aspeed: Drop aspeed_board_init_flashes() parameter @errp Markus Armbruster
` (4 subsequent siblings)
17 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2020-06-22 9:42 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, berrange, ehabkost, qemu-block,
Philippe Mathieu-Daudé,
mreitz, pbonzini, jsnow
qdev_prop_set_drive() can fail. None of the other qdev_prop_set_FOO()
can; they abort on error.
To clean up this inconsistency, rename qdev_prop_set_drive() to
qdev_prop_set_drive_err(), and create a qdev_prop_set_drive() that
aborts on error.
Coccinelle script to update callers:
@ depends on !(file in "hw/core/qdev-properties-system.c")@
expression dev, name, value;
symbol error_abort;
@@
- qdev_prop_set_drive(dev, name, value, &error_abort);
+ qdev_prop_set_drive(dev, name, value);
@@
expression dev, name, value, errp;
@@
- qdev_prop_set_drive(dev, name, value, errp);
+ qdev_prop_set_drive_err(dev, name, value, errp);
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
include/hw/qdev-properties.h | 16 +++++++++++++---
hw/arm/aspeed.c | 8 ++++----
hw/arm/cubieboard.c | 2 +-
hw/arm/exynos4210.c | 2 +-
hw/arm/imx25_pdk.c | 2 +-
hw/arm/mcimx6ul-evk.c | 2 +-
hw/arm/mcimx7d-sabre.c | 2 +-
hw/arm/msf2-som.c | 4 ++--
hw/arm/nseries.c | 4 ++--
hw/arm/orangepi.c | 2 +-
hw/arm/raspi.c | 2 +-
hw/arm/sabrelite.c | 6 +++---
hw/arm/vexpress.c | 3 +--
hw/arm/xilinx_zynq.c | 7 ++++---
hw/arm/xlnx-versal-virt.c | 2 +-
hw/arm/xlnx-zcu102.c | 10 +++++-----
hw/block/fdc.c | 6 +++---
hw/block/nand.c | 2 +-
hw/block/pflash_cfi01.c | 6 +++---
hw/block/pflash_cfi02.c | 2 +-
hw/core/qdev-properties-system.c | 10 ++++++++--
hw/ide/qdev.c | 4 ++--
hw/m68k/q800.c | 3 +--
hw/microblaze/petalogix_ml605_mmu.c | 5 +++--
hw/ppc/pnv.c | 3 +--
hw/ppc/spapr.c | 4 ++--
hw/scsi/scsi-bus.c | 2 +-
hw/sd/milkymist-memcard.c | 2 +-
hw/sd/pxa2xx_mmci.c | 2 +-
hw/sd/sd.c | 2 +-
hw/sd/ssi-sd.c | 3 ++-
hw/xtensa/xtfpga.c | 3 +--
32 files changed, 74 insertions(+), 59 deletions(-)
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index fc0b2f6b45..49c6cd2460 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -233,8 +233,16 @@ extern const PropertyInfo qdev_prop_pcie_link_width;
#define DEFINE_PROP_END_OF_LIST() \
{}
-/* Set properties between creation and init. */
-void *qdev_get_prop_ptr(DeviceState *dev, Property *prop);
+/*
+ * Set properties between creation and realization.
+ */
+void qdev_prop_set_drive_err(DeviceState *dev, const char *name,
+ BlockBackend *value, Error **errp);
+
+/*
+ * Set properties between creation and realization.
+ * @value must be valid. Each property may be set at most once.
+ */
void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value);
void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value);
void qdev_prop_set_uint16(DeviceState *dev, const char *name, uint16_t value);
@@ -245,11 +253,13 @@ void qdev_prop_set_string(DeviceState *dev, const char *name, const char *value)
void qdev_prop_set_chr(DeviceState *dev, const char *name, Chardev *value);
void qdev_prop_set_netdev(DeviceState *dev, const char *name, NetClientState *value);
void qdev_prop_set_drive(DeviceState *dev, const char *name,
- BlockBackend *value, Error **errp);
+ BlockBackend *value);
void qdev_prop_set_macaddr(DeviceState *dev, const char *name,
const uint8_t *value);
void qdev_prop_set_enum(DeviceState *dev, const char *name, int value);
+void *qdev_get_prop_ptr(DeviceState *dev, Property *prop);
+
void qdev_prop_register_global(GlobalProperty *prop);
const GlobalProperty *qdev_find_global_prop(DeviceState *dev,
const char *name);
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 0ad08a2b4c..42d818b81c 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -227,8 +227,8 @@ static void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
fl->flash = qdev_new(flashtype);
if (dinfo) {
- qdev_prop_set_drive(fl->flash, "drive", blk_by_legacy_dinfo(dinfo),
- errp);
+ qdev_prop_set_drive_err(fl->flash, "drive",
+ blk_by_legacy_dinfo(dinfo), errp);
}
qdev_realize_and_unref(fl->flash, BUS(s->spi), &error_fatal);
@@ -243,8 +243,8 @@ static void sdhci_attach_drive(SDHCIState *sdhci, DriveInfo *dinfo)
card = qdev_new(TYPE_SD_CARD);
if (dinfo) {
- qdev_prop_set_drive(card, "drive", blk_by_legacy_dinfo(dinfo),
- &error_fatal);
+ qdev_prop_set_drive_err(card, "drive", blk_by_legacy_dinfo(dinfo),
+ &error_fatal);
}
qdev_realize_and_unref(card,
qdev_get_child_bus(DEVICE(sdhci), "sd-bus"),
diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c
index a96c860575..5cbd115c53 100644
--- a/hw/arm/cubieboard.c
+++ b/hw/arm/cubieboard.c
@@ -93,7 +93,7 @@ static void cubieboard_init(MachineState *machine)
/* Plug in SD card */
carddev = qdev_new(TYPE_SD_CARD);
- qdev_prop_set_drive(carddev, "drive", blk, &error_fatal);
+ qdev_prop_set_drive_err(carddev, "drive", blk, &error_fatal);
qdev_realize_and_unref(carddev, bus, &error_fatal);
memory_region_add_subregion(get_system_memory(), AW_A10_SDRAM_BASE,
diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index b888a5c9ab..fa639806ec 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -434,7 +434,7 @@ static void exynos4210_realize(DeviceState *socdev, Error **errp)
di = drive_get(IF_SD, 0, n);
blk = di ? blk_by_legacy_dinfo(di) : NULL;
carddev = qdev_new(TYPE_SD_CARD);
- qdev_prop_set_drive(carddev, "drive", blk, &error_abort);
+ qdev_prop_set_drive(carddev, "drive", blk);
qdev_realize_and_unref(carddev, qdev_get_child_bus(dev, "sd-bus"),
&error_fatal);
}
diff --git a/hw/arm/imx25_pdk.c b/hw/arm/imx25_pdk.c
index af8f7f969c..1c201d0d8e 100644
--- a/hw/arm/imx25_pdk.c
+++ b/hw/arm/imx25_pdk.c
@@ -130,7 +130,7 @@ static void imx25_pdk_init(MachineState *machine)
blk = di ? blk_by_legacy_dinfo(di) : NULL;
bus = qdev_get_child_bus(DEVICE(&s->soc.esdhc[i]), "sd-bus");
carddev = qdev_new(TYPE_SD_CARD);
- qdev_prop_set_drive(carddev, "drive", blk, &error_fatal);
+ qdev_prop_set_drive_err(carddev, "drive", blk, &error_fatal);
qdev_realize_and_unref(carddev, bus, &error_fatal);
}
diff --git a/hw/arm/mcimx6ul-evk.c b/hw/arm/mcimx6ul-evk.c
index 3d1d2e3c04..2f845cedfc 100644
--- a/hw/arm/mcimx6ul-evk.c
+++ b/hw/arm/mcimx6ul-evk.c
@@ -55,7 +55,7 @@ static void mcimx6ul_evk_init(MachineState *machine)
blk = di ? blk_by_legacy_dinfo(di) : NULL;
bus = qdev_get_child_bus(DEVICE(&s->usdhc[i]), "sd-bus");
carddev = qdev_new(TYPE_SD_CARD);
- qdev_prop_set_drive(carddev, "drive", blk, &error_fatal);
+ qdev_prop_set_drive_err(carddev, "drive", blk, &error_fatal);
qdev_realize_and_unref(carddev, bus, &error_fatal);
}
diff --git a/hw/arm/mcimx7d-sabre.c b/hw/arm/mcimx7d-sabre.c
index 365f8183bc..e57d52b344 100644
--- a/hw/arm/mcimx7d-sabre.c
+++ b/hw/arm/mcimx7d-sabre.c
@@ -57,7 +57,7 @@ static void mcimx7d_sabre_init(MachineState *machine)
blk = di ? blk_by_legacy_dinfo(di) : NULL;
bus = qdev_get_child_bus(DEVICE(&s->usdhc[i]), "sd-bus");
carddev = qdev_new(TYPE_SD_CARD);
- qdev_prop_set_drive(carddev, "drive", blk, &error_fatal);
+ qdev_prop_set_drive_err(carddev, "drive", blk, &error_fatal);
qdev_realize_and_unref(carddev, bus, &error_fatal);
}
diff --git a/hw/arm/msf2-som.c b/hw/arm/msf2-som.c
index 355966c073..f9b61c36dd 100644
--- a/hw/arm/msf2-som.c
+++ b/hw/arm/msf2-som.c
@@ -86,8 +86,8 @@ static void emcraft_sf2_s2s010_init(MachineState *machine)
spi_flash = qdev_new("s25sl12801");
qdev_prop_set_uint8(spi_flash, "spansion-cr2nv", 1);
if (dinfo) {
- qdev_prop_set_drive(spi_flash, "drive", blk_by_legacy_dinfo(dinfo),
- &error_fatal);
+ qdev_prop_set_drive_err(spi_flash, "drive",
+ blk_by_legacy_dinfo(dinfo), &error_fatal);
}
qdev_realize_and_unref(spi_flash, spi_bus, &error_fatal);
cs_line = qdev_get_gpio_in_named(spi_flash, SSI_GPIO_CS, 0);
diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c
index 02678dda2d..428a2a2c5a 100644
--- a/hw/arm/nseries.c
+++ b/hw/arm/nseries.c
@@ -182,8 +182,8 @@ static void n8x0_nand_setup(struct n800_s *s)
qdev_prop_set_int32(s->nand, "shift", 1);
dinfo = drive_get(IF_MTD, 0, 0);
if (dinfo) {
- qdev_prop_set_drive(s->nand, "drive", blk_by_legacy_dinfo(dinfo),
- &error_fatal);
+ qdev_prop_set_drive_err(s->nand, "drive", blk_by_legacy_dinfo(dinfo),
+ &error_fatal);
}
sysbus_realize_and_unref(SYS_BUS_DEVICE(s->nand), &error_fatal);
sysbus_connect_irq(SYS_BUS_DEVICE(s->nand), 0,
diff --git a/hw/arm/orangepi.c b/hw/arm/orangepi.c
index 678c93033e..843dcbbd62 100644
--- a/hw/arm/orangepi.c
+++ b/hw/arm/orangepi.c
@@ -95,7 +95,7 @@ static void orangepi_init(MachineState *machine)
/* Plug in SD card */
carddev = qdev_new(TYPE_SD_CARD);
- qdev_prop_set_drive(carddev, "drive", blk, &error_fatal);
+ qdev_prop_set_drive_err(carddev, "drive", blk, &error_fatal);
qdev_realize_and_unref(carddev, bus, &error_fatal);
/* SDRAM */
diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index 380978fc27..09bf02ec9c 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -298,7 +298,7 @@ static void raspi_machine_init(MachineState *machine)
exit(1);
}
carddev = qdev_new(TYPE_SD_CARD);
- qdev_prop_set_drive(carddev, "drive", blk, &error_fatal);
+ qdev_prop_set_drive_err(carddev, "drive", blk, &error_fatal);
qdev_realize_and_unref(carddev, bus, &error_fatal);
vcram_size = object_property_get_uint(OBJECT(&s->soc), "vcram-size",
diff --git a/hw/arm/sabrelite.c b/hw/arm/sabrelite.c
index a27e5baf60..91d8c43a7e 100644
--- a/hw/arm/sabrelite.c
+++ b/hw/arm/sabrelite.c
@@ -77,9 +77,9 @@ static void sabrelite_init(MachineState *machine)
flash_dev = qdev_new("sst25vf016b");
if (dinfo) {
- qdev_prop_set_drive(flash_dev, "drive",
- blk_by_legacy_dinfo(dinfo),
- &error_fatal);
+ qdev_prop_set_drive_err(flash_dev, "drive",
+ blk_by_legacy_dinfo(dinfo),
+ &error_fatal);
}
qdev_realize_and_unref(flash_dev, BUS(spi_bus), &error_fatal);
diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index 7ca5d523a4..725d024c91 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -517,8 +517,7 @@ static PFlashCFI01 *ve_pflash_cfi01_register(hwaddr base, const char *name,
DeviceState *dev = qdev_new(TYPE_PFLASH_CFI01);
if (di) {
- qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(di),
- &error_abort);
+ qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(di));
}
qdev_prop_set_uint32(dev, "num-blocks",
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 4247c4dbd8..ed970273f3 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -159,8 +159,9 @@ static inline void zynq_init_spi_flashes(uint32_t base_addr, qemu_irq irq,
DriveInfo *dinfo = drive_get_next(IF_MTD);
flash_dev = qdev_new("n25q128");
if (dinfo) {
- qdev_prop_set_drive(flash_dev, "drive",
- blk_by_legacy_dinfo(dinfo), &error_fatal);
+ qdev_prop_set_drive_err(flash_dev, "drive",
+ blk_by_legacy_dinfo(dinfo),
+ &error_fatal);
}
qdev_realize_and_unref(flash_dev, BUS(spi), &error_fatal);
@@ -290,7 +291,7 @@ static void zynq_init(MachineState *machine)
di = drive_get_next(IF_SD);
blk = di ? blk_by_legacy_dinfo(di) : NULL;
carddev = qdev_new(TYPE_SD_CARD);
- qdev_prop_set_drive(carddev, "drive", blk, &error_fatal);
+ qdev_prop_set_drive_err(carddev, "drive", blk, &error_fatal);
qdev_realize_and_unref(carddev, qdev_get_child_bus(dev, "sd-bus"),
&error_fatal);
}
diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
index 5bcca7f95b..a3b1ce9c7c 100644
--- a/hw/arm/xlnx-versal-virt.c
+++ b/hw/arm/xlnx-versal-virt.c
@@ -465,7 +465,7 @@ static void sd_plugin_card(SDHCIState *sd, DriveInfo *di)
card = qdev_new(TYPE_SD_CARD);
object_property_add_child(OBJECT(sd), "card[*]", OBJECT(card));
- qdev_prop_set_drive(card, "drive", blk, &error_fatal);
+ qdev_prop_set_drive_err(card, "drive", blk, &error_fatal);
qdev_realize_and_unref(card, qdev_get_child_bus(DEVICE(sd), "sd-bus"),
&error_fatal);
}
diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
index b920bcee94..77449759c6 100644
--- a/hw/arm/xlnx-zcu102.c
+++ b/hw/arm/xlnx-zcu102.c
@@ -143,7 +143,7 @@ static void xlnx_zcu102_init(MachineState *machine)
exit(1);
}
carddev = qdev_new(TYPE_SD_CARD);
- qdev_prop_set_drive(carddev, "drive", blk, &error_fatal);
+ qdev_prop_set_drive_err(carddev, "drive", blk, &error_fatal);
qdev_realize_and_unref(carddev, bus, &error_fatal);
}
@@ -159,8 +159,8 @@ static void xlnx_zcu102_init(MachineState *machine)
flash_dev = qdev_new("sst25wf080");
if (dinfo) {
- qdev_prop_set_drive(flash_dev, "drive", blk_by_legacy_dinfo(dinfo),
- &error_fatal);
+ qdev_prop_set_drive_err(flash_dev, "drive",
+ blk_by_legacy_dinfo(dinfo), &error_fatal);
}
qdev_realize_and_unref(flash_dev, spi_bus, &error_fatal);
@@ -182,8 +182,8 @@ static void xlnx_zcu102_init(MachineState *machine)
flash_dev = qdev_new("n25q512a11");
if (dinfo) {
- qdev_prop_set_drive(flash_dev, "drive", blk_by_legacy_dinfo(dinfo),
- &error_fatal);
+ qdev_prop_set_drive_err(flash_dev, "drive",
+ blk_by_legacy_dinfo(dinfo), &error_fatal);
}
qdev_realize_and_unref(flash_dev, spi_bus, &error_fatal);
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 7e143cbab0..f4493d6afa 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -2508,8 +2508,8 @@ static void fdctrl_init_drives(FloppyBus *bus, DriveInfo **fds)
dev = qdev_new("floppy");
qdev_prop_set_uint32(dev, "unit", i);
qdev_prop_set_enum(dev, "drive-type", FLOPPY_DRIVE_TYPE_AUTO);
- qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(fds[i]),
- &error_fatal);
+ qdev_prop_set_drive_err(dev, "drive", blk_by_legacy_dinfo(fds[i]),
+ &error_fatal);
qdev_realize_and_unref(dev, &bus->bus, &error_fatal);
}
}
@@ -2566,7 +2566,7 @@ static void fdctrl_connect_drives(FDCtrl *fdctrl, DeviceState *fdc_dev,
blk_ref(blk);
blk_detach_dev(blk, fdc_dev);
fdctrl->qdev_for_drives[i].blk = NULL;
- qdev_prop_set_drive(dev, "drive", blk, &local_err);
+ qdev_prop_set_drive_err(dev, "drive", blk, &local_err);
blk_unref(blk);
if (local_err) {
diff --git a/hw/block/nand.c b/hw/block/nand.c
index 7e25681d59..654e0cb5d1 100644
--- a/hw/block/nand.c
+++ b/hw/block/nand.c
@@ -648,7 +648,7 @@ DeviceState *nand_init(BlockBackend *blk, int manf_id, int chip_id)
qdev_prop_set_uint8(dev, "manufacturer_id", manf_id);
qdev_prop_set_uint8(dev, "chip_id", chip_id);
if (blk) {
- qdev_prop_set_drive(dev, "drive", blk, &error_fatal);
+ qdev_prop_set_drive_err(dev, "drive", blk, &error_fatal);
}
qdev_realize(dev, NULL, &error_fatal);
diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 9f0c1d61ca..cddc3a5a0c 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -962,7 +962,7 @@ PFlashCFI01 *pflash_cfi01_register(hwaddr base,
DeviceState *dev = qdev_new(TYPE_PFLASH_CFI01);
if (blk) {
- qdev_prop_set_drive(dev, "drive", blk, &error_abort);
+ qdev_prop_set_drive(dev, "drive", blk);
}
assert(QEMU_IS_ALIGNED(size, sector_len));
qdev_prop_set_uint32(dev, "num-blocks", size / sector_len);
@@ -1010,8 +1010,8 @@ void pflash_cfi01_legacy_drive(PFlashCFI01 *fl, DriveInfo *dinfo)
error_report("clashes with -machine");
exit(1);
}
- qdev_prop_set_drive(DEVICE(fl), "drive",
- blk_by_legacy_dinfo(dinfo), &error_fatal);
+ qdev_prop_set_drive_err(DEVICE(fl), "drive", blk_by_legacy_dinfo(dinfo),
+ &error_fatal);
loc_pop(&loc);
}
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 6eb66e7bb0..b40ce2335a 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -1001,7 +1001,7 @@ PFlashCFI02 *pflash_cfi02_register(hwaddr base,
DeviceState *dev = qdev_new(TYPE_PFLASH_CFI02);
if (blk) {
- qdev_prop_set_drive(dev, "drive", blk, &error_abort);
+ qdev_prop_set_drive(dev, "drive", blk);
}
assert(QEMU_IS_ALIGNED(size, sector_len));
qdev_prop_set_uint32(dev, "num-blocks", size / sector_len);
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 2561fa09a8..38b0c9f09b 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -425,8 +425,8 @@ const PropertyInfo qdev_prop_audiodev = {
.set = set_audiodev,
};
-void qdev_prop_set_drive(DeviceState *dev, const char *name,
- BlockBackend *value, Error **errp)
+void qdev_prop_set_drive_err(DeviceState *dev, const char *name,
+ BlockBackend *value, Error **errp)
{
const char *ref = "";
@@ -443,6 +443,12 @@ void qdev_prop_set_drive(DeviceState *dev, const char *name,
object_property_set_str(OBJECT(dev), ref, name, errp);
}
+void qdev_prop_set_drive(DeviceState *dev, const char *name,
+ BlockBackend *value)
+{
+ qdev_prop_set_drive_err(dev, name, value, &error_abort);
+}
+
void qdev_prop_set_chr(DeviceState *dev, const char *name,
Chardev *value)
{
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 3ccb5e2529..f68fbee93d 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -129,8 +129,8 @@ IDEDevice *ide_create_drive(IDEBus *bus, int unit, DriveInfo *drive)
dev = qdev_new(drive->media_cd ? "ide-cd" : "ide-hd");
qdev_prop_set_uint32(dev, "unit", unit);
- qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(drive),
- &error_fatal);
+ qdev_prop_set_drive_err(dev, "drive", blk_by_legacy_dinfo(drive),
+ &error_fatal);
qdev_realize_and_unref(dev, &bus->qbus, &error_fatal);
return DO_UPCAST(IDEDevice, qdev, dev);
}
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 503ec54f5d..459d326af0 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -221,8 +221,7 @@ static void q800_init(MachineState *machine)
via_dev = qdev_new(TYPE_MAC_VIA);
dinfo = drive_get(IF_MTD, 0, 0);
if (dinfo) {
- qdev_prop_set_drive(via_dev, "drive", blk_by_legacy_dinfo(dinfo),
- &error_abort);
+ qdev_prop_set_drive(via_dev, "drive", blk_by_legacy_dinfo(dinfo));
}
sysbus = SYS_BUS_DEVICE(via_dev);
sysbus_realize_and_unref(sysbus, &error_fatal);
diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c
index 23420028f5..fff2c578ef 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -188,8 +188,9 @@ petalogix_ml605_init(MachineState *machine)
dev = qdev_new("n25q128");
if (dinfo) {
- qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo),
- &error_fatal);
+ qdev_prop_set_drive_err(dev, "drive",
+ blk_by_legacy_dinfo(dinfo),
+ &error_fatal);
}
qdev_realize_and_unref(dev, BUS(spi), &error_fatal);
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 80b4afd211..8bd03f3b10 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -730,8 +730,7 @@ static void pnv_init(MachineState *machine)
*/
dev = qdev_new(TYPE_PNV_PNOR);
if (pnor) {
- qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(pnor),
- &error_abort);
+ qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(pnor));
}
sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
pnv->pnor = PNV_PNOR(dev);
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 8d630baa5d..bd9345cdac 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1716,8 +1716,8 @@ static void spapr_create_nvram(SpaprMachineState *spapr)
DriveInfo *dinfo = drive_get(IF_PFLASH, 0, 0);
if (dinfo) {
- qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo),
- &error_fatal);
+ qdev_prop_set_drive_err(dev, "drive", blk_by_legacy_dinfo(dinfo),
+ &error_fatal);
}
qdev_realize_and_unref(dev, &spapr->vio_bus->bus, &error_fatal);
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 1a7320c0af..27843bb04b 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -277,7 +277,7 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockBackend *blk,
if (serial && object_property_find(OBJECT(dev), "serial", NULL)) {
qdev_prop_set_string(dev, "serial", serial);
}
- qdev_prop_set_drive(dev, "drive", blk, &err);
+ qdev_prop_set_drive_err(dev, "drive", blk, &err);
if (err) {
error_propagate(errp, err);
object_unparent(OBJECT(dev));
diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
index 4cfdf7b64c..1c23310715 100644
--- a/hw/sd/milkymist-memcard.c
+++ b/hw/sd/milkymist-memcard.c
@@ -279,7 +279,7 @@ static void milkymist_memcard_realize(DeviceState *dev, Error **errp)
dinfo = drive_get_next(IF_SD);
blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL;
carddev = qdev_new(TYPE_SD_CARD);
- qdev_prop_set_drive(carddev, "drive", blk, &err);
+ qdev_prop_set_drive_err(carddev, "drive", blk, &err);
qdev_realize_and_unref(carddev, BUS(&s->sdbus), &err);
if (err) {
error_setg(errp, "failed to init SD card: %s", error_get_pretty(err));
diff --git a/hw/sd/pxa2xx_mmci.c b/hw/sd/pxa2xx_mmci.c
index 623be70b26..3407617afc 100644
--- a/hw/sd/pxa2xx_mmci.c
+++ b/hw/sd/pxa2xx_mmci.c
@@ -496,7 +496,7 @@ PXA2xxMMCIState *pxa2xx_mmci_init(MemoryRegion *sysmem,
/* Create and plug in the sd card */
carddev = qdev_new(TYPE_SD_CARD);
- qdev_prop_set_drive(carddev, "drive", blk, &err);
+ qdev_prop_set_drive_err(carddev, "drive", blk, &err);
if (err) {
error_reportf_err(err, "failed to init SD card: ");
return NULL;
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 7070a116ea..97a9d32964 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -706,7 +706,7 @@ SDState *sd_init(BlockBackend *blk, bool is_spi)
obj = object_new(TYPE_SD_CARD);
dev = DEVICE(obj);
- qdev_prop_set_drive(dev, "drive", blk, &err);
+ qdev_prop_set_drive_err(dev, "drive", blk, &err);
if (err) {
error_reportf_err(err, "sd_init failed: ");
return NULL;
diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index f98a6f3ae1..25cec2ddea 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -254,7 +254,8 @@ static void ssi_sd_realize(SSISlave *d, Error **errp)
dinfo = drive_get_next(IF_SD);
carddev = qdev_new(TYPE_SD_CARD);
if (dinfo) {
- qdev_prop_set_drive(carddev, "drive", blk_by_legacy_dinfo(dinfo), &err);
+ qdev_prop_set_drive_err(carddev, "drive", blk_by_legacy_dinfo(dinfo),
+ &err);
if (err) {
goto fail;
}
diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
index 5d0834c1d9..10de15855a 100644
--- a/hw/xtensa/xtfpga.c
+++ b/hw/xtensa/xtfpga.c
@@ -173,8 +173,7 @@ static PFlashCFI01 *xtfpga_flash_init(MemoryRegion *address_space,
SysBusDevice *s;
DeviceState *dev = qdev_new(TYPE_PFLASH_CFI01);
- qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo),
- &error_abort);
+ qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
qdev_prop_set_uint32(dev, "num-blocks",
board->flash->size / board->flash->sector_size);
qdev_prop_set_uint64(dev, "sector-length", board->flash->sector_size);
--
2.26.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 14/16] arm/aspeed: Drop aspeed_board_init_flashes() parameter @errp
2020-06-22 9:42 [PATCH v2 00/16] Crazy shit around -global (pardon my french) Markus Armbruster
` (12 preceding siblings ...)
2020-06-22 9:42 ` [PATCH v2 13/16] qdev: Make qdev_prop_set_drive() match the other helpers Markus Armbruster
@ 2020-06-22 9:42 ` Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 15/16] sd/pxa2xx_mmci: Don't crash on pxa2xx_mmci_init() error Markus Armbruster
` (3 subsequent siblings)
17 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2020-06-22 9:42 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, Peter Maydell, berrange, ehabkost, qemu-block,
Andrew Jeffery, Philippe Mathieu-Daudé,
mreitz, qemu-arm, Cédric Le Goater, pbonzini, jsnow,
Joel Stanley
We always pass &error_abort. Drop the parameter, use &error_abort
directly.
Cc: Cédric Le Goater <clg@kaod.org>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Andrew Jeffery <andrew@aj.id.au>
Cc: Joel Stanley <joel@jms.id.au>
Cc: qemu-arm@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/arm/aspeed.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 42d818b81c..863757e1f0 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -215,8 +215,8 @@ static void write_boot_rom(DriveInfo *dinfo, hwaddr addr, size_t rom_size,
g_free(storage);
}
-static void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
- Error **errp)
+static void aspeed_board_init_flashes(AspeedSMCState *s,
+ const char *flashtype)
{
int i ;
@@ -227,8 +227,8 @@ static void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
fl->flash = qdev_new(flashtype);
if (dinfo) {
- qdev_prop_set_drive_err(fl->flash, "drive",
- blk_by_legacy_dinfo(dinfo), errp);
+ qdev_prop_set_drive(fl->flash, "drive",
+ blk_by_legacy_dinfo(dinfo));
}
qdev_realize_and_unref(fl->flash, BUS(s->spi), &error_fatal);
@@ -314,8 +314,8 @@ static void aspeed_machine_init(MachineState *machine)
"max_ram", max_ram_size - ram_size);
memory_region_add_subregion(&bmc->ram_container, ram_size, &bmc->max_ram);
- aspeed_board_init_flashes(&bmc->soc.fmc, amc->fmc_model, &error_abort);
- aspeed_board_init_flashes(&bmc->soc.spi[0], amc->spi_model, &error_abort);
+ aspeed_board_init_flashes(&bmc->soc.fmc, amc->fmc_model);
+ aspeed_board_init_flashes(&bmc->soc.spi[0], amc->spi_model);
/* Install first FMC flash content as a boot rom. */
if (drive0) {
--
2.26.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 15/16] sd/pxa2xx_mmci: Don't crash on pxa2xx_mmci_init() error
2020-06-22 9:42 [PATCH v2 00/16] Crazy shit around -global (pardon my french) Markus Armbruster
` (13 preceding siblings ...)
2020-06-22 9:42 ` [PATCH v2 14/16] arm/aspeed: Drop aspeed_board_init_flashes() parameter @errp Markus Armbruster
@ 2020-06-22 9:42 ` Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 16/16] sd/milkymist-memcard: Fix error API violation Markus Armbruster
` (2 subsequent siblings)
17 siblings, 0 replies; 24+ messages in thread
From: Markus Armbruster @ 2020-06-22 9:42 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, Peter Maydell, berrange, ehabkost, qemu-block,
Philippe Mathieu-Daudé,
mreitz, qemu-arm, pbonzini, jsnow
On error, pxa2xx_mmci_init() reports to stderr and returns NULL.
Callers don't check for errors. Machines akita, borzoi, mainstone,
spitz, terrier, tosa, and z2 crash shortly after, like this:
$ qemu-system-aarch64 -M akita -drive if=sd,readonly=on
qemu-system-aarch64: failed to init SD card: Cannot use read-only drive as SD card
Segmentation fault (core dumped)
Machines connex and verdex reach the check for orphaned drives first:
$ aarch64-softmmu/qemu-system-aarch64 -M connex -drive if=sd,readonly=on -accel qtest
qemu-system-aarch64: failed to init SD card: Cannot use read-only drive as SD card
qemu-system-aarch64: -drive if=sd,readonly=on: machine type does not support if=sd,bus=0,unit=0
Make pxa2xx_mmci_init() fail cleanly right away.
Cc: Andrzej Zaborowski <balrogg@gmail.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/sd/pxa2xx_mmci.c | 15 +++------------
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/hw/sd/pxa2xx_mmci.c b/hw/sd/pxa2xx_mmci.c
index 3407617afc..68bed24480 100644
--- a/hw/sd/pxa2xx_mmci.c
+++ b/hw/sd/pxa2xx_mmci.c
@@ -18,7 +18,6 @@
#include "hw/arm/pxa.h"
#include "hw/sd/sd.h"
#include "hw/qdev-properties.h"
-#include "qemu/error-report.h"
#include "qemu/log.h"
#include "qemu/module.h"
#include "trace.h"
@@ -483,7 +482,6 @@ PXA2xxMMCIState *pxa2xx_mmci_init(MemoryRegion *sysmem,
DeviceState *dev, *carddev;
SysBusDevice *sbd;
PXA2xxMMCIState *s;
- Error *err = NULL;
dev = qdev_new(TYPE_PXA2XX_MMCI);
s = PXA2XX_MMCI(dev);
@@ -496,16 +494,9 @@ PXA2xxMMCIState *pxa2xx_mmci_init(MemoryRegion *sysmem,
/* Create and plug in the sd card */
carddev = qdev_new(TYPE_SD_CARD);
- qdev_prop_set_drive_err(carddev, "drive", blk, &err);
- if (err) {
- error_reportf_err(err, "failed to init SD card: ");
- return NULL;
- }
- qdev_realize_and_unref(carddev, qdev_get_child_bus(dev, "sd-bus"), &err);
- if (err) {
- error_reportf_err(err, "failed to init SD card: ");
- return NULL;
- }
+ qdev_prop_set_drive_err(carddev, "drive", blk, &error_fatal);
+ qdev_realize_and_unref(carddev, qdev_get_child_bus(dev, "sd-bus"),
+ &error_fatal);
return s;
}
--
2.26.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 16/16] sd/milkymist-memcard: Fix error API violation
2020-06-22 9:42 [PATCH v2 00/16] Crazy shit around -global (pardon my french) Markus Armbruster
` (14 preceding siblings ...)
2020-06-22 9:42 ` [PATCH v2 15/16] sd/pxa2xx_mmci: Don't crash on pxa2xx_mmci_init() error Markus Armbruster
@ 2020-06-22 9:42 ` Markus Armbruster
2020-06-22 9:56 ` Philippe Mathieu-Daudé
2020-06-22 10:07 ` [PATCH v2 00/16] Crazy shit around -global (pardon my french) no-reply
2020-06-24 15:40 ` John Snow
17 siblings, 1 reply; 24+ messages in thread
From: Markus Armbruster @ 2020-06-22 9:42 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, berrange, ehabkost, qemu-block, mreitz, Michael Walle,
pbonzini, jsnow
The Error ** argument must be NULL, &error_abort, &error_fatal, or a
pointer to a variable containing NULL. Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
call.
milkymist_memcard_realize() is wrong that way: it passes &err to
qdev_prop_set_drive_err() and qdev_realize_and_unref(). Currently
harmless, because the latter uses it only as first argument of
error_propagate().
Making qdev_prop_set_drive_err() fail involves abuse of -global.
Leave handling that to qdev_prop_set_drive(), like we do elsewhere.
Cc: Michael Walle <michael@walle.cc>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/sd/milkymist-memcard.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
index 1c23310715..482e97191e 100644
--- a/hw/sd/milkymist-memcard.c
+++ b/hw/sd/milkymist-memcard.c
@@ -279,7 +279,7 @@ static void milkymist_memcard_realize(DeviceState *dev, Error **errp)
dinfo = drive_get_next(IF_SD);
blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL;
carddev = qdev_new(TYPE_SD_CARD);
- qdev_prop_set_drive_err(carddev, "drive", blk, &err);
+ qdev_prop_set_drive(carddev, "drive", blk);
qdev_realize_and_unref(carddev, BUS(&s->sdbus), &err);
if (err) {
error_setg(errp, "failed to init SD card: %s", error_get_pretty(err));
--
2.26.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v2 16/16] sd/milkymist-memcard: Fix error API violation
2020-06-22 9:42 ` [PATCH v2 16/16] sd/milkymist-memcard: Fix error API violation Markus Armbruster
@ 2020-06-22 9:56 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 24+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-22 9:56 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel
Cc: kwolf, berrange, ehabkost, qemu-block, mreitz, Michael Walle,
pbonzini, jsnow
On 6/22/20 11:42 AM, Markus Armbruster wrote:
> The Error ** argument must be NULL, &error_abort, &error_fatal, or a
> pointer to a variable containing NULL. Passing an argument of the
> latter kind twice without clearing it in between is wrong: if the
> first call sets an error, it no longer points to NULL for the second
> call.
>
> milkymist_memcard_realize() is wrong that way: it passes &err to
> qdev_prop_set_drive_err() and qdev_realize_and_unref(). Currently
> harmless, because the latter uses it only as first argument of
> error_propagate().
>
> Making qdev_prop_set_drive_err() fail involves abuse of -global.
> Leave handling that to qdev_prop_set_drive(), like we do elsewhere.
>
> Cc: Michael Walle <michael@walle.cc>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/sd/milkymist-memcard.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
> index 1c23310715..482e97191e 100644
> --- a/hw/sd/milkymist-memcard.c
> +++ b/hw/sd/milkymist-memcard.c
> @@ -279,7 +279,7 @@ static void milkymist_memcard_realize(DeviceState *dev, Error **errp)
> dinfo = drive_get_next(IF_SD);
> blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL;
> carddev = qdev_new(TYPE_SD_CARD);
> - qdev_prop_set_drive_err(carddev, "drive", blk, &err);
> + qdev_prop_set_drive(carddev, "drive", blk);
> qdev_realize_and_unref(carddev, BUS(&s->sdbus), &err);
> if (err) {
> error_setg(errp, "failed to init SD card: %s", error_get_pretty(err));
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 00/16] Crazy shit around -global (pardon my french)
2020-06-22 9:42 [PATCH v2 00/16] Crazy shit around -global (pardon my french) Markus Armbruster
` (15 preceding siblings ...)
2020-06-22 9:42 ` [PATCH v2 16/16] sd/milkymist-memcard: Fix error API violation Markus Armbruster
@ 2020-06-22 10:07 ` no-reply
2020-06-24 15:40 ` John Snow
17 siblings, 0 replies; 24+ messages in thread
From: no-reply @ 2020-06-22 10:07 UTC (permalink / raw)
To: armbru
Cc: kwolf, berrange, ehabkost, qemu-block, qemu-devel, mreitz,
pbonzini, jsnow
Patchew URL: https://patchew.org/QEMU/20200622094227.1271650-1-armbru@redhat.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [PATCH v2 00/16] Crazy shit around -global (pardon my french)
Type: series
Message-id: 20200622094227.1271650-1-armbru@redhat.com
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
From https://github.com/patchew-project/qemu
* [new tag] patchew/20200622094227.1271650-1-armbru@redhat.com -> patchew/20200622094227.1271650-1-armbru@redhat.com
Switched to a new branch 'test'
f0116ee sd/milkymist-memcard: Fix error API violation
3ce05b4 sd/pxa2xx_mmci: Don't crash on pxa2xx_mmci_init() error
7bb382e arm/aspeed: Drop aspeed_board_init_flashes() parameter @errp
7fc39f8 qdev: Make qdev_prop_set_drive() match the other helpers
a1bad90 qdev: Reject chardev property override
b73765e qdev: Reject drive property override
34072ae qdev: Improve netdev property override error a bit
fa6bc3e qdev: Eliminate get_pointer(), set_pointer()
2e22ad2 blockdev: Deprecate -drive with bogus interface type
2fdd4f4 docs/qdev-device-use.txt: Update section "Default Devices"
3bb6131 fdc: Deprecate configuring floppies with -global isa-fdc
1442ef5 fdc: Open-code fdctrl_init_isa()
6ab9dc2 fdc: Reject clash between -drive if=floppy and -global isa-fdc
00e64a2 iotests/172: Cover -global floppy.drive=...
40c2844 iotests/172: Cover empty filename and multiple use of drives
ba868ba iotests/172: Include "info block" in test output
=== OUTPUT BEGIN ===
1/16 Checking commit ba868baf44f7 (iotests/172: Include "info block" in test output)
2/16 Checking commit 40c28442f546 (iotests/172: Cover empty filename and multiple use of drives)
ERROR: trailing whitespace
#48: FILE: tests/qemu-iotests/172.out:190:
+Testing: -fdb $
total: 1 errors, 0 warnings, 86 lines checked
Patch 2/16 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/16 Checking commit 00e64a2b1a99 (iotests/172: Cover -global floppy.drive=...)
4/16 Checking commit 6ab9dc296d08 (fdc: Reject clash between -drive if=floppy and -global isa-fdc)
5/16 Checking commit 1442ef5c1dad (fdc: Open-code fdctrl_init_isa())
6/16 Checking commit 3bb61318d607 (fdc: Deprecate configuring floppies with -global isa-fdc)
7/16 Checking commit 2fdd4f45b355 (docs/qdev-device-use.txt: Update section "Default Devices")
8/16 Checking commit 2e22ad2cf40d (blockdev: Deprecate -drive with bogus interface type)
9/16 Checking commit fa6bc3ea041f (qdev: Eliminate get_pointer(), set_pointer())
10/16 Checking commit 34072ae828ca (qdev: Improve netdev property override error a bit)
11/16 Checking commit b73765e2d86c (qdev: Reject drive property override)
12/16 Checking commit a1bad90fffeb (qdev: Reject chardev property override)
13/16 Checking commit 7fc39f8555b1 (qdev: Make qdev_prop_set_drive() match the other helpers)
14/16 Checking commit 7bb382e31bf4 (arm/aspeed: Drop aspeed_board_init_flashes() parameter @errp)
15/16 Checking commit 3ce05b4e108e (sd/pxa2xx_mmci: Don't crash on pxa2xx_mmci_init() error)
16/16 Checking commit f0116ee141ee (sd/milkymist-memcard: Fix error API violation)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20200622094227.1271650-1-armbru@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 00/16] Crazy shit around -global (pardon my french)
2020-06-22 9:42 [PATCH v2 00/16] Crazy shit around -global (pardon my french) Markus Armbruster
` (16 preceding siblings ...)
2020-06-22 10:07 ` [PATCH v2 00/16] Crazy shit around -global (pardon my french) no-reply
@ 2020-06-24 15:40 ` John Snow
2020-06-25 4:45 ` Markus Armbruster
17 siblings, 1 reply; 24+ messages in thread
From: John Snow @ 2020-06-24 15:40 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel
Cc: kwolf, berrange, ehabkost, qemu-block, mreitz, pbonzini
On 6/22/20 5:42 AM, Markus Armbruster wrote:
> There are three ways to configure backends:
>
> * -nic, -serial, -drive, ... (onboard devices)
>
> * Set the property with -device, or, if you feel masochistic, with
> -set device (pluggable devices)
>
> * Set the property with -global (both)
>
> The trouble is -global is terrible.
>
> It gets applied in object_new(), which can't fail. We treat failure
> to apply -global as fatal error, except when hot-plugging, where we
> treat it as warning *boggle*. I'm not addressing that today.
>
> Some code falls apart when you use both -global and the other way.
>
> To make life more interesting, we gave -drive two roles: with
> interface type other than none, it's for configuring onboard devices,
> and with interface type none, it's for defining backends for use with
> -device and such. Since we neglect to require interface type none for
> the latter, you can use one -drive in both roles. This confuses the
> code about as much as you, dear reader, probably are by now.
>
> Because this still isn't interesting enough, there's yet another way
> to configure backends, just for floppies: set the floppy controller's
> property. Goes back to the time when floppy wasn't a separate device,
> and involves some Bad Magic. Now -global can interact with itself!
>
> Digging through all this took me an embarrassing amount of time.
> Hair, too.
>
> My patches reject some the silliest uses outright, and deprecate some
> not so silly ones that have replacements.
>
> Apply on top of my "[PATCH v2 00/58] qdev: Rework how we plug into the
> parent bus".
>
Oof. Thank you for your work in fixing our darkest corners. I sincerely
appreciate it.
The qdev tree ordering problems don't cause any issues for migration, do
they?
(I see you already sent a PR, so whatever!)
> Enjoy!
>
> v2:
> * Rebased; tests/qemu-iotests/172.out regenerated to resolve conflicts
> * PATCH 10-12: check_non_null() renamed to check_prop_still_unset()
> [Philippe]
>
> Markus Armbruster (16):
> iotests/172: Include "info block" in test output
> iotests/172: Cover empty filename and multiple use of drives
> iotests/172: Cover -global floppy.drive=...
> fdc: Reject clash between -drive if=floppy and -global isa-fdc
> fdc: Open-code fdctrl_init_isa()
> fdc: Deprecate configuring floppies with -global isa-fdc
> docs/qdev-device-use.txt: Update section "Default Devices"
> blockdev: Deprecate -drive with bogus interface type
> qdev: Eliminate get_pointer(), set_pointer()
> qdev: Improve netdev property override error a bit
> qdev: Reject drive property override
> qdev: Reject chardev property override
> qdev: Make qdev_prop_set_drive() match the other helpers
> arm/aspeed: Drop aspeed_board_init_flashes() parameter @errp
> sd/pxa2xx_mmci: Don't crash on pxa2xx_mmci_init() error
> sd/milkymist-memcard: Fix error API violation
>
> docs/qdev-device-use.txt | 17 +-
> docs/system/deprecated.rst | 34 ++
> include/hw/block/fdc.h | 2 +-
> include/hw/qdev-properties.h | 18 +-
> include/sysemu/blockdev.h | 2 +
> blockdev.c | 27 +-
> hw/arm/aspeed.c | 16 +-
> hw/arm/cubieboard.c | 2 +-
> hw/arm/exynos4210.c | 2 +-
> hw/arm/imx25_pdk.c | 2 +-
> hw/arm/mcimx6ul-evk.c | 2 +-
> hw/arm/mcimx7d-sabre.c | 2 +-
> hw/arm/msf2-som.c | 4 +-
> hw/arm/nseries.c | 4 +-
> hw/arm/orangepi.c | 2 +-
> hw/arm/raspi.c | 2 +-
> hw/arm/sabrelite.c | 6 +-
> hw/arm/vexpress.c | 3 +-
> hw/arm/xilinx_zynq.c | 7 +-
> hw/arm/xlnx-versal-virt.c | 2 +-
> hw/arm/xlnx-zcu102.c | 10 +-
> hw/block/fdc.c | 82 ++--
> hw/block/nand.c | 2 +-
> hw/block/pflash_cfi01.c | 6 +-
> hw/block/pflash_cfi02.c | 2 +-
> hw/core/qdev-properties-system.c | 151 ++++---
> hw/core/qdev-properties.c | 17 +
> hw/i386/pc.c | 8 +-
> hw/ide/qdev.c | 4 +-
> hw/isa/isa-superio.c | 18 +-
> hw/m68k/q800.c | 3 +-
> hw/microblaze/petalogix_ml605_mmu.c | 5 +-
> hw/ppc/pnv.c | 3 +-
> hw/ppc/spapr.c | 4 +-
> hw/scsi/scsi-bus.c | 2 +-
> hw/sd/milkymist-memcard.c | 2 +-
> hw/sd/pxa2xx_mmci.c | 15 +-
> hw/sd/sd.c | 2 +-
> hw/sd/ssi-sd.c | 3 +-
> hw/sparc64/sun4u.c | 9 +-
> hw/xtensa/xtfpga.c | 3 +-
> softmmu/vl.c | 8 +
> tests/qemu-iotests/172 | 27 +-
> tests/qemu-iotests/172.out | 656 +++++++++++++++++++++++++---
> 44 files changed, 928 insertions(+), 270 deletions(-)
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 00/16] Crazy shit around -global (pardon my french)
2020-06-24 15:40 ` John Snow
@ 2020-06-25 4:45 ` Markus Armbruster
2020-06-26 15:11 ` John Snow
0 siblings, 1 reply; 24+ messages in thread
From: Markus Armbruster @ 2020-06-25 4:45 UTC (permalink / raw)
To: John Snow
Cc: kwolf, berrange, ehabkost, qemu-block, qemu-devel, mreitz, pbonzini
John Snow <jsnow@redhat.com> writes:
> On 6/22/20 5:42 AM, Markus Armbruster wrote:
>> There are three ways to configure backends:
>>
>> * -nic, -serial, -drive, ... (onboard devices)
>>
>> * Set the property with -device, or, if you feel masochistic, with
>> -set device (pluggable devices)
>>
>> * Set the property with -global (both)
>>
>> The trouble is -global is terrible.
>>
>> It gets applied in object_new(), which can't fail. We treat failure
>> to apply -global as fatal error, except when hot-plugging, where we
>> treat it as warning *boggle*. I'm not addressing that today.
>>
>> Some code falls apart when you use both -global and the other way.
>>
>> To make life more interesting, we gave -drive two roles: with
>> interface type other than none, it's for configuring onboard devices,
>> and with interface type none, it's for defining backends for use with
>> -device and such. Since we neglect to require interface type none for
>> the latter, you can use one -drive in both roles. This confuses the
>> code about as much as you, dear reader, probably are by now.
>>
>> Because this still isn't interesting enough, there's yet another way
>> to configure backends, just for floppies: set the floppy controller's
>> property. Goes back to the time when floppy wasn't a separate device,
>> and involves some Bad Magic. Now -global can interact with itself!
>>
>> Digging through all this took me an embarrassing amount of time.
>> Hair, too.
>>
>> My patches reject some the silliest uses outright, and deprecate some
>> not so silly ones that have replacements.
>>
>> Apply on top of my "[PATCH v2 00/58] qdev: Rework how we plug into the
>> parent bus".
>>
>
> Oof. Thank you for your work in fixing our darkest corners. I sincerely
> appreciate it.
>
> The qdev tree ordering problems don't cause any issues for migration, do
> they?
This series should only change device configuration, not device state or
its encoding in the migration stream.
I'm not sure what you mean by "qdev tree ordering problems". Ist it
commit e8c9e65816 'qom: Make "info qom-tree" show children sorted'?
> (I see you already sent a PR, so whatever!)
A question that might avoid a later migration debugging session is
*never* "whatever"!
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 00/16] Crazy shit around -global (pardon my french)
2020-06-25 4:45 ` Markus Armbruster
@ 2020-06-26 15:11 ` John Snow
2020-06-27 12:22 ` Markus Armbruster
0 siblings, 1 reply; 24+ messages in thread
From: John Snow @ 2020-06-26 15:11 UTC (permalink / raw)
To: Markus Armbruster
Cc: kwolf, berrange, ehabkost, qemu-block, qemu-devel, mreitz, pbonzini
On 6/25/20 12:45 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
>
>> On 6/22/20 5:42 AM, Markus Armbruster wrote:
>>> There are three ways to configure backends:
>>>
>>> * -nic, -serial, -drive, ... (onboard devices)
>>>
>>> * Set the property with -device, or, if you feel masochistic, with
>>> -set device (pluggable devices)
>>>
>>> * Set the property with -global (both)
>>>
>>> The trouble is -global is terrible.
>>>
>>> It gets applied in object_new(), which can't fail. We treat failure
>>> to apply -global as fatal error, except when hot-plugging, where we
>>> treat it as warning *boggle*. I'm not addressing that today.
>>>
>>> Some code falls apart when you use both -global and the other way.
>>>
>>> To make life more interesting, we gave -drive two roles: with
>>> interface type other than none, it's for configuring onboard devices,
>>> and with interface type none, it's for defining backends for use with
>>> -device and such. Since we neglect to require interface type none for
>>> the latter, you can use one -drive in both roles. This confuses the
>>> code about as much as you, dear reader, probably are by now.
>>>
>>> Because this still isn't interesting enough, there's yet another way
>>> to configure backends, just for floppies: set the floppy controller's
>>> property. Goes back to the time when floppy wasn't a separate device,
>>> and involves some Bad Magic. Now -global can interact with itself!
>>>
>>> Digging through all this took me an embarrassing amount of time.
>>> Hair, too.
>>>
>>> My patches reject some the silliest uses outright, and deprecate some
>>> not so silly ones that have replacements.
>>>
>>> Apply on top of my "[PATCH v2 00/58] qdev: Rework how we plug into the
>>> parent bus".
>>>
>>
>> Oof. Thank you for your work in fixing our darkest corners. I sincerely
>> appreciate it.
>>
>> The qdev tree ordering problems don't cause any issues for migration, do
>> they?
>
> This series should only change device configuration, not device state or
> its encoding in the migration stream.
>
> I'm not sure what you mean by "qdev tree ordering problems". Ist it
> commit e8c9e65816 'qom: Make "info qom-tree" show children sorted'?
>
>> (I see you already sent a PR, so whatever!)
>
> A question that might avoid a later migration debugging session is
> *never* "whatever"!
>
I thought I had read that one of these patches changes the order in
which devices get instantiated, which I thought might change their QOM
paths. Which I thought *might* have some ramifications for migration,
but wasn't sure.
If it's just showing the same path outputs *sorted*, then there's no
problem.
Likely misread.
--js
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 00/16] Crazy shit around -global (pardon my french)
2020-06-26 15:11 ` John Snow
@ 2020-06-27 12:22 ` Markus Armbruster
2020-06-29 8:39 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 24+ messages in thread
From: Markus Armbruster @ 2020-06-27 12:22 UTC (permalink / raw)
To: John Snow
Cc: kwolf, berrange, ehabkost, qemu-block, qemu-devel,
Markus Armbruster, pbonzini, mreitz, David Alan Gilbert
Cc: David for insurance against me spewing nonsense about migration.
John Snow <jsnow@redhat.com> writes:
> On 6/25/20 12:45 AM, Markus Armbruster wrote:
>> John Snow <jsnow@redhat.com> writes:
>>
>>> On 6/22/20 5:42 AM, Markus Armbruster wrote:
>>>> There are three ways to configure backends:
>>>>
>>>> * -nic, -serial, -drive, ... (onboard devices)
>>>>
>>>> * Set the property with -device, or, if you feel masochistic, with
>>>> -set device (pluggable devices)
>>>>
>>>> * Set the property with -global (both)
>>>>
>>>> The trouble is -global is terrible.
>>>>
>>>> It gets applied in object_new(), which can't fail. We treat failure
>>>> to apply -global as fatal error, except when hot-plugging, where we
>>>> treat it as warning *boggle*. I'm not addressing that today.
>>>>
>>>> Some code falls apart when you use both -global and the other way.
>>>>
>>>> To make life more interesting, we gave -drive two roles: with
>>>> interface type other than none, it's for configuring onboard devices,
>>>> and with interface type none, it's for defining backends for use with
>>>> -device and such. Since we neglect to require interface type none for
>>>> the latter, you can use one -drive in both roles. This confuses the
>>>> code about as much as you, dear reader, probably are by now.
>>>>
>>>> Because this still isn't interesting enough, there's yet another way
>>>> to configure backends, just for floppies: set the floppy controller's
>>>> property. Goes back to the time when floppy wasn't a separate device,
>>>> and involves some Bad Magic. Now -global can interact with itself!
>>>>
>>>> Digging through all this took me an embarrassing amount of time.
>>>> Hair, too.
>>>>
>>>> My patches reject some the silliest uses outright, and deprecate some
>>>> not so silly ones that have replacements.
>>>>
>>>> Apply on top of my "[PATCH v2 00/58] qdev: Rework how we plug into the
>>>> parent bus".
>>>>
>>>
>>> Oof. Thank you for your work in fixing our darkest corners. I sincerely
>>> appreciate it.
>>>
>>> The qdev tree ordering problems don't cause any issues for migration, do
>>> they?
>>
>> This series should only change device configuration, not device state or
>> its encoding in the migration stream.
>>
>> I'm not sure what you mean by "qdev tree ordering problems". Ist it
>> commit e8c9e65816 'qom: Make "info qom-tree" show children sorted'?
>>
>>> (I see you already sent a PR, so whatever!)
>>
>> A question that might avoid a later migration debugging session is
>> *never* "whatever"!
>>
>
> I thought I had read that one of these patches changes the order in
> which devices get instantiated, which I thought might change their QOM
> paths. Which I thought *might* have some ramifications for migration,
> but wasn't sure.
Device instantiation order changes should not break migration.
The order in which devices appear in the migration stream should not
matter.
> If it's just showing the same path outputs *sorted*, then there's no
> problem.
>
> Likely misread.
>
> --js
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 00/16] Crazy shit around -global (pardon my french)
2020-06-27 12:22 ` Markus Armbruster
@ 2020-06-29 8:39 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 24+ messages in thread
From: Dr. David Alan Gilbert @ 2020-06-29 8:39 UTC (permalink / raw)
To: Markus Armbruster
Cc: kwolf, berrange, ehabkost, qemu-block, qemu-devel, mreitz,
pbonzini, John Snow
* Markus Armbruster (armbru@redhat.com) wrote:
> Cc: David for insurance against me spewing nonsense about migration.
>
> John Snow <jsnow@redhat.com> writes:
>
> > On 6/25/20 12:45 AM, Markus Armbruster wrote:
> >> John Snow <jsnow@redhat.com> writes:
> >>
> >>> On 6/22/20 5:42 AM, Markus Armbruster wrote:
> >>>> There are three ways to configure backends:
> >>>>
> >>>> * -nic, -serial, -drive, ... (onboard devices)
> >>>>
> >>>> * Set the property with -device, or, if you feel masochistic, with
> >>>> -set device (pluggable devices)
> >>>>
> >>>> * Set the property with -global (both)
> >>>>
> >>>> The trouble is -global is terrible.
> >>>>
> >>>> It gets applied in object_new(), which can't fail. We treat failure
> >>>> to apply -global as fatal error, except when hot-plugging, where we
> >>>> treat it as warning *boggle*. I'm not addressing that today.
> >>>>
> >>>> Some code falls apart when you use both -global and the other way.
> >>>>
> >>>> To make life more interesting, we gave -drive two roles: with
> >>>> interface type other than none, it's for configuring onboard devices,
> >>>> and with interface type none, it's for defining backends for use with
> >>>> -device and such. Since we neglect to require interface type none for
> >>>> the latter, you can use one -drive in both roles. This confuses the
> >>>> code about as much as you, dear reader, probably are by now.
> >>>>
> >>>> Because this still isn't interesting enough, there's yet another way
> >>>> to configure backends, just for floppies: set the floppy controller's
> >>>> property. Goes back to the time when floppy wasn't a separate device,
> >>>> and involves some Bad Magic. Now -global can interact with itself!
> >>>>
> >>>> Digging through all this took me an embarrassing amount of time.
> >>>> Hair, too.
> >>>>
> >>>> My patches reject some the silliest uses outright, and deprecate some
> >>>> not so silly ones that have replacements.
> >>>>
> >>>> Apply on top of my "[PATCH v2 00/58] qdev: Rework how we plug into the
> >>>> parent bus".
> >>>>
> >>>
> >>> Oof. Thank you for your work in fixing our darkest corners. I sincerely
> >>> appreciate it.
> >>>
> >>> The qdev tree ordering problems don't cause any issues for migration, do
> >>> they?
> >>
> >> This series should only change device configuration, not device state or
> >> its encoding in the migration stream.
> >>
> >> I'm not sure what you mean by "qdev tree ordering problems". Ist it
> >> commit e8c9e65816 'qom: Make "info qom-tree" show children sorted'?
> >>
> >>> (I see you already sent a PR, so whatever!)
> >>
> >> A question that might avoid a later migration debugging session is
> >> *never* "whatever"!
> >>
> >
> > I thought I had read that one of these patches changes the order in
> > which devices get instantiated, which I thought might change their QOM
> > paths. Which I thought *might* have some ramifications for migration,
> > but wasn't sure.
>
> Device instantiation order changes should not break migration.
They shouldn't; although I only narrowly stopped a new device from
making a mistake that would have made it dependent.
Of course you do have to explicitly state PCI/USB slot IDs otherwise the
allocation of those is order dependent.
> The order in which devices appear in the migration stream should not
> matter.
Order in the stream is a separate issue; we have ways to enforce that;
for example you want the interrupt controller to arrive before a device
that will raise an interrupt.
Dave
Dave
> > If it's just showing the same path outputs *sorted*, then there's no
> > problem.
> >
> > Likely misread.
> >
> > --js
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2020-06-29 8:41 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-22 9:42 [PATCH v2 00/16] Crazy shit around -global (pardon my french) Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 01/16] iotests/172: Include "info block" in test output Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 02/16] iotests/172: Cover empty filename and multiple use of drives Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 03/16] iotests/172: Cover -global floppy.drive= Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 04/16] fdc: Reject clash between -drive if=floppy and -global isa-fdc Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 05/16] fdc: Open-code fdctrl_init_isa() Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 06/16] fdc: Deprecate configuring floppies with -global isa-fdc Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 07/16] docs/qdev-device-use.txt: Update section "Default Devices" Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 08/16] blockdev: Deprecate -drive with bogus interface type Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 09/16] qdev: Eliminate get_pointer(), set_pointer() Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 10/16] qdev: Improve netdev property override error a bit Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 11/16] qdev: Reject drive property override Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 12/16] qdev: Reject chardev " Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 13/16] qdev: Make qdev_prop_set_drive() match the other helpers Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 14/16] arm/aspeed: Drop aspeed_board_init_flashes() parameter @errp Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 15/16] sd/pxa2xx_mmci: Don't crash on pxa2xx_mmci_init() error Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 16/16] sd/milkymist-memcard: Fix error API violation Markus Armbruster
2020-06-22 9:56 ` Philippe Mathieu-Daudé
2020-06-22 10:07 ` [PATCH v2 00/16] Crazy shit around -global (pardon my french) no-reply
2020-06-24 15:40 ` John Snow
2020-06-25 4:45 ` Markus Armbruster
2020-06-26 15:11 ` John Snow
2020-06-27 12:22 ` Markus Armbruster
2020-06-29 8:39 ` Dr. David Alan Gilbert
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).