All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Snow <jsnow@redhat.com>
To: Kevin Wolf <kwolf@redhat.com>, qemu-block@nongnu.org
Cc: qemu-devel@nongnu.org, armbru@redhat.com, mreitz@redhat.com,
	fred.konrad@greensocs.com
Subject: Re: [Qemu-devel] [PATCH v5 0/4] fdc: Use separate qdev device for drives
Date: Mon, 24 Oct 2016 14:44:30 -0400	[thread overview]
Message-ID: <4d44da4e-ef4a-d8bc-154c-fc91e96a96df@redhat.com> (raw)
In-Reply-To: <1477309068-1333-1-git-send-email-kwolf@redhat.com>



On 10/24/2016 07:37 AM, Kevin Wolf wrote:
> We have been complaining for a long time about how the floppy controller and
> floppy drives are combined in a single qdev device and how this makes the
> device awkward to work with because it behaves different from all other block
> devices.
>
> The latest reason to complain was when I noticed that using qdev device names
> in QMP commands (e.g. for media change) doesn't really work when only the
> controller is a qdev device, but the drives aren't.
>
> So I decided to have a go at it, and this is the result.
>
> It doesn't actually change any of the inner workings of the floppy controller,
> but it wires things up differently on the qdev layer so that a floppy
> controller now exposes a bus on which the floppy drives sit. This results in a
> structure that is similar to IDE where the actual drive state is still in the
> controller and the qdev device basically just contains the qdev properties -
> not pretty, but quite workable.
>
> The commit message of patch 3 explains how to use it. In short, there is a
> '-device floppy' now and it does what you would expect if you ever used ide-cd.
>
> The other problem is old command lines, especially those using things like
> '-global isa-fdc,driveA=...'. In order to keep them working, we need to forward
> the property to an internally created floppy drive device. This is a bit like
> usb-storage, which we know is ugly, but works well enough in practice. The good
> news here is that in contrast to usb-storage, the floppy controller only does
> the forwarding for legacy configurations; as soon as you start using '-device
> floppy', it doesn't happen any more.
>
> So as you may have expected, this conversion doesn't result in a perfect
> device, but I think it's definitely an improvement over the old state. I hope
> you like it despite the warts. :-)
>
> v5:
> - Apply _filter_qemu to stderr, too [John]
> - Rename the bus to floppy-bus [Frederic]
> - Use FLOPPY_BUS() instead of DO_UPDATE() [Frederic]
>
> v4:
> - John says that his grep is broken and hangs at 100% CPU with my attempt to
>   extract the floppy controller from info qtree. Use a simpler sed command
>   instead (which, unlike the grep command, doesn't handle arbitrary indentation
>   level of the next item, but we know what comes next, so just hardcoding 10
>   spaces works, too).
>
> v3:
> - Fixed omissons in the conversion sysbus-fdc and the Sun one. Nice, combining
>   floppy controllers and weird platforms in a single series. [John]
>
> v2:
> - Added patch 4 (qemu-iotests case for floppy config on the command line)
> - Patch 2: Create a floppy device only if a BlockBackend exists instead of
>   always creating two of them
> - Patch 2: Initialise drive->fdctrl even if no drive is attached, it is
>   accessed anyway during migration
> - Patch 3: Keep 'type' qdev property and FDrive->drive in sync
> - Patch 3: Removed if with condition that is always true
>
> Alberto Garcia (2):
>   throttle: Correct access to wrong BlockBackendPublic structures
>   qemu-iotests: Test I/O in a single drive from a throttling group
>
> Halil Pasic (1):
>   block: improve error handling in raw_open
>
> Pino Toscano (1):
>   qapi: fix memory leak in bdrv_image_info_specific_dump
>

uhhh

http://i.imgur.com/60YQvmg.png

>  block/qapi.c               |  1 +
>  block/raw-posix.c          |  1 +
>  block/raw-win32.c          |  1 +
>  block/throttle-groups.c    | 27 +++++++++++++++++++++++----
>  tests/qemu-iotests/093     | 33 ++++++++++++++++++++++++++++-----
>  tests/qemu-iotests/093.out |  4 ++--
>  6 files changed, 56 insertions(+), 11 deletions(-)
>

  parent reply	other threads:[~2016-10-24 18:44 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-24 11:37 [Qemu-devel] [PATCH v5 0/4] fdc: Use separate qdev device for drives Kevin Wolf
2016-10-24 11:37 ` [Qemu-devel] [PATCH v5 1/4] block: improve error handling in raw_open Kevin Wolf
2016-10-24 11:37 ` [Qemu-devel] [PATCH v5 2/4] qapi: fix memory leak in bdrv_image_info_specific_dump Kevin Wolf
2016-10-24 11:37 ` [Qemu-devel] [PATCH v5 3/4] throttle: Correct access to wrong BlockBackendPublic structures Kevin Wolf
2016-10-24 11:37 ` [Qemu-devel] [PATCH v5 4/4] qemu-iotests: Test I/O in a single drive from a throttling group Kevin Wolf
2016-10-24 18:44 ` John Snow [this message]
2016-10-25  9:02   ` [Qemu-devel] [PATCH v5 0/4] fdc: Use separate qdev device for drives Kevin Wolf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4d44da4e-ef4a-d8bc-154c-fc91e96a96df@redhat.com \
    --to=jsnow@redhat.com \
    --cc=armbru@redhat.com \
    --cc=fred.konrad@greensocs.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.