[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 03/11] fdc: add disk field
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v3 03/11] fdc: add disk field |
Date: |
Thu, 17 Dec 2015 19:15:25 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
John Snow <address@hidden> writes:
> On 12/17/2015 03:30 AM, Markus Armbruster wrote:
>> John Snow <address@hidden> 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 <address@hidden>
>>> ---
>>> 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.
Could the following work?
@drive is the connected drive's type, if any. Can't change without a
power cycle.
@flags, @last_sect, ... together describe the medium (a.k.a. disk), if
any. @drive constrains the possible values.
*Except* we can have a special "Schrödinger's drive", for backward
compatibility. It's type is indeterminate until something looks at it.
Then its wave function collapses, and an ordinary drive emerges.
[...]
[Qemu-devel] [PATCH v3 05/11] fdc: Add fallback option, John Snow, 2015/12/16
[Qemu-devel] [PATCH v3 08/11] fdc: add physical disk sizes, John Snow, 2015/12/16
[Qemu-devel] [PATCH v3 02/11] fdc: refactor pick_geometry, John Snow, 2015/12/16