John Snow <js...@redhat.com> writes:
> This patch adds a new explicit Floppy Drive Type option. The existing
> behavior in QEMU is to automatically guess a drive type based on the
> media inserted, or if a diskette is not present, arbitrarily assign one.
>
> This behavior can be described as "auto." This patch adds explicit
> behaviors: 120, 144, 288, auto, and none. The new "auto" behavior
> is intended to mimick current behavior, while the other types pick
> one explicitly.
>
> In a future patch, the goal is to change the FDC's default drive type
> from auto (falling back to 1.44MB) to auto (falling back to 2.88MB).
Err, will we have two different kinds of auto then?
> In order to allow users to obtain the old behaviors, though, a mechanism
> for specifying the exact type of drive we want is needed.
I'd expect users who understand drive types sufficiently to pick one not
to pick the old behavior, because it's *nuts*.
> This patch adds the properties, but it is not acted on yet in favor of
> making those changes a little more explicitly clear in standalone patches
> later in this patch set.
>
> Signed-off-by: John Snow <js...@redhat.com>
> ---
> hw/block/fdc.c | 107
> ++++++++++++++++++++++++++-----------------
> hw/core/qdev-properties.c | 11 +++++
> hw/i386/pc.c | 17 +++----
> include/hw/block/fdc.h | 9 +---
> include/hw/qdev-properties.h | 1 +
> qapi/block.json | 16 +++++++
> 6 files changed, 102 insertions(+), 59 deletions(-)
>
> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> index 13fef23..ad0e052 100644
> --- a/hw/block/fdc.c
> +++ b/hw/block/fdc.c
> @@ -60,58 +60,60 @@ typedef enum FDriveRate {
> } FDriveRate;
>
> typedef struct FDFormat {
> - FDriveType drive;
> + FloppyDriveType drive;
> uint8_t last_sect;
> uint8_t max_track;
> uint8_t max_head;
> FDriveRate rate;
> } FDFormat;
>
> +#define FDRIVE_DEFAULT FLOPPY_DRIVE_TYPE_AUTO
> +
> static const FDFormat fd_formats[] = {
> /* First entry is default format */
> /* 1.44 MB 3"1/2 floppy disks */
> - { FDRIVE_DRV_144, 18, 80, 1, FDRIVE_RATE_500K, },
> - { FDRIVE_DRV_144, 20, 80, 1, FDRIVE_RATE_500K, },
> - { FDRIVE_DRV_144, 21, 80, 1, FDRIVE_RATE_500K, },
> - { FDRIVE_DRV_144, 21, 82, 1, FDRIVE_RATE_500K, },
> - { FDRIVE_DRV_144, 21, 83, 1, FDRIVE_RATE_500K, },
> - { FDRIVE_DRV_144, 22, 80, 1, FDRIVE_RATE_500K, },
> - { FDRIVE_DRV_144, 23, 80, 1, FDRIVE_RATE_500K, },
> - { FDRIVE_DRV_144, 24, 80, 1, FDRIVE_RATE_500K, },
> + { FLOPPY_DRIVE_TYPE_144, 18, 80, 1, FDRIVE_RATE_500K, },
> + { FLOPPY_DRIVE_TYPE_144, 20, 80, 1, FDRIVE_RATE_500K, },
> + { FLOPPY_DRIVE_TYPE_144, 21, 80, 1, FDRIVE_RATE_500K, },
> + { FLOPPY_DRIVE_TYPE_144, 21, 82, 1, FDRIVE_RATE_500K, },
> + { FLOPPY_DRIVE_TYPE_144, 21, 83, 1, FDRIVE_RATE_500K, },
> + { FLOPPY_DRIVE_TYPE_144, 22, 80, 1, FDRIVE_RATE_500K, },
> + { FLOPPY_DRIVE_TYPE_144, 23, 80, 1, FDRIVE_RATE_500K, },
> + { FLOPPY_DRIVE_TYPE_144, 24, 80, 1, FDRIVE_RATE_500K, },
> /* 2.88 MB 3"1/2 floppy disks */
> - { FDRIVE_DRV_288, 36, 80, 1, FDRIVE_RATE_1M, },
> - { FDRIVE_DRV_288, 39, 80, 1, FDRIVE_RATE_1M, },
> - { FDRIVE_DRV_288, 40, 80, 1, FDRIVE_RATE_1M, },
> - { FDRIVE_DRV_288, 44, 80, 1, FDRIVE_RATE_1M, },
> - { FDRIVE_DRV_288, 48, 80, 1, FDRIVE_RATE_1M, },
> + { FLOPPY_DRIVE_TYPE_288, 36, 80, 1, FDRIVE_RATE_1M, },
> + { FLOPPY_DRIVE_TYPE_288, 39, 80, 1, FDRIVE_RATE_1M, },
> + { FLOPPY_DRIVE_TYPE_288, 40, 80, 1, FDRIVE_RATE_1M, },
> + { FLOPPY_DRIVE_TYPE_288, 44, 80, 1, FDRIVE_RATE_1M, },
> + { FLOPPY_DRIVE_TYPE_288, 48, 80, 1, FDRIVE_RATE_1M, },
> /* 720 kB 3"1/2 floppy disks */
> - { FDRIVE_DRV_144, 9, 80, 1, FDRIVE_RATE_250K, },
> - { FDRIVE_DRV_144, 10, 80, 1, FDRIVE_RATE_250K, },
> - { FDRIVE_DRV_144, 10, 82, 1, FDRIVE_RATE_250K, },
> - { FDRIVE_DRV_144, 10, 83, 1, FDRIVE_RATE_250K, },
> - { FDRIVE_DRV_144, 13, 80, 1, FDRIVE_RATE_250K, },
> - { FDRIVE_DRV_144, 14, 80, 1, FDRIVE_RATE_250K, },
> + { FLOPPY_DRIVE_TYPE_144, 9, 80, 1, FDRIVE_RATE_250K, },
> + { FLOPPY_DRIVE_TYPE_144, 10, 80, 1, FDRIVE_RATE_250K, },
> + { FLOPPY_DRIVE_TYPE_144, 10, 82, 1, FDRIVE_RATE_250K, },
> + { FLOPPY_DRIVE_TYPE_144, 10, 83, 1, FDRIVE_RATE_250K, },
> + { FLOPPY_DRIVE_TYPE_144, 13, 80, 1, FDRIVE_RATE_250K, },
> + { FLOPPY_DRIVE_TYPE_144, 14, 80, 1, FDRIVE_RATE_250K, },
> /* 1.2 MB 5"1/4 floppy disks */
> - { FDRIVE_DRV_120, 15, 80, 1, FDRIVE_RATE_500K, },
> - { FDRIVE_DRV_120, 18, 80, 1, FDRIVE_RATE_500K, },
> - { FDRIVE_DRV_120, 18, 82, 1, FDRIVE_RATE_500K, },
> - { FDRIVE_DRV_120, 18, 83, 1, FDRIVE_RATE_500K, },
> - { FDRIVE_DRV_120, 20, 80, 1, FDRIVE_RATE_500K, },
> + { FLOPPY_DRIVE_TYPE_120, 15, 80, 1, FDRIVE_RATE_500K, },
> + { FLOPPY_DRIVE_TYPE_120, 18, 80, 1, FDRIVE_RATE_500K, },
> + { FLOPPY_DRIVE_TYPE_120, 18, 82, 1, FDRIVE_RATE_500K, },
> + { FLOPPY_DRIVE_TYPE_120, 18, 83, 1, FDRIVE_RATE_500K, },
> + { FLOPPY_DRIVE_TYPE_120, 20, 80, 1, FDRIVE_RATE_500K, },
> /* 720 kB 5"1/4 floppy disks */
> - { FDRIVE_DRV_120, 9, 80, 1, FDRIVE_RATE_250K, },
> - { FDRIVE_DRV_120, 11, 80, 1, FDRIVE_RATE_250K, },
> + { FLOPPY_DRIVE_TYPE_120, 9, 80, 1, FDRIVE_RATE_250K, },
> + { FLOPPY_DRIVE_TYPE_120, 11, 80, 1, FDRIVE_RATE_250K, },
> /* 360 kB 5"1/4 floppy disks */
> - { FDRIVE_DRV_120, 9, 40, 1, FDRIVE_RATE_300K, },
> - { FDRIVE_DRV_120, 9, 40, 0, FDRIVE_RATE_300K, },
> - { FDRIVE_DRV_120, 10, 41, 1, FDRIVE_RATE_300K, },
> - { FDRIVE_DRV_120, 10, 42, 1, FDRIVE_RATE_300K, },
> + { FLOPPY_DRIVE_TYPE_120, 9, 40, 1, FDRIVE_RATE_300K, },
> + { FLOPPY_DRIVE_TYPE_120, 9, 40, 0, FDRIVE_RATE_300K, },
> + { FLOPPY_DRIVE_TYPE_120, 10, 41, 1, FDRIVE_RATE_300K, },
> + { FLOPPY_DRIVE_TYPE_120, 10, 42, 1, FDRIVE_RATE_300K, },
> /* 320 kB 5"1/4 floppy disks */
> - { FDRIVE_DRV_120, 8, 40, 1, FDRIVE_RATE_250K, },
> - { FDRIVE_DRV_120, 8, 40, 0, FDRIVE_RATE_250K, },
> + { FLOPPY_DRIVE_TYPE_120, 8, 40, 1, FDRIVE_RATE_250K, },
> + { FLOPPY_DRIVE_TYPE_120, 8, 40, 0, FDRIVE_RATE_250K, },
> /* 360 kB must match 5"1/4 better than 3"1/2... */
> - { FDRIVE_DRV_144, 9, 80, 0, FDRIVE_RATE_250K, },
> + { FLOPPY_DRIVE_TYPE_144, 9, 80, 0, FDRIVE_RATE_250K, },
> /* end */
> - { FDRIVE_DRV_NONE, -1, -1, 0, 0, },
> + { FLOPPY_DRIVE_TYPE_NONE, -1, -1, 0, 0, },
> };
>
> #define GET_CUR_DRV(fdctrl) ((fdctrl)->cur_drv)
> @@ -133,8 +135,9 @@ typedef struct FDrive {
> FDCtrl *fdctrl;
> BlockBackend *blk;
> /* Drive status */
> - FDriveType drive; /* CMOS drive type */
> - FDriveType disk; /* Current disk type */
> + FloppyDriveType drive; /* CMOS drive type */
> + FloppyDriveType disk; /* Current disk type */
> +
> uint8_t perpendicular; /* 2.88 MB access mode */
> /* Position */
> uint8_t head;
> @@ -155,10 +158,10 @@ typedef struct FDrive {
> static void fd_init(FDrive *drv)
> {
> /* Drive */
> - drv->drive = FDRIVE_DRV_NONE;
> + drv->drive = FLOPPY_DRIVE_TYPE_NONE; /* FIXME: Obey CLI properties */
> drv->perpendicular = 0;
> /* Disk */
> - drv->disk = FDRIVE_DRV_NONE;
> + drv->disk = FLOPPY_DRIVE_TYPE_NONE;
> drv->last_sect = 0;
> drv->max_track = 0;
> }
> @@ -255,11 +258,11 @@ static void pick_geometry(FDrive *drv)
> first_match = -1;
> for (i = 0; ; i++) {
> parse = &fd_formats[i];
> - if (parse->drive == FDRIVE_DRV_NONE) {
> + if (parse->drive == FLOPPY_DRIVE_TYPE_NONE) {
> break;
> }
> if (drv->drive == parse->drive ||
> - drv->drive == FDRIVE_DRV_NONE) {
> + drv->drive == FLOPPY_DRIVE_TYPE_NONE) {
> size = (parse->max_head + 1) * parse->max_track *
> parse->last_sect;
> if (nb_sectors == size) {
> @@ -288,7 +291,7 @@ static void pick_geometry(FDrive *drv)
> drv->max_track = parse->max_track;
> drv->last_sect = parse->last_sect;
> drv->drive = parse->drive;
> - drv->disk = drv->media_inserted ? parse->drive : FDRIVE_DRV_NONE;
> + drv->disk = drv->media_inserted ? parse->drive : FLOPPY_DRIVE_TYPE_NONE;
> drv->media_rate = parse->rate;
>
> if (drv->media_inserted) {
> @@ -566,6 +569,7 @@ struct FDCtrl {
> /* Timers state */
> uint8_t timer0;
> uint8_t timer1;
> +
> };
>
> #define TYPE_SYSBUS_FDC "base-sysbus-fdc"
> @@ -2224,6 +2228,8 @@ static void fdctrl_connect_drives(FDCtrl *fdctrl, Error
> **errp)
> if (drive->blk) {
> blk_set_dev_ops(drive->blk, &fdctrl_block_ops, drive);
> drive->media_inserted = blk_is_inserted(drive->blk);
> + } else {
> + drive->drive = FLOPPY_DRIVE_TYPE_NONE;
> }
> }
> }
> @@ -2396,7 +2402,7 @@ static void sysbus_fdc_common_realize(DeviceState *dev,
> Error **errp)
> fdctrl_realize_common(fdctrl, errp);
> }
>
> -FDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i)
> +FloppyDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i)
> {
> FDCtrlISABus *isa = ISA_FDC(fdc);
>
> @@ -2421,6 +2427,12 @@ static Property isa_fdc_properties[] = {
> DEFINE_PROP_DRIVE("driveB", FDCtrlISABus, state.drives[1].blk),
> DEFINE_PROP_BIT("check_media_rate", FDCtrlISABus, state.check_media_rate,
> 0, true),
> + DEFINE_PROP_DEFAULT("fdtypeA", FDCtrlISABus, state.drives[0].drive,
> + FDRIVE_DEFAULT, qdev_prop_fdc_drive_type,
> + FloppyDriveType),
> + DEFINE_PROP_DEFAULT("fdtypeB", FDCtrlISABus, state.drives[1].drive,
> + FDRIVE_DEFAULT, qdev_prop_fdc_drive_type,
> + FloppyDriveType),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> @@ -2469,6 +2481,12 @@ 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_DEFAULT("fdtypeA", FDCtrlSysBus, state.drives[0].drive,
> + FDRIVE_DEFAULT, qdev_prop_fdc_drive_type,
> + FloppyDriveType),
> + DEFINE_PROP_DEFAULT("fdtypeB", FDCtrlSysBus, state.drives[1].drive,
> + FDRIVE_DEFAULT, qdev_prop_fdc_drive_type,
> + FloppyDriveType),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> @@ -2489,6 +2507,9 @@ static const TypeInfo sysbus_fdc_info = {
>
> static Property sun4m_fdc_properties[] = {
> DEFINE_PROP_DRIVE("drive", FDCtrlSysBus, state.drives[0].blk),
> + DEFINE_PROP_DEFAULT("fdtype", FDCtrlSysBus, state.drives[0].drive,
> + FDRIVE_DEFAULT, qdev_prop_fdc_drive_type,
> + FloppyDriveType),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 33e245e..b43943e 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -541,6 +541,17 @@ PropertyInfo qdev_prop_bios_chs_trans = {
> .set = set_enum,
> };
>
> +/* --- FDC default drive types */
> +
> +PropertyInfo qdev_prop_fdc_drive_type = {
> + .name = "FdcDriveType",
> + .description = "FDC drive type, "
> + "144/288/120/none/auto",
> + .enum_table = FloppyDriveType_lookup,
> + .get = get_enum,
> + .set = set_enum
> +};
> +
> /* --- pci address --- */
>
> /*
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 72d9b9c..02bfc74 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -207,24 +207,24 @@ static void pic_irq_request(void *opaque, int irq, int
> level)
>
> #define REG_EQUIPMENT_BYTE 0x14
>
> -static int cmos_get_fd_drive_type(FDriveType fd0)
> +static int cmos_get_fd_drive_type(FloppyDriveType fd0)
> {
> int val;
>
> switch (fd0) {
> - case FDRIVE_DRV_144:
> + case FLOPPY_DRIVE_TYPE_144:
> /* 1.44 Mb 3"5 drive */
> val = 4;
> break;
> - case FDRIVE_DRV_288:
> + case FLOPPY_DRIVE_TYPE_288:
> /* 2.88 Mb 3"5 drive */
> val = 5;
> break;
> - case FDRIVE_DRV_120:
> + case FLOPPY_DRIVE_TYPE_120:
> /* 1.2 Mb 5"5 drive */
> val = 2;
> break;
> - case FDRIVE_DRV_NONE:
> + case FLOPPY_DRIVE_TYPE_NONE:
> default:
> val = 0;
> break;
> @@ -295,7 +295,8 @@ static void pc_boot_set(void *opaque, const char
> *boot_device, Error **errp)
> static void pc_cmos_init_floppy(ISADevice *rtc_state, ISADevice *floppy)
> {
> int val, nb, i;
> - FDriveType fd_type[2] = { FDRIVE_DRV_NONE, FDRIVE_DRV_NONE };
> + FloppyDriveType fd_type[2] = { FLOPPY_DRIVE_TYPE_NONE,
> + FLOPPY_DRIVE_TYPE_NONE };
>
> /* floppy type */
> if (floppy) {
> @@ -309,10 +310,10 @@ static void pc_cmos_init_floppy(ISADevice *rtc_state,
> ISADevice *floppy)
>
> val = rtc_get_memory(rtc_state, REG_EQUIPMENT_BYTE);
> nb = 0;
> - if (fd_type[0] < FDRIVE_DRV_NONE) {
> + if (fd_type[0] != FLOPPY_DRIVE_TYPE_NONE) {
> nb++;
> }
> - if (fd_type[1] < FDRIVE_DRV_NONE) {
> + if (fd_type[1] != FLOPPY_DRIVE_TYPE_NONE) {
> nb++;
> }
> switch (nb) {
> diff --git a/include/hw/block/fdc.h b/include/hw/block/fdc.h
> index d48b2f8..adce14f 100644
> --- a/include/hw/block/fdc.h
> +++ b/include/hw/block/fdc.h
> @@ -6,13 +6,6 @@
> /* fdc.c */
> #define MAX_FD 2
>
> -typedef enum FDriveType {
> - FDRIVE_DRV_144 = 0x00, /* 1.44 MB 3"5 drive */
> - FDRIVE_DRV_288 = 0x01, /* 2.88 MB 3"5 drive */
> - FDRIVE_DRV_120 = 0x02, /* 1.2 MB 5"25 drive */
> - FDRIVE_DRV_NONE = 0x03, /* No drive connected */
> -} FDriveType;
> -
> #define TYPE_ISA_FDC "isa-fdc"
>
> ISADevice *fdctrl_init_isa(ISABus *bus, DriveInfo **fds);
> @@ -21,6 +14,6 @@ void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
> void sun4m_fdctrl_init(qemu_irq irq, hwaddr io_base,
> DriveInfo **fds, qemu_irq *fdc_tc);
>
> -FDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i);
> +FloppyDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i);
>
> #endif
> diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
> index 77538a8..c26797e 100644
> --- a/include/hw/qdev-properties.h
> +++ b/include/hw/qdev-properties.h
> @@ -20,6 +20,7 @@ extern PropertyInfo qdev_prop_ptr;
> extern PropertyInfo qdev_prop_macaddr;
> extern PropertyInfo qdev_prop_losttickpolicy;
> extern PropertyInfo qdev_prop_bios_chs_trans;
> +extern PropertyInfo qdev_prop_fdc_drive_type;
> extern PropertyInfo qdev_prop_drive;
> extern PropertyInfo qdev_prop_netdev;
> extern PropertyInfo qdev_prop_vlan;
> diff --git a/qapi/block.json b/qapi/block.json
> index 84022f1..36d4bac 100644
> --- a/qapi/block.json
> +++ b/qapi/block.json
> @@ -40,6 +40,22 @@
> 'data': ['auto', 'none', 'lba', 'large', 'rechs']}
>
> ##
> +# @FloppyDriveType
> +#
> +# Type of Floppy drive to be emulated by the Floppy Disk Controller.
> +#
> +# @144: 1.44MB 3.5" drive
> +# @288: 2.88MB 3.5" drive
> +# @120: 1.5MB 5.25" drive
> +# @none: No drive connected
> +# @auto: Automatically determined by inserted media at boot
> +#
> +# Since: 2.6
> +##
> +{ 'enum': 'FloppyDriveType',
> + 'data': ['144', '288', '120', 'none', 'auto']}
We have to permit members names starting with a digit, but that can't
make me like them.
> +
> +##
> # @BlockdevSnapshotInternal
> #
> # @device: the name of the device to generate the snapshot from
Massive churn because you chose to rename FDriveType and its members.
Could be avoided if anyone cares.
However, the churn makes the other changes in this patch hard to see.
Can you split it into a pure rename (including the new QAPI type) and
the rest?
