From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38714) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bw3fo-0001bd-ID for qemu-devel@nongnu.org; Mon, 17 Oct 2016 04:53:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bw3fn-0004A5-HH for qemu-devel@nongnu.org; Mon, 17 Oct 2016 04:53:32 -0400 Date: Mon, 17 Oct 2016 10:53:24 +0200 From: Kevin Wolf Message-ID: <20161017085324.GB4821@noname.redhat.com> References: <1475264368-9838-1-git-send-email-kwolf@redhat.com> <1475264368-9838-4-git-send-email-kwolf@redhat.com> <58a36fa8-41ad-1dc3-b9ff-c9f01ef5f73a@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <58a36fa8-41ad-1dc3-b9ff-c9f01ef5f73a@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 3/4] fdc: Move qdev properties to FloppyDrive List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow Cc: qemu-block@nongnu.org, qemu-devel@nongnu.org, armbru@redhat.com, mreitz@redhat.com 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_t unit; > >+ DeviceState qdev; > >+ uint32_t unit; > >+ 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