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

["Denis V. Lunev" <den-lists@parallels.com>]

On 01/20/2016 10:40 PM, John Snow wrote:
>
> On 01/20/2016 02:55 AM, Denis V. Lunev wrote:
>> On 01/20/2016 09:51 AM, John Snow wrote:
>>> requires: 1448895398-13465-1-git-send-email-ehabkost@redhat.com
>>>             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.
may be. I have not personally checked, may be it just
works. I think that OVMF should be taken into the
equation...

So, in two words. We care about the floppy too :) Though
we have static config that works for our case.

Den