Re: [Qemu-devel] [Qemu-block] [PATCH v4 00/12] fdc: fix 2.88mb floppy diskette support

[John Snow]

On 01/20/2016 02:55 AM, Denis V. Lunev wrote:
> On 01/20/2016 09:51 AM, John Snow wrote:
>> requires: address@hidden
>>            pc: Add pc-*-2.6 machine classes
>>
>> Yes, it's been broken for ten years.
>> No, it's not a CVE.
>>
>> The problem is that QEMU doesn't have a configuration option for the type
>> of floppy drive you want. It determines that based on the type of
>> diskette inserted at boot time.
>>
>> If you don't insert one, it always chooses a 1.44MB type.
>>
>> If you want to insert a 2.88MB floppy after boot, you simply cannot.
>>
>> "Wow, who cares?"
>>
>> Good question -- Unfortunately, the virtio-win floppy disk images that
>> Red Hat/Fedora ship require a 2.88MB drive, so if you forgot to insert
>> them at boot, you'd have to change your VM configuration and try again.
>>
>> For a one-shot operation, that's kind of obnoxious -- it'd be nice to
>> allow one to just insert the diskette on-demand.
>>
>> "OK, What are you changing in this decades-old device?"
>>
>> (1) Add a new property to allow users to specify what kind of drive they
>>      want without relying on magical guessing behavior.
>>      Choices are: 120, 144, 288, auto, and none.
>>
>>      120, 144 and 288 refer to 1.20MB, 1.44MB, and 2.88MB drives.
>>      auto refers to the auto-detect behavior QEMU currently has.
>>      none ... hides the drive. You probably don't want to use this,
>>      but it's there if you feel like creating a drive you can't use.
>>
>> (2) Add a new "fallback" property for use with the "auto" drive type
>>      that allows us to specify the backup behavior, too. In most cases
>>      this property won't be needed, but it is provided for allowing
>>      QEMU to be fully backwards compatible.
>>
>> (3) Add the concept of physical diskette size to QEMU, classifying
>>      120-style diskettes as fundamentally different from 144 and 288
>> ones.
>>
>> (4) Revamp the automatic guessing heuristic to understand that
>>      2.88MB style drives can accept 1.44MB diskettes.
>>
>> (5) Change the automatic fallback type for the automatic guessing
>>      heuristic from 1.44MB to 2.88MB for 2.6 machines and beyond,
>>      leaving 2.5- machines set to default to auto/144.
>>
>> (6) A lot of code cleanup in general.
>>
>> "Won't this break everything, you madman?"
>>
>> No: I tested this in MS-DOS 6.22, Fedora 23 and Windows 8.1. All
>> seemed perfectly happy with 2.88MB drives as the default for 1.44
>> or 2.88MB floppy diskette images.
>>
>> And: Older machine types will happily still default to the 1.44
>>       type just like they used to, so really nothing should change
>>       at all for most guests.
>>
>> If there ARE any guests affected in 2.6+ machine types, you are
>> urged to use an explicit drive type that matches your application
>> if the automatic behavior is unsuitable.
>>
>> ===
>> v4:
>> ===
>>
>> Hopefully a more logical patch order with smaller changes.
>>
>> 001/12:[----] [--] 'fdc: move pick_geometry'
>> 002/12:[down] 'fdc: reduce number of pick_geometry arguments'
>> 003/12:[down] 'fdc: add drive type qapi enum'
>> 004/12:[0008] [FC] 'fdc: add disk field'
>> 005/12:[down] 'fdc: Throw an assertion on misconfigured fd_formats table'
>> 006/12:[down] 'fdc: add pick_drive'
>> 007/12:[0018] [FC] 'fdc: Add fallback option'
>> 008/12:[down] 'fdc: add drive type option'
>> 009/12:[----] [-C] 'fdc: add physical disk sizes'
>> 010/12:[0014] [FC] 'fdc: rework pick_geometry'
>> 011/12:[----] [--] 'qtest/fdc: Support for 2.88MB drives'
>> 012/12:[0010] [FC] 'fdc: change auto fallback drive for ISA FDC to 288'
>>
>> 02: Kept both debug printfs in fd_revalidate.
>> 03: New patch, QAPI enumeration change only.
>> 04: Re-ordered FDrive fields
>>      Fallout from 03.
>> 05: New patch.
>> 06: Almost completely re-done.
>> 07: Added media_validated property
>>      Fallout from patch re-ordering.
>> 08: Re-ordered.
>> 10: Changed return type of pick_geometry to int.
>>      Changed one error pathway to abort, as it's not a run-time problem.
>> 12: Rebased on top of current master.
>>
>> ===
>> v3:
>> ===
>>
>> 001/11:[----] [--] 'fdc: move pick_geometry'
>> 002/11:[----] [--] 'fdc: refactor pick_geometry'
>> 003/11:[----] [--] 'fdc: add disk field'
>> 004/11:[0037] [FC] 'fdc: add default drive type option'
>> 005/11:[down] 'fdc: Add fallback option'
>> 006/11:[----] [-C] 'fdc: do not call revalidate on eject'
>> 007/11:[0030] [FC] 'fdc: implement new drive type property'
>> 008/11:[----] [-C] 'fdc: add physical disk sizes'
>> 009/11:[0018] [FC] 'fdc: rework pick_geometry'
>> 010/11:[----] [--] 'qtest/fdc: Support for 2.88MB drives'
>> 011/11:[down] 'fdc: change auto fallback drive for ISA FDC to 288'
>>
>> 04: Remove typeA/typeB members of FDCtrl. Store e.g. -fdtypeA options
>>             directly into FDCtrl.drives[x].drive instead.
>> 05: Add a new fallback= option that controls fdtype{A,B}=auto behavior.
>> 07: replace get_default_drive_type which is no longer needed
>>      add get_fallback_drive_type.
>> 09: Reworked the auto/fallback section of pick_geometry.
>>
>> ________________________________________________________________________________
>>
>>
>> For convenience, this branch is available at:
>> https://github.com/jnsnow/qemu.git branch fdc-default
>> https://github.com/jnsnow/qemu/tree/fdc-default
>>
>> This version is tagged fdc-default-v4:
>> https://github.com/jnsnow/qemu/releases/tag/fdc-default-v4
>>
>> John Snow (12):
>>    fdc: move pick_geometry
>>    fdc: reduce number of pick_geometry arguments
>>    fdc: add drive type qapi enum
>>    fdc: add disk field
>>    fdc: Throw an assertion on misconfigured fd_formats table
>>    fdc: add pick_drive
>>    fdc: Add fallback option
>>    fdc: add drive type option
>>    fdc: add physical disk sizes
>>    fdc: rework pick_geometry
>>    qtest/fdc: Support for 2.88MB drives
>>    fdc: change auto fallback drive for ISA FDC to 288
>>
>>   hw/block/fdc.c               | 315
>> +++++++++++++++++++++++++++++--------------
>>   hw/core/qdev-properties.c    |  11 ++
>>   hw/i386/pc.c                 |  17 +--
>>   include/hw/block/fdc.h       |   9 +-
>>   include/hw/compat.h          |   4 +
>>   include/hw/qdev-properties.h |   1 +
>>   qapi/block.json              |  16 +++
>>   tests/fdc-test.c             |   2 +-
>>   8 files changed, 259 insertions(+), 116 deletions(-)
>>
> should we recreate ACPI tables after geometry switch?
> This would be especially interesting for the case of
> Win2k12 (or Win8.1 if you prefer) under OVMF.
> 
> Den

This series doesn't really alter the concept that disk geometry can
change at runtime -- Not knowing much about the ACPI reverse engineering
that happened to make Windows 8/10 happy, does it work currently? Can
you change to different density floppies and have it work out alright?

If not, you can submit a patch against master as it is today -- this
series only does two things:

(1) Alters the heuristics for which type of floppy drive is chosen at
boot time (No change to ACPI table generation should be needed.)

(2) Allows 1.44MB diskettes to be recognized by 2.88MB drive types. This
might require some changes, but check out pick_geometry both before and
after this patchset -- there's a whole table of different geometries
that we already allow users to switch between at runtime. If the
geometry needs to update there, too, then it's already broken before
this patchset.

It should be easy enough to slide a geometry update in fd_revalidate()
if needed.