mirrors/qemu
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fdc: Reject clash between -drive if=floppy and -global isa-fdc

Browse Source 
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>
Message-Id: <20200622094227.1271650-5-armbru@redhat.com>
This commit is contained in:
Markus Armbruster 2020-06-22 11:42:15 +02:00
parent 02b83f7d7c
commit 6172e067a4
6 changed files with 79 additions and 167 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

51
hw/block/fdc.c
View File

@ -2497,6 +2497,29 @@ static void fdctrl_result_timer(void *opaque)
} }
/* Init functions */ /* 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, static void fdctrl_connect_drives(FDCtrl *fdctrl, DeviceState *fdc_dev,
Error **errp) Error **errp)
{ {
@ -2544,25 +2567,15 @@ static void fdctrl_connect_drives(FDCtrl *fdctrl, DeviceState *fdc_dev,
ISADevice *fdctrl_init_isa(ISABus *bus, DriveInfo **fds) ISADevice *fdctrl_init_isa(ISABus *bus, DriveInfo **fds)
{ {
DeviceState *dev;
ISADevice *isadev; ISADevice *isadev;
isadev = isa_try_new(TYPE_ISA_FDC); isadev = isa_try_new(TYPE_ISA_FDC);
if (!isadev) { if (!isadev) {
return NULL; 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_realize_and_unref(isadev, bus, &error_fatal);
isa_fdc_init_drives(isadev, fds);
return isadev; return isadev;
} }
@ -2578,18 +2591,12 @@ void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
sys = SYSBUS_FDC(dev); sys = SYSBUS_FDC(dev);
fdctrl = &sys->state; fdctrl = &sys->state;
fdctrl->dma_chann = dma_chann; /* FIXME */ 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); sbd = SYS_BUS_DEVICE(dev);
sysbus_realize_and_unref(sbd, &error_fatal); sysbus_realize_and_unref(sbd, &error_fatal);
sysbus_connect_irq(sbd, 0, irq); sysbus_connect_irq(sbd, 0, irq);
sysbus_mmio_map(sbd, 0, mmio_base); sysbus_mmio_map(sbd, 0, mmio_base);
fdctrl_init_drives(&sys->state.bus, fds);
} }
void sun4m_fdctrl_init(qemu_irq irq, hwaddr io_base, 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; FDCtrlSysBus *sys;
dev = qdev_new("SUNW,fdtwo"); 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); sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
sys = SYSBUS_FDC(dev); sys = SYSBUS_FDC(dev);
sysbus_connect_irq(SYS_BUS_DEVICE(sys), 0, irq); sysbus_connect_irq(SYS_BUS_DEVICE(sys), 0, irq);
sysbus_mmio_map(SYS_BUS_DEVICE(sys), 0, io_base); sysbus_mmio_map(SYS_BUS_DEVICE(sys), 0, io_base);
*fdc_tc = qdev_get_gpio_in(dev, 0); *fdc_tc = qdev_get_gpio_in(dev, 0);
fdctrl_init_drives(&sys->state.bus, fds);
} }
static void fdctrl_realize_common(DeviceState *dev, FDCtrl *fdctrl, static void fdctrl_realize_common(DeviceState *dev, FDCtrl *fdctrl,

18
hw/isa/isa-superio.c
View File

@ -17,6 +17,7 @@
#include "sysemu/sysemu.h" #include "sysemu/sysemu.h"
#include "sysemu/blockdev.h" #include "sysemu/blockdev.h"
#include "chardev/char.h" #include "chardev/char.h"
#include "hw/block/fdc.h"
#include "hw/isa/superio.h" #include "hw/isa/superio.h"
#include "hw/qdev-properties.h" #include "hw/qdev-properties.h"
#include "hw/input/i8042.h" #include "hw/input/i8042.h"
@ -31,7 +32,7 @@ static void isa_superio_realize(DeviceState *dev, Error **errp)
ISADevice *isa; ISADevice *isa;
DeviceState *d; DeviceState *d;
Chardev *chr; Chardev *chr;
DriveInfo *drive; DriveInfo *fd[MAX_FD];
char *name; char *name;
int i; int i;
@ -115,7 +116,7 @@ static void isa_superio_realize(DeviceState *dev, Error **errp)
/* Floppy disc */ /* Floppy disc */
if (!k->floppy.is_enabled || k->floppy.is_enabled(sio, 0)) { 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); d = DEVICE(isa);
if (k->floppy.get_iobase) { if (k->floppy.get_iobase) {
qdev_prop_set_uint32(d, "iobase", k->floppy.get_iobase(sio, 0)); 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)); qdev_prop_set_uint32(d, "irq", k->floppy.get_irq(sio, 0));
} }
/* FIXME use a qdev drive property instead of drive_get() */ /* FIXME use a qdev drive property instead of drive_get() */
drive = drive_get(IF_FLOPPY, 0, 0); for (i = 0; i < MAX_FD; i++) {
if (drive != NULL) { fd[i] = drive_get(IF_FLOPPY, 0, i);
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);
} }
object_property_add_child(OBJECT(sio), "isa-fdc", OBJECT(isa)); object_property_add_child(OBJECT(sio), "isa-fdc", OBJECT(isa));
isa_realize_and_unref(isa, bus, &error_fatal); isa_realize_and_unref(isa, bus, &error_fatal);
isa_fdc_init_drives(isa, fd);
sio->floppy = isa; sio->floppy = isa;
trace_superio_create_floppy(0, trace_superio_create_floppy(0,
k->floppy.get_iobase ? k->floppy.get_iobase ?

9
hw/sparc64/sun4u.c
View File

@ -341,16 +341,9 @@ static void ebus_realize(PCIDevice *pci_dev, Error **errp)
} }
isa_dev = isa_new(TYPE_ISA_FDC); isa_dev = isa_new(TYPE_ISA_FDC);
dev = DEVICE(isa_dev); 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); qdev_prop_set_uint32(dev, "dma", -1);
isa_realize_and_unref(isa_dev, s->isa_bus, &error_fatal); isa_realize_and_unref(isa_dev, s->isa_bus, &error_fatal);
isa_fdc_init_drives(isa_dev, fd);
/* Power */ /* Power */
dev = qdev_new(TYPE_SUN4U_POWER); dev = qdev_new(TYPE_SUN4U_POWER);

1
include/hw/block/fdc.h
View File

@ -9,6 +9,7 @@
#define TYPE_ISA_FDC "isa-fdc" #define TYPE_ISA_FDC "isa-fdc"
void isa_fdc_init_drives(ISADevice *fdc, DriveInfo **fds);
ISADevice *fdctrl_init_isa(ISABus *bus, DriveInfo **fds); ISADevice *fdctrl_init_isa(ISABus *bus, DriveInfo **fds);
void fdctrl_init_sysbus(qemu_irq irq, int dma_chann, void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
hwaddr mmio_base, DriveInfo **fds); hwaddr mmio_base, DriveInfo **fds);

3
tests/qemu-iotests/172
View File

@ -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 -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 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 -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 -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 check_floppy_qtree -fda "$TEST_IMG" -drive if=none,file="$TEST_IMG.2" -global floppy.drive=none0
echo echo

164
tests/qemu-iotests/172.out
View File

@ -205,22 +205,22 @@ Testing: -fdb
dev: floppy, id "" dev: floppy, id ""
unit = 1 (0x1) unit = 1 (0x1)
drive = "floppy1" drive = "floppy1"
logical_block_size = 512 (0x200) logical_block_size = 512 (512 B)
physical_block_size = 512 (0x200) physical_block_size = 512 (512 B)
min_io_size = 0 (0x0) min_io_size = 0 (0 B)
opt_io_size = 0 (0x0) opt_io_size = 0 (0 B)
discard_granularity = 4294967295 (0xffffffff) discard_granularity = 4294967295 (4 GiB)
write-cache = "auto" write-cache = "auto"
share-rw = false share-rw = false
drive-type = "288" drive-type = "288"
dev: floppy, id "" dev: floppy, id ""
unit = 0 (0x0) unit = 0 (0x0)
drive = "floppy0" drive = "floppy0"
logical_block_size = 512 (0x200) logical_block_size = 512 (512 B)
physical_block_size = 512 (0x200) physical_block_size = 512 (512 B)
min_io_size = 0 (0x0) min_io_size = 0 (0 B)
opt_io_size = 0 (0x0) opt_io_size = 0 (0 B)
discard_granularity = 4294967295 (0xffffffff) discard_granularity = 4294967295 (4 GiB)
write-cache = "auto" write-cache = "auto"
share-rw = false share-rw = false
drive-type = "288" 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 isa irq 6
bus: floppy-bus.0 bus: floppy-bus.0
type floppy-bus 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 "" dev: floppy, id ""
unit = 0 (0x0) unit = 0 (0x0)
drive = "floppy0" 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" write-cache = "auto"
share-rw = false share-rw = false
drive-type = "144" 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) 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 Removable device: not locked, tray closed
Cache mode: writeback Cache mode: writeback
none0 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2) 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 Removable device: not locked, tray closed
Cache mode: writeback 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 Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveA=none0
QEMU_PROG: Floppy unit 0 is in use
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
Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveB=none0 Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveB=none0
QEMU_PROG: Floppy unit 1 is in use
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
Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global floppy.drive=none0 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 "" dev: floppy, id ""
unit = 0 (0x0) unit = 0 (0x0)
drive = "floppy0" drive = "floppy0"
logical_block_size = 512 (0x200) logical_block_size = 512 (512 B)
physical_block_size = 512 (0x200) physical_block_size = 512 (512 B)
min_io_size = 0 (0x0) min_io_size = 0 (0 B)
opt_io_size = 0 (0x0) opt_io_size = 0 (0 B)
discard_granularity = 4294967295 (0xffffffff) discard_granularity = 4294967295 (4 GiB)
write-cache = "auto" write-cache = "auto"
share-rw = false share-rw = false
drive-type = "144" drive-type = "144"
@ -1500,11 +1418,11 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -global floppy.drive=none0 -device
dev: floppy, id "" dev: floppy, id ""
unit = 0 (0x0) unit = 0 (0x0)
drive = "none0" drive = "none0"
logical_block_size = 512 (0x200) logical_block_size = 512 (512 B)
physical_block_size = 512 (0x200) physical_block_size = 512 (512 B)
min_io_size = 0 (0x0) min_io_size = 0 (0 B)
opt_io_size = 0 (0x0) opt_io_size = 0 (0 B)
discard_granularity = 4294967295 (0xffffffff) discard_granularity = 4294967295 (4 GiB)
write-cache = "auto" write-cache = "auto"
share-rw = false share-rw = false
drive-type = "144" 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 "" dev: floppy, id ""
unit = 0 (0x0) unit = 0 (0x0)
drive = "none1" drive = "none1"
logical_block_size = 512 (0x200) logical_block_size = 512 (512 B)
physical_block_size = 512 (0x200) physical_block_size = 512 (512 B)
min_io_size = 0 (0x0) min_io_size = 0 (0 B)
opt_io_size = 0 (0x0) opt_io_size = 0 (0 B)
discard_granularity = 4294967295 (0xffffffff) discard_granularity = 4294967295 (4 GiB)
write-cache = "auto" write-cache = "auto"
share-rw = false share-rw = false
drive-type = "144" 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?) 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 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 Testing: -device floppy,drive=floppy0
QEMU_PROG: -device floppy,drive=floppy0: Property 'floppy.drive' can't find value 'floppy0' QEMU_PROG: -device floppy,drive=floppy0: Property 'floppy.drive' can't find value 'floppy0'