Re: [Qemu-block] [Qemu-devel] [PATCH v2 3/4] fdc: Move qdev properties to FloppyDrive
Am 15.10.2016 um 00:32 hat John Snow geschrieben: > On 09/30/2016 03:39 PM, Kevin Wolf wrote: > >This makes the FloppyDrive qdev object actually useful: Now that it has > >all properties that don't belong to the controller, you can actually > >use '-device floppy' and get a working result. > > > >Command line semantics is consistent with CD-ROM drives: By default you > >get a single empty floppy drive. You can override it with -drive and > >using the same index, but if you use -drive to add a floppy to a > >different index, you get both of them. However, as soon as you use any > >'-device floppy', even to a different slot, the default drive is > >disabled. > > > >Using '-device floppy' without specifying the unit will choose the first > >free slot on the controller. > > > >Signed-off-by: Kevin Wolf > >--- > > hw/block/fdc.c | 112 > > ++--- > > vl.c | 1 + > > 2 files changed, 85 insertions(+), 28 deletions(-) > > > >diff --git a/hw/block/fdc.c b/hw/block/fdc.c > >index 5aa8e52..00c0ec6 100644 > >--- a/hw/block/fdc.c > >+++ b/hw/block/fdc.c > >@@ -35,6 +35,7 @@ > > #include "qemu/timer.h" > > #include "hw/isa/isa.h" > > #include "hw/sysbus.h" > >+#include "hw/block/block.h" > > #include "sysemu/block-backend.h" > > #include "sysemu/blockdev.h" > > #include "sysemu/sysemu.h" > >@@ -487,12 +488,18 @@ static const BlockDevOps fd_block_ops = { > > OBJECT_CHECK(FloppyDrive, (obj), TYPE_FLOPPY_DRIVE) > > > > typedef struct FloppyDrive { > >-DeviceState qdev; > >-uint32_tunit; > >+DeviceState qdev; > >+uint32_tunit; > >+BlockConf conf; > >+FloppyDriveType type; > > } FloppyDrive; > > > > static Property floppy_drive_properties[] = { > > DEFINE_PROP_UINT32("unit", FloppyDrive, unit, -1), > >+DEFINE_BLOCK_PROPERTIES(FloppyDrive, conf), > >+DEFINE_PROP_DEFAULT("drive-type", FloppyDrive, type, > >+FLOPPY_DRIVE_TYPE_AUTO, qdev_prop_fdc_drive_type, > >+FloppyDriveType), > > DEFINE_PROP_END_OF_LIST(), > > }; > > > >@@ -501,6 +508,7 @@ static int floppy_drive_init(DeviceState *qdev) > > FloppyDrive *dev = FLOPPY_DRIVE(qdev); > > FloppyBus *bus = DO_UPCAST(FloppyBus, bus, dev->qdev.parent_bus); > > FDrive *drive; > >+int ret; > > > > if (dev->unit == -1) { > > for (dev->unit = 0; dev->unit < MAX_FD; dev->unit++) { > >@@ -517,29 +525,57 @@ static int floppy_drive_init(DeviceState *qdev) > > return -1; > > } > > > >-/* TODO Check whether unit is in use */ > >- > > drive = get_drv(bus->fdc, dev->unit); > >- > > if (drive->blk) { > >-if (blk_get_on_error(drive->blk, 0) != BLOCKDEV_ON_ERROR_ENOSPC) { > >-error_report("fdc doesn't support drive option werror"); > >-return -1; > >-} > >-if (blk_get_on_error(drive->blk, 1) != BLOCKDEV_ON_ERROR_REPORT) { > >-error_report("fdc doesn't support drive option rerror"); > >-return -1; > >-} > >-} else { > >+error_report("Floppy unit %d is in use", dev->unit); > >+return -1; > >+} > >+ > >+if (!dev->conf.blk) { > > /* Anonymous BlockBackend for an empty drive */ > >-drive->blk = blk_new(); > >+dev->conf.blk = blk_new(); > >+ret = blk_attach_dev(dev->conf.blk, dev); > > Missing a 'q' here: ^ Yes. It has the same value, but after my last pull request we need a DeviceState* here indeed rather than a void*. > >@@ -2782,8 +2838,8 @@ static const VMStateDescription vmstate_sysbus_fdc ={ > > }; > > > > static Property sysbus_fdc_properties[] = { > >-DEFINE_PROP_DRIVE("driveA", FDCtrlSysBus, state.drives[0].blk), > >-DEFINE_PROP_DRIVE("driveB", FDCtrlSysBus, state.drives[1].blk), > >+DEFINE_PROP_DRIVE("driveA", FDCtrlSysBus, state.qdev_for_drives[0].blk), > >+DEFINE_PROP_DRIVE("driveB", FDCtrlSysBus, state.qdev_for_drives[1].blk), > > DEFINE_PROP_DEFAULT("fdtypeA", FDCtrlSysBus, state.drives[0].drive, > > FLOPPY_DRIVE_TYPE_AUTO, qdev_prop_fdc_drive_type, > > FloppyDriveType), > > ^ Does sysbus' type property not need updating ...? Doing half of the properties here felt like a good transitional step from fully converting the PC device to completely ignoring Sun. Well, I guess, I should fix that... Kevin
Re: [Qemu-block] [Qemu-devel] [PATCH v2 3/4] fdc: Move qdev properties to FloppyDrive
On 09/30/2016 03:39 PM, Kevin Wolf wrote: This makes the FloppyDrive qdev object actually useful: Now that it has all properties that don't belong to the controller, you can actually use '-device floppy' and get a working result. Command line semantics is consistent with CD-ROM drives: By default you get a single empty floppy drive. You can override it with -drive and using the same index, but if you use -drive to add a floppy to a different index, you get both of them. However, as soon as you use any '-device floppy', even to a different slot, the default drive is disabled. Using '-device floppy' without specifying the unit will choose the first free slot on the controller. Signed-off-by: Kevin Wolf --- hw/block/fdc.c | 112 ++--- vl.c | 1 + 2 files changed, 85 insertions(+), 28 deletions(-) diff --git a/hw/block/fdc.c b/hw/block/fdc.c index 5aa8e52..00c0ec6 100644 --- a/hw/block/fdc.c +++ b/hw/block/fdc.c @@ -35,6 +35,7 @@ #include "qemu/timer.h" #include "hw/isa/isa.h" #include "hw/sysbus.h" +#include "hw/block/block.h" #include "sysemu/block-backend.h" #include "sysemu/blockdev.h" #include "sysemu/sysemu.h" @@ -487,12 +488,18 @@ static const BlockDevOps fd_block_ops = { OBJECT_CHECK(FloppyDrive, (obj), TYPE_FLOPPY_DRIVE) typedef struct FloppyDrive { -DeviceState qdev; -uint32_tunit; +DeviceState qdev; +uint32_tunit; +BlockConf conf; +FloppyDriveType type; } FloppyDrive; static Property floppy_drive_properties[] = { DEFINE_PROP_UINT32("unit", FloppyDrive, unit, -1), +DEFINE_BLOCK_PROPERTIES(FloppyDrive, conf), +DEFINE_PROP_DEFAULT("drive-type", FloppyDrive, type, +FLOPPY_DRIVE_TYPE_AUTO, qdev_prop_fdc_drive_type, +FloppyDriveType), DEFINE_PROP_END_OF_LIST(), }; @@ -501,6 +508,7 @@ static int floppy_drive_init(DeviceState *qdev) FloppyDrive *dev = FLOPPY_DRIVE(qdev); FloppyBus *bus = DO_UPCAST(FloppyBus, bus, dev->qdev.parent_bus); FDrive *drive; +int ret; if (dev->unit == -1) { for (dev->unit = 0; dev->unit < MAX_FD; dev->unit++) { @@ -517,29 +525,57 @@ static int floppy_drive_init(DeviceState *qdev) return -1; } -/* TODO Check whether unit is in use */ - drive = get_drv(bus->fdc, dev->unit); - if (drive->blk) { -if (blk_get_on_error(drive->blk, 0) != BLOCKDEV_ON_ERROR_ENOSPC) { -error_report("fdc doesn't support drive option werror"); -return -1; -} -if (blk_get_on_error(drive->blk, 1) != BLOCKDEV_ON_ERROR_REPORT) { -error_report("fdc doesn't support drive option rerror"); -return -1; -} -} else { +error_report("Floppy unit %d is in use", dev->unit); +return -1; +} + +if (!dev->conf.blk) { /* Anonymous BlockBackend for an empty drive */ -drive->blk = blk_new(); +dev->conf.blk = blk_new(); +ret = blk_attach_dev(dev->conf.blk, dev); Missing a 'q' here: ^ +assert(ret == 0); } -fd_init(drive); -if (drive->blk) { -blk_set_dev_ops(drive->blk, &fd_block_ops, drive); -pick_drive_type(drive); +blkconf_blocksizes(&dev->conf); +if (dev->conf.logical_block_size != 512 || +dev->conf.physical_block_size != 512) +{ +error_report("Physical and logical block size must be 512 for floppy"); +return -1; +} + +/* rerror/werror aren't supported by fdc and therefore not even registered + * with qdev. So set the defaults manually before they are used in + * blkconf_apply_backend_options(). */ +dev->conf.rerror = BLOCKDEV_ON_ERROR_AUTO; +dev->conf.werror = BLOCKDEV_ON_ERROR_AUTO; +blkconf_apply_backend_options(&dev->conf); + +/* 'enospc' is the default for -drive, 'report' is what blk_new() gives us + * for empty drives. */ +if (blk_get_on_error(dev->conf.blk, 0) != BLOCKDEV_ON_ERROR_ENOSPC && +blk_get_on_error(dev->conf.blk, 0) != BLOCKDEV_ON_ERROR_REPORT) { +error_report("fdc doesn't support drive option werror"); +return -1; } +if (blk_get_on_error(dev->conf.blk, 1) != BLOCKDEV_ON_ERROR_REPORT) { +error_report("fdc doesn't support drive option rerror"); +return -1; +} + +drive->blk = dev->conf.blk; +drive->fdctrl = bus->fdc; + +fd_init(drive); +blk_set_dev_ops(drive->blk, &fd_block_ops, drive); + +/* Keep 'type' qdev property and FDrive->drive in sync */ +drive->drive = dev->type; +pick_drive_type(drive); +dev->type = drive->drive; + fd_revalidate(drive); return 0; @@ -808,6 +844,10 @@ struct FDCtrl { FloppyBus bus; uint8_t num_floppies; FDrive drives[MAX_FD]; +struct { +BlockBackend *blk; +FloppyDriveType type; +} qdev_for_drives[MAX_FD]; int