[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH v2 11/16] qdev: Reject drive property override
From: |
Markus Armbruster |
Subject: |
[PATCH v2 11/16] qdev: Reject drive property override |
Date: |
Mon, 22 Jun 2020 11:42:22 +0200 |
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
- [PATCH v2 01/16] iotests/172: Include "info block" in test output, (continued)
- [PATCH v2 01/16] iotests/172: Include "info block" in test output, Markus Armbruster, 2020/06/22
- [PATCH v2 08/16] blockdev: Deprecate -drive with bogus interface type, Markus Armbruster, 2020/06/22
- [PATCH v2 09/16] qdev: Eliminate get_pointer(), set_pointer(), Markus Armbruster, 2020/06/22
- [PATCH v2 06/16] fdc: Deprecate configuring floppies with -global isa-fdc, Markus Armbruster, 2020/06/22
- [PATCH v2 04/16] fdc: Reject clash between -drive if=floppy and -global isa-fdc, Markus Armbruster, 2020/06/22
- [PATCH v2 13/16] qdev: Make qdev_prop_set_drive() match the other helpers, Markus Armbruster, 2020/06/22
- [PATCH v2 14/16] arm/aspeed: Drop aspeed_board_init_flashes() parameter @errp, Markus Armbruster, 2020/06/22
- [PATCH v2 15/16] sd/pxa2xx_mmci: Don't crash on pxa2xx_mmci_init() error, Markus Armbruster, 2020/06/22
- [PATCH v2 16/16] sd/milkymist-memcard: Fix error API violation, Markus Armbruster, 2020/06/22
- [PATCH v2 11/16] qdev: Reject drive property override,
Markus Armbruster <=
- [PATCH v2 12/16] qdev: Reject chardev property override, Markus Armbruster, 2020/06/22
- Re: [PATCH v2 00/16] Crazy shit around -global (pardon my french), no-reply, 2020/06/22
- Re: [PATCH v2 00/16] Crazy shit around -global (pardon my french), John Snow, 2020/06/24