On 12/17/2015 03:30 AM, Markus Armbruster wrote:
> John Snow <js...@redhat.com> writes:
>
>> This allows us to distinguish between the current disk type and the
>> current drive type. The drive is what's reported to CMOS, the disk is
>> whatever the pick_geometry function suspects has been inserted.
>>
>> The drive field maintains the exact same meaning as it did previously,
>> however pick_geometry/fd_revalidate will be refactored to *never* update
>> the drive field, considering it frozen in-place during an earlier
>> initialization call.
>>
>> Before this patch, pick_geometry/fd_revalidate could only change the
>> drive field when it was FDRIVE_DRV_NONE, which indicated that we had
>> not yet reported our drive type to CMOS. After we "pick one," even
>> though pick_geometry/fd_revalidate re-set drv->drive, it should always
>> be the same as the value going into these calls, so it is effectively
>> already static.
>>
>> As of this patch, disk and drive will always be the same, but that may
>> not be true by the end of this series.
>>
>> Disk does not need to be migrated because it is not user-visible state
>> nor is it currently used for any calculations. It is purely informative,
>> and will be rebuilt automatically via fd_revalidate on the new host.
>>
>> Signed-off-by: John Snow <js...@redhat.com>
>> ---
>> hw/block/fdc.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
>> index 09bb63d..13fef23 100644
>> --- a/hw/block/fdc.c
>> +++ b/hw/block/fdc.c
>> @@ -133,7 +133,8 @@ typedef struct FDrive {
> typedef struct FDrive {
>> FDCtrl *fdctrl;
>> BlockBackend *blk;
>> /* Drive status */
>> - FDriveType drive;
>> + FDriveType drive; /* CMOS drive type */
>> + FDriveType disk; /* Current disk type */
>
> where
>
> 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;
>
> FDriveType makes obvious sense as drive type.
>
Sadly yes, because we have very thoroughly intermixed the concept of
media and drive, so DriveType still makes sense for the Diskette.
>> uint8_t perpendicular; /* 2.88 MB access mode */
>
> If I understand @perpendicular correctly, it's just an extra hoop a
> driver needs to jump through to actually access a 2.88M disk.
>
>> /* Position */
>> uint8_t head;
> uint8_t track;
> uint8_t sect;
> /* Media */
>
> Shouldn't @disk go here?
>
Oh, yes.
> FDiskFlags flags;
> uint8_t last_sect; /* Nb sector per track */
> uint8_t max_track; /* Nb of tracks */
> uint16_t bps; /* Bytes per sector */
> uint8_t ro; /* Is read-only */
> uint8_t media_changed; /* Is media changed */
> uint8_t media_rate; /* Data rate of medium */
>
> bool media_inserted; /* Is there a medium in the tray */
> } FDrive;
>
> A disk / medium is characterised by it's form factor, density /
> FDriveRate, #sides, #tracks, #sectors per track, and a read-only bit
> (ignoring a few details that don't interest us). Obviously, the drive
> type restricts possible disk types.
>
> What does @disk add over the media description we already have?
>
It's mostly a semantic game to allow the pick_geometry function to
never, ever, ever write to the "drive" field -- it will operate
exclusively on the "disk" field. Callers can decide what to do with that
information.
The idea in the long haul is to make @drive a "write once or never" kind
of ordeal, and I introduced the new field to help enforce that.
It's purely sugar.
Maybe in the future if I work on the FDC some more it will become useful
for other purposes, but for now it's almost exclusively to inform the
'drive' type when drive is set to AUTO.
> Aside: @bps appears to be write-only, and the value written looks odd.
>
>> @@ -157,6 +158,7 @@ static void fd_init(FDrive *drv)
>> drv->drive = FDRIVE_DRV_NONE;
>> drv->perpendicular = 0;
>> /* Disk */
>> + drv->disk = FDRIVE_DRV_NONE;
>> drv->last_sect = 0;
>> drv->max_track = 0;
>> }
>> @@ -286,6 +288,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->media_rate = parse->rate;
>>
>> if (drv->media_inserted) {
--
—js
