Re: [PATCH-for-5.2 7/7] hw/block/fdc: Add ASCII art schema of QOM relations

[Kevin Wolf]

Am 06.08.2020 um 12:33 hat Philippe Mathieu-Daudé geschrieben:
> On 8/6/20 10:57 AM, Kevin Wolf wrote:
> > Am 06.08.2020 um 10:08 hat Philippe Mathieu-Daudé geschrieben:
> >> Without knowing the QEMU history, it is hard to relate QEMU objects
> >> with the hardware datasheet.
> >>
> >> For example, one naively expects:
> >>
> >> * a floppy disk is plugged / unplugged on the bus
> >>
> >>   Wrong! QEMU floppy disks always sit on the bus. The block drives
> >>   are plugged / unplugged on the disks, and the disks magically
> >>   re-adapt their proprieties to match the block drive.
> > 
> > This is because what sits on the bus is not a floppy disk, but a floppy
> > drive. FloppyDrive is also what the type is called.
> > 
> > The disk is represented by the BlockDriverState (the actual image file)
> > that is inserted in the BlockBackend (which is logically part of the
> > drive).
> > 
> >> * a floppy controller has a fixed number of disks pluggable on the bus
> >>
> >>   Wrong! QEMU floppy controllers have as much slots as the number of
> >>   floppy drive provided when a machine is created. Then the ACPI table
> >>   are generated and the number of slots can not be modified. So if you
> >>   expect a dual slot controller being created with slot A and B, if
> >>   the machine is created with a single drive attached, the controller
> >>   will only have slot A created, and you will never be able to plug
> >>   drive B without risking a mismatch in the ACPI tables.
> > 
> > Hm... I guess hotplugging floppy drives might actually have worked,
> > though I have never tried it on real hardware. I'm pretty sure it wasn't
> > an official feature, though, and ACPI tables certainly won't magically
> > change if you do this because (apart from polling, I guess) software has
> > no way to detect that you tinkered with the floppy cable. :-)
> > 
> >> * a floppy controller supporting 4 disks uses 2 buses
> >>
> >>   Wrong! QEMU uses a single bus to plug the 4 disks.
> > 
> > But we don't even emulate floppy controllers that can have more than two
> > floppy drives:
> > 
> >     $ x86_64-softmmu/qemu-system-x86_64 -device floppy -device floppy 
> > -device floppy
> >     qemu-system-x86_64: -device floppy: Can't create floppy unit 2, bus 
> > supports only 2 units
> 
> This comment is for developers, the warning is for user.
> 
> It comes from:
> 
>     if (dev->unit >= MAX_FD) {
>         error_setg(errp, "Can't create floppy unit %d, bus supports "
>                    "only %d units", dev->unit, MAX_FD);
>         return;
>     }
> 
> But you can compile QEMU with MAX_FD=4:
> [...]

Ah, I wasn't aware that this was supposed to be changed.

I don't even have the spec here for a floppy controller that supports
four drives. My copy has "reserved" for those bits that apparently refer
to the additional drives (if the QEMU code is right).

> > This is checked in floppy_drive_realize(), so it applies to all
> > variants of the controller.
> > 
> > If you want more floppy drives, you have to create a second controller
> > (with a different iobase). Though I don't think I actually got this
> > working when I tried. I wasn't sure if the problem was the emulation or
> > the guest OSes (or SeaBIOS actually for DOS).
> > 
> >> As all these false assumptions are not obvious (we don't plug a disk,
> >> we plug a block drive into a disk, etc...), start documenting the QOM
> >> relationships with a simple ASCII schema.
> > 
> > Maybe be more specific to have: "floppy (drive)" and "blk (disk)".
> > Because the ASCII schema is actually true, though you seem to have
> > misunderstood what each item in it is supposed to represent.
> > 
> > Actually "blk (disk)" is not 100% accurate either because the drive
> > always has a BlockBackend present. It's really the BlockDriverState
> > inserted into the BlockBackend that is the disk.
> > 
> > In summary, to be honest, I believe since its qdevification, floppy is
> > one of the block devices that is modelled the best on the QOM + block
> > backend level. Only SCSI might be comparable, but IDE, virtio-blk and
> > usb-storage are a mess in comparison.
> 
> I'm sorry I didn't want to criticize the model or hurt you, I just want
> to note the differences between how the controller is described in the
> Intel 82078 datasheet and how the QEMU model works. Maybe I'm wrong
> assuming there would be a 1:1 match.

Sorry for not being clear. This isn't about criticism at all. I just
wanted to suggest that if you're trying to find out how to model new
devices, looking at floppy is probably a better start than looking at
most other block devices.

I don't know the floppy emulation well enough to say much about the
MAX_FD == 4 case, but the basic separation into controller and drive,
and that removable media are not represented in qdev, but just as block
nodes inserted into the BlockBackend of the drive, feels like the right
approach.

> I'll repost with the name updated in the schema and removing my
> assumptions from the commit description that appears as simple
> critics.

I think the most important part to fix in the commit message would be
the confusion between drives and disks.

Kevin