Luiz Capitulino <lcapitul...@redhat.com> writes: > I've tried to implement a BLOCK_MEDIUM_EJECT event that, as we discussed[1], > would be emitted by guest-initiated ejects and by the QMP/HMP eject and change > commands. > > However, that turned to be a bit problematic, because the eject and change > commands don't exactly handle tray movements: they actually insert/purge a > medium from from the drive.
The monitor commands are complex, because they do several things, and the exact things they do depends on circumstances. In my experience, it's best to think in basic operations until you understand the problem, then bring in the complex commands. If you bring them in any earlier, you'll get lost in detail and confused. > Consider this example: you have a medium inserted and locked; a first eject > from HMP will tell the guest to eject the medium; if the guest does eject, a > second eject from HMP will just purge the medium (in which case > BLOCK_MEDIUM_EJECT is a bad event to be emitted). > > What we really want to do is to tell mngt that the medium was purged. > > The same is valid for the change command: we want to inform mngt if a medium > was inserted or purged and not emulate tray movements with two eject events > as we discussed[1]. > > So, the solution I came up with is to have two events: > > o GUEST_MEDIUM_EJECTED: emitted when the tray state is changed by the guest > o BLOCK_MEDIUM_CHANGED: emitted when there's a medium change. This should > happen when the eject and change QMP/HMP commands are used I think what I got in mind is close to your proposal, but the thinking that gets me there is different. Let me explain it. The tray can be modelled as a simple state machine. Our problem "notify management app of tray-related events" then becomes "notify on state transition". Tray state is just three bits: open/closed, locked/unlocked, medium/empty. A state transition changes one bit, i.e. we have three pairs of them. There are a few constraints, all obvious: closed -> open requires unlocked, and empty <-> medium require open. The three pairs of state transitions correspond to three pairs of QMP events, each with a boolean argument[*]: one for open <-> closed, one for locked <-> unlocked, and one for medium <-> empty. These events report tray state transitions fully by construction. That's a feature. The medium/empty bit belongs to the block layer. So the block layer should emit the corresponding event. The open/closed bit and the locked/unlocked bit belong to the device model. So the device model should emit the event when it changes the bit. When device models need to call into the block layer whenever they change such a bit, then its fine (even desirable) to put the even emission into the block layer. Note there is no mention whatsoever of what caused the state transition. That's a feature. Now let's compare your proposal to my ideas. Your BLOCK_MEDIUM_CHANGED appears to track my medium <-> empty. You emit it from the block layer's bdrv_dev_change_media_cb(). Called to notify device models whenever the block layer changes media. The "whenever it changes media" part makes it a convenient choke point. Your GUEST_MEDIUM_EJECTED does *not* track my open <-> closed. I think it's more complex than a straight open <-> closed event. Evidence: your event documentation in qmp-events.txt needs an extra note to clarify when exactly the event is emitted. You don't have an event for my locked <-> unlocked, but that's fine. We can always start with a subset of the known-complete set of events, because moving from there to the complete set only involves adding events (easy), not changing events (messy). This is just my analysis of the problem. If folks think your proposal serves their needs, and Kevin is happy with it, I won't object. [*] Or six events without arguments, I don't care.
