qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [Qemu-devel] [PATCH v3 03/11] fdc: add disk field


From: Markus Armbruster
Subject: Re: [Qemu-block] [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) {



reply via email to

[Prev in Thread] Current Thread [Next in Thread]