From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47783) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLyfV-0000rK-TW for qemu-devel@nongnu.org; Wed, 20 Jan 2016 14:43:51 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aLyfU-0002YD-B1 for qemu-devel@nongnu.org; Wed, 20 Jan 2016 14:43:49 -0500 References: <1453272694-17106-1-git-send-email-jsnow@redhat.com> <569F3D8B.8000607@parallels.com> <569FE29E.1060901@redhat.com> From: "Denis V. Lunev" Message-ID: <569FE35F.6060600@parallels.com> Date: Wed, 20 Jan 2016 22:43:27 +0300 MIME-Version: 1.0 In-Reply-To: <569FE29E.1060901@redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Qemu-block] [PATCH v4 00/12] fdc: fix 2.88mb floppy diskette support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow , qemu-block@nongnu.org Cc: kwolf@redhat.com, armbru@redhat.com, qemu-devel@nongnu.org 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