[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RFC PATCH 2/2] Split fdd devices off the floppy contro

From: Anthony Liguori
Subject: Re: [Qemu-devel] [RFC PATCH 2/2] Split fdd devices off the floppy controller
Date: Mon, 14 May 2012 08:42:26 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20120329 Thunderbird/11.0.1

On 05/11/2012 10:22 AM, Markus Armbruster wrote:
For historical reasons, and unlike other block devices, our floppy
devices isa-fdc, sysbus-fdc and SUNW,fdtwo integrate the controller
and the drive(s) in a single qdev.  This makes them weird: we need
-global to set up floppy drives, unlike every other optional device.

This patch explores separating them.  It's not a working solution,
yet.  I'm posting it to give us something concrete to discuss.

Separating them involves splitting the per-drive state (struct FDrive)
into a part that belongs to the controller (remains in FDrive), and a
part that belongs to the drive (moves to new struct FDD).  I should
perhaps rename FDrive to reflect that.

An example for state that clearly belongs to the drive is the block
backend.  This patch moves it.  More members of FDrive need moving,
e.g. drive.  To be done in separate commits.  Might impact migration.

I put the fdd objects right into /machine/.  Maybe they should go
elsewhere.  For what it's worth, IDE drives are in

Bug: I give all of them the same name "fdd".  QOM happily accepts it.

Instead of definining a full-blown qbus to connect controller and
drives, I link them directly.

There's no use of QOM links in the tree, yet, and the QOM
documentation isn't terribly helpful there.  Please review my

I add one link per fdd the board supports, but I set it only for the
fdds actually present.  With one of two fdds present, qom-fuse shows:

     $ ls -l machine/unattached/device\[18\]/fdd*
     lrwxr-xr-x. 2 1000 1000 4096 Jan  1  1970 machine/unattached/device[18]/fdd0 
->  ../../../machine/fdd
     lrwxr-xr-x. 2 1000 1000 4096 Jan  1  1970 machine/unattached/device[18]/fdd1 
->  ../../..

The second one is weird.

That's a bug in qom-fuse. It's an empty link and I unconditionally prepend the relative path to the root to make the non-empty links work. It's an easy fix.

Unfortunately, eliding the qbus means I can't make the floppy disk a
qdev (sub-class of TYPE_DEVICE), because qdevs can only connect to a
qbus.  Anthony tells me that restriction is gone in his latest QOM

Yup. It's queued in qom-next actually (which is probably where you want to work now).

Since it's not a qdev, -device fdd does not work.  Pity, because it
defeats the stated purpose of making floppy disk drives work like
other existing optional devices.

There doesn't seem to be a way to create QOM objects via command line
or monitor.

Speaking of monitor: the QOM commands are only implemented in QMP, and
they are entirely undocumented.  Sets a bad example; wonder how it got
past the maintainer ;)

They're thoroughly documented actually...

# @qom-get:
# This command will get a property from a object model path and return the
# value.
# @path: The path within the object model.  There are two forms of supported
#        paths--absolute and partial paths.
#        Absolute paths are derived from the root object and can follow child<>
#        or link<> properties.  Since they can follow link<> properties, they
#        can be arbitrarily long.  Absolute paths look like absolute filenames
#        and are prefixed  with a leading slash.
#        Partial paths look like relative filenames.  They do not begin
#        with a prefix.  The matching rules for partial paths are subtle but
#        designed to make specifying objects easy.  At each level of the
#        composition tree, the partial path is matched as an absolute path.
#        The first match is not returned.  At least two matches are searched
#        for.  A successful result is only returned if only one match is
#        found.  If more than one match is found, a flag is return to
#        indicate that the match was ambiguous.
# @property: The property name to read
# Returns: The property value.  The type depends on the property type.  legacy<>
#          properties are returned as #str.  child<> and link<> properties are
#          returns as #str pathnames.  All integer property types (u8, u16, etc)
#          are returned as #int.
# Since: 1.1
# Notes: This command is experimental and may change syntax in future releases.
{ 'command': 'qom-get',
  'data': { 'path': 'str', 'property': 'str' },
  'returns': 'visitor',
  'gen': 'no' }

I assume you're looking in qmp-commands.hx... At this point, we should just remove all of the docs in that file to avoid future confusion.

Note: I *break* -global isa-fdc.driveA=...  The properties are simply
gone.  Fixable if we need backwards compatibility there.

We do need to make sure we preserve backwards compat here.

The floppy controller device should probably be a child of a super I/O
device, grandchild of a south bridge device, or similar, depending on
the board.  Outside this commit's scope.

I looked through the PIIX4 documentation the other day and it doesn't appear that there is a floppy controller in the PIIX4. I think it must have been an ISA device so I think inheriting from ISA and being a child of /machine makes sense conceptionally.

Signed-off-by: Markus Armbruster<address@hidden>
  hw/fdc.c |  124 +++++++++++++++++++++++++++++++++++--------------------------
  1 files changed, 71 insertions(+), 53 deletions(-)

diff --git a/hw/fdc.c b/hw/fdc.c
index d9c4fbf..98ff87a 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -54,6 +54,33 @@
  /* Floppy drive emulation                               */

+typedef struct FDD {
+    Object obj;
+    BlockDriverState *bs;
+} FDD;
+#define TYPE_FDD "fdd"
+static TypeInfo fdd_info = {
+    .name          = TYPE_FDD,
+    .parent        = TYPE_OBJECT,
+    .instance_size = sizeof(FDD),
+static void fdd_create(Object *fdc, const char *link_name,
+                       BlockDriverState *bs)
+    Object *obj = object_new(TYPE_FDD);
+    FDD *fdd = OBJECT_CHECK(FDD, obj, TYPE_FDD);
+    fdd->bs = bs;
+    object_property_add_child(qdev_get_machine(), "fdd", obj, NULL);
+    object_property_set_link(fdc, obj, link_name, NULL);

This is not quite right. You want to do the actual initialization in instance_init as a method. You should make the BlockDriverState a property too. The fdd_create() call then because object_new() + setting properties only.


Anthony Liguori

reply via email to

[Prev in Thread] Current Thread [Next in Thread]