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

[Luiz Capitulino <lcapitulino@redhat.com>]

On Fri, 10 Feb 2012 10:27:21 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
> 
> > On Thu, 09 Feb 2012 16:01:21 +0100
> > Markus Armbruster <armbru@redhat.com> wrote:
> >
> >> Luiz Capitulino <lcapitulino@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.
> [...]
> >> 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.
> 
> What's complex about an event that fires when the tray moves?  Isn't
> that's as simple as it gets?  The complexity is purely in the two
> monitor commands.  Adding events doesn't increase that complexity, it
> merely makes it easier to observe.

In principle this is completely right, but in practice I see a few problems
because the change and eject commands require the event to be emitted as
special cases (as those commands don't always move the tray).

But note that I'm not saying this is undoable, just saying that I don't
like as much as I like emitting the event only when the guest moves the tray.

> I feel strongly that we shouldn't design events around monitor
> commands[*].  Especially not complex, messy monitor commands that'll
> have to be broken up for QMP eventually.  Events should be about
> noteworthy state transitions.

Well, if we break eject/change into multiple commands and each command does
a single state transition, then we end up doing what you're saying, except
that no explicit state machine code exists (like the RunState code).

> Fundamental state machines such as the one modelling tray behavior don't
> change much.  But triggers for their state transitions are less stable.
> I expect an event that just reports a state transition to age more
> gracefully than one that is also tied to triggers, such as "the guest
> did it".
> 
> >> 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.
> 
> 
> [*] I'm oversimplifying.  Events are *usually* about noteworthy state
> transitions.  But we sometimes use the event mechanism for asynchronous
> replies to synchronous monitor commands, like in your "QMP: add
> balloon-get-memory-stats command" series.  That's fine.
>