Re: [Qemu-devel] [PULL 07/13] fdc: add pick_drive

[John Snow]

On 01/27/2016 07:38 AM, Paolo Bonzini wrote:
> 
> 
> On 25/01/2016 20:41, John Snow wrote:
>> Split apart pick_geometry by creating a pick_drive routine that will only
>> ever called during device bring-up instead of relying on pick_geometry to
>> be used in both cases.
>>
>> With this change, the drive field is changed to be 'write once'. It is
>> not altered after the initialization routines exit.
>>
>> media_validated does not need to be migrated. The target VM
>> will just revalidate the media on post_load anyway.
>>
>> Reviewed-by: Eric Blake <address@hidden>
>> Signed-off-by: John Snow <address@hidden>
>> Message-id: address@hidden
> 
> Sorry, this breaks the RHEL6.5 Linux installer CD.  It just hangs at
> floppy detection and finally panics.
> 
> Paolo
> 

Crap, alright. I'll get right to it. That's what I get for poking a
hibernating bear.

>> ---
>>  hw/block/fdc.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++----------
>>  1 file changed, 46 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
>> index a8f0cf2..f8e070e 100644
>> --- a/hw/block/fdc.c
>> +++ b/hw/block/fdc.c
>> @@ -151,6 +151,7 @@ typedef struct FDrive {
>>      uint8_t media_rate;       /* Data rate of medium    */
>>  
>>      bool media_inserted;      /* Is there a medium in the tray */
>> +    bool media_validated;     /* Have we validated the media? */
>>  } FDrive;
>>  
>>  static void fd_init(FDrive *drv)
>> @@ -162,6 +163,8 @@ static void fd_init(FDrive *drv)
>>      drv->disk = FLOPPY_DRIVE_TYPE_NONE;
>>      drv->last_sect = 0;
>>      drv->max_track = 0;
>> +    drv->ro = true;
>> +    drv->media_changed = 1;
>>  }
>>  
>>  #define NUM_SIDES(drv) ((drv)->flags & FDISK_DBL_SIDES ? 2 : 1)
>> @@ -244,13 +247,24 @@ static void fd_recalibrate(FDrive *drv)
>>      fd_seek(drv, 0, 0, 1, 1);
>>  }
>>  
>> -static void pick_geometry(FDrive *drv)
>> +/**
>> + * Determine geometry based on inserted diskette.
>> + * Will not operate on an empty drive.
>> + *
>> + * @return: 0 on success, -1 if the drive is empty.
>> + */
>> +static int pick_geometry(FDrive *drv)
>>  {
>>      BlockBackend *blk = drv->blk;
>>      const FDFormat *parse;
>>      uint64_t nb_sectors, size;
>>      int i, first_match, match;
>>  
>> +    /* We can only pick a geometry if we have a diskette. */
>> +    if (!drv->media_inserted) {
>> +        return -1;
>> +    }
>> +
>>      blk_get_geometry(blk, &nb_sectors);
>>      match = -1;
>>      first_match = -1;
>> @@ -290,31 +304,51 @@ 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 : FLOPPY_DRIVE_TYPE_NONE;
>> +    drv->disk = parse->drive;
>>      drv->media_rate = parse->rate;
>> +    return 0;
>> +}
>> +
>> +static void pick_drive_type(FDrive *drv)
>> +{
>> +    if (pick_geometry(drv) == 0) {
>> +        drv->drive = drv->disk;
>> +    } else {
>> +        /* Legacy behavior: default to 1.44MB floppy */
>> +        drv->drive = FLOPPY_DRIVE_TYPE_144;
>> +    }
>>  }
>>  
>>  /* Revalidate a disk drive after a disk change */
>>  static void fd_revalidate(FDrive *drv)
>>  {
>> +    int rc;
>> +
>>      FLOPPY_DPRINTF("revalidate\n");
>>      if (drv->blk != NULL) {
>>          drv->ro = blk_is_read_only(drv->blk);
>> -        pick_geometry(drv);
>>          if (!drv->media_inserted) {
>>              FLOPPY_DPRINTF("No disk in drive\n");
>> -        } else {
>> -            FLOPPY_DPRINTF("Floppy disk (%d h %d t %d s) %s\n",
>> -                           (drv->flags & FDISK_DBL_SIDES) ? 2 : 1,
>> -                           drv->max_track, drv->last_sect,
>> -                           drv->ro ? "ro" : "rw");
>> +            drv->disk = FLOPPY_DRIVE_TYPE_NONE;
>> +        } else if (!drv->media_validated) {
>> +            rc = pick_geometry(drv);
>> +            if (rc) {
>> +                FLOPPY_DPRINTF("Could not validate floppy drive media");
>> +            } else {
>> +                drv->media_validated = true;
>> +                FLOPPY_DPRINTF("Floppy disk (%d h %d t %d s) %s\n",
>> +                               (drv->flags & FDISK_DBL_SIDES) ? 2 : 1,
>> +                               drv->max_track, drv->last_sect,
>> +                               drv->ro ? "ro" : "rw");
>> +            }
>>          }
>>      } else {
>>          FLOPPY_DPRINTF("No drive connected\n");
>>          drv->last_sect = 0;
>>          drv->max_track = 0;
>>          drv->flags &= ~FDISK_DBL_SIDES;
>> +        drv->drive = FLOPPY_DRIVE_TYPE_NONE;
>> +        drv->disk = FLOPPY_DRIVE_TYPE_NONE;
>>      }
>>  }
>>  
>> @@ -2185,6 +2219,7 @@ static void fdctrl_change_cb(void *opaque, bool load)
>>      drive->media_inserted = load && drive->blk && 
>> blk_is_inserted(drive->blk);
>>  
>>      drive->media_changed = 1;
>> +    drive->media_validated = false;
>>      fd_revalidate(drive);
>>  }
>>  
>> @@ -2221,11 +2256,12 @@ static void fdctrl_connect_drives(FDCtrl *fdctrl, 
>> Error **errp)
>>          }
>>  
>>          fd_init(drive);
>> -        fdctrl_change_cb(drive, 0);
>>          if (drive->blk) {
>>              blk_set_dev_ops(drive->blk, &fdctrl_block_ops, drive);
>>              drive->media_inserted = blk_is_inserted(drive->blk);
>> +            pick_drive_type(drive);
>>          }
>> +        fd_revalidate(drive);
>>      }
>>  }
>>  
>>
> 

-- 
—js