On Thu, 09 Feb 2012 16:01:21 +0100 Markus Armbruster <arm...@redhat.com> wrote:
> 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. The purpose of the note is not to clarify, but to emphasize that the event is about guest initiated ejects. Also, I disagree it's more complex, actually I think it's quite simple vs. emitting the eject event from the eject and change commands, as this can be messy because of their complicated semantics. > 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. No feedback so far.
