Re: [Qemu-devel] [PATCH v4 09/12] fdc: add physical disk sizes

[John Snow <jsnow@redhat.com>]

On 01/20/2016 05:48 PM, Eric Blake wrote:
> On 01/19/2016 11:51 PM, John Snow wrote:
>> 2.88MB capable drives can accept 1.44MB floppies,
>> for instance. To rework the pick_geometry function,
>> we need to know if our current drive can even accept
>> the type of disks we're considering.
>>
>> NB: This allows us to distinguish between all of the
>> "total sectors" collisions between 1.20MB and 1.44MB
>> diskette types, by using the physical drive size as a
>> differentiator.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>  hw/block/fdc.c | 40 ++++++++++++++++++++++++++++++++--------
>>  1 file changed, 32 insertions(+), 8 deletions(-)
>>
> 
>> +/* In many cases, the total sector size of a format is enough to uniquely
>> + * identify it. However, there are some total sector collisions between
>> + * formats of different physical size, and these are noted below by
>> + * highlighting the total sector size for entries with collisions. */
>>  static const FDFormat fd_formats[] = {
>>      /* First entry is default format */
>>      /* 1.44 MB 3"1/2 floppy disks */
>> -    { FLOPPY_DRIVE_TYPE_144, 18, 80, 1, FDRIVE_RATE_500K, },
>> -    { FLOPPY_DRIVE_TYPE_144, 20, 80, 1, FDRIVE_RATE_500K, },
>> +    { FLOPPY_DRIVE_TYPE_144, 18, 80, 1, FDRIVE_RATE_500K, }, /* 3.5" 2880 */
>> +    { FLOPPY_DRIVE_TYPE_144, 20, 80, 1, FDRIVE_RATE_500K, }, /* 3.5" 3200 */
>>      { FLOPPY_DRIVE_TYPE_144, 21, 80, 1, FDRIVE_RATE_500K, },
>>      { FLOPPY_DRIVE_TYPE_144, 21, 82, 1, FDRIVE_RATE_500K, },
>>      { FLOPPY_DRIVE_TYPE_144, 21, 83, 1, FDRIVE_RATE_500K, },
>> @@ -85,7 +95,7 @@ static const FDFormat fd_formats[] = {
>>      { FLOPPY_DRIVE_TYPE_288, 44, 80, 1, FDRIVE_RATE_1M, },
>>      { FLOPPY_DRIVE_TYPE_288, 48, 80, 1, FDRIVE_RATE_1M, },
>>      /* 720 kB 3"1/2 floppy disks */
>> -    { FLOPPY_DRIVE_TYPE_144,  9, 80, 1, FDRIVE_RATE_250K, },
>> +    { FLOPPY_DRIVE_TYPE_144,  9, 80, 1, FDRIVE_RATE_250K, }, /* 3.5" 1400 */
> 
> Typo? I think you meant 1440.
> 

Good catch.

>> +__attribute__((__unused__))
>> +static FDriveSize drive_size(FloppyDriveType drive)
>> +{
>> +    switch (drive) {
>> +    case FLOPPY_DRIVE_TYPE_120:
>> +        return FDRIVE_SIZE_525;
>> +    case FLOPPY_DRIVE_TYPE_144:
>> +    case FLOPPY_DRIVE_TYPE_288:
>> +        return FDRIVE_SIZE_350;
>> +    default:
>> +        return FDRIVE_SIZE_UNKNOWN;
>> +    }
>> +}
> 
> With that fixed,
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 

Thanks,
--js