[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 09:30:14 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
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.
> 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?
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?
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) {
[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