Re: [Qemu-devel] [RFC 0/5]: QMP: Introduce GUEST_MEDIUM_EJECT & BLOCK_MEDIUM_CHANGED

[Markus Armbruster]

Luiz Capitulino <address@hidden> 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.