John Snow js...@redhat.com writes:
We want to change the current default drive type,
but to be kind, we need to allow users to specify
the old drive type somehow.
Uh, what is *this* commit about? as far as I can tell, it adds drive
type properties (not a default drive type option), but doesn't put
them to use, yet.
Signed-off-by: John Snow js...@redhat.com
---
hw/block/fdc.c | 13 +
hw/core/qdev-properties.c| 12
include/hw/block/fdc.h | 7 +--
include/hw/qdev-properties.h | 1 +
qapi/block.json | 15 +++
5 files changed, 42 insertions(+), 6 deletions(-)
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index cdf9e09..1023a01 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -67,6 +67,8 @@ typedef struct FDFormat {
FDriveRate rate;
} FDFormat;
+#define FDRIVE_DEFAULT FDRIVE_DRV_144
+
static const FDFormat fd_formats[] = {
/* First entry is default format */
/* 1.44 MB 31/2 floppy disks */
@@ -578,6 +580,9 @@ struct FDCtrl {
/* Timers state */
uint8_t timer0;
uint8_t timer1;
+
+FDriveType defaultA;
+FDriveType defaultB;
};
#define TYPE_SYSBUS_FDC base-sysbus-fdc
@@ -2423,6 +2428,10 @@ 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(defaultA, FDCtrlISABus, state.defaultA,
+FDRIVE_DEFAULT, qdev_prop_fdc_drive_type,
FDriveType),
+DEFINE_PROP_DEFAULT(defaultB, FDCtrlISABus, state.defaultB,
+FDRIVE_DEFAULT, qdev_prop_fdc_drive_type,
FDriveType),
DEFINE_PROP_END_OF_LIST(),
};
@@ -2471,6 +2480,10 @@ 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(defaultA, FDCtrlSysBus, state.defaultA,
+FDRIVE_DEFAULT, qdev_prop_fdc_drive_type,
FDriveType),
+DEFINE_PROP_DEFAULT(defaultB, FDCtrlSysBus, state.defaultB,
+FDRIVE_DEFAULT, qdev_prop_fdc_drive_type,
FDriveType),
DEFINE_PROP_END_OF_LIST(),
};
defaultA is not exactly a self-documenting name for a property
selecting the drive type. Even equally laconic typeA feels better.
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 47c1e8f..7ff8030 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -7,6 +7,7 @@
#include net/hub.h
#include qapi/visitor.h
#include sysemu/char.h
+#include hw/block/fdc.h
Why do you need to add the include?
void qdev_prop_set_after_realize(DeviceState *dev, const char *name,
Error **errp)
@@ -543,6 +544,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,
+ none/120/144/288,
+.enum_table = FDRIVE_DRV_lookup,
+.get = get_enum,
+.set = set_enum
+};
+
/* --- pci address --- */
/*
diff --git a/include/hw/block/fdc.h b/include/hw/block/fdc.h
index d48b2f8..f727027 100644
--- a/include/hw/block/fdc.h
+++ b/include/hw/block/fdc.h
@@ -6,12 +6,7 @@
/* fdc.c */
#define MAX_FD 2
-typedef enum FDriveType {
-FDRIVE_DRV_144 = 0x00, /* 1.44 MB 35 drive */
-FDRIVE_DRV_288 = 0x01, /* 2.88 MB 35 drive */
-FDRIVE_DRV_120 = 0x02, /* 1.2 MB 525 drive */
-FDRIVE_DRV_NONE = 0x03, /* No drive connected */
-} FDriveType;
+typedef FDRIVE_DRV FDriveType;
See below.
#define TYPE_ISA_FDC isa-fdc
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 0cfff1c..0872b41 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 aad645c..0c747a1 100644
--- a/qapi/block.json
+++ b/qapi/block.json
@@ -40,6 +40,21 @@
'data': ['auto', 'none', 'lba', 'large', 'rechs']}
##
+# FDRIVE_DRV:
Stick in the usual @, please, just for consistency.
FDRIVE_DRV is an unusual name for a QAPI type. I guess you chose it so
only the type name changes, but the enumeration constants stay the same.
You then hide away the type name change with a typedef in
