Re: [Qemu-devel] [PATCH] qmp: add BLOCK_MEDIUM_EJECT event

[Luiz Capitulino]

On Tue, 31 Jan 2012 10:23:59 +0100
Markus Armbruster <address@hidden> wrote:

> Kevin Wolf <address@hidden> writes:
> 
> > Am 30.01.2012 16:18, schrieb Luiz Capitulino:
> >> On Fri, 27 Jan 2012 10:52:15 +0100
> >> Kevin Wolf <address@hidden> wrote:
> >> 
> >>> Am 26.01.2012 18:57, schrieb Luiz Capitulino:
> >>>> On Wed, 25 Jan 2012 10:42:04 -0200
> >>>> Luiz Capitulino <address@hidden> wrote:
> >>>>
> >>>>> On Wed, 25 Jan 2012 09:41:20 +0100
> >>>>> Kevin Wolf <address@hidden> wrote:
> >>>>>
> >>>>>> Am 24.01.2012 21:03, schrieb Eric Blake:
> >>>>>>> On 01/24/2012 11:16 AM, Luiz Capitulino wrote:
> >>>>>>>> Libvirt wants to be notified when the guest ejects a medium, so that
> >>>>>>>> it can update its view of the guest.
> >>>>>>>>
> >>>>>>>> This code has been originally written by Daniel Berrange. It adds
> >>>>>>>> the event to IDE and SCSI emulation.
> >>>>>>>>
> >>>>>>>> Please, note that this only covers guest initiated ejects, that's,
> >>>>>>>> the QMP/HMP commands 'eject' and 'change' are not covered.
> >>>>>>
> >>>>>> What's the reason for this behaviour? It feels inconsistent.
> >>>>>
> >>>>> I don't think it's inconsistent because we're limiting it to guest 
> >>>>> initiated
> >>>>> actions. Also, the mngt app knows when it sends a 'eject' or 'change' 
> >>>>> command.
> >>>>> The exception is if it allows HMP to run in parallel with QMP, but I 
> >>>>> don't think
> >>>>> this is recommended (at least not for commands that change any VM 
> >>>>> state).
> >>>>
> >>>> Let me elaborate more. We have two options:
> >>>>
> >>>>  1. Emit the event for guest initiated ejects (this patch, although I 
> >>>> think
> >>>>     the event should be renamed to GUEST_MEDIUM_EJECT)
> >>>>
> >>>>  2. Emit the event for guest & QMP initiated ejects, that's, also emit 
> >>>> the
> >>>>     event for the eject and change commands
> >>>>
> >>>> The first thing to note is that, they're not mutually exclusive. If we do
> >>>> item 1 now, we can add 2 later (as a different event).
> >>>>
> >>>> But my point is that doing 2 is problematic and we should avoid it. The 
> >>>> problem
> >>>> is that the semantics of eject and change are complex and/or buggy. And 
> >>>> something
> >>>> I've learned about confusing semantics in QMP is that, they will give 
> >>>> you headaches
> >>>> soon or later.
> >>>
> >>> But I'm not really interested in the semantics of QMP commands, because
> >>> they are irrelevant for the events.
> >> 
> >> I do think they are relevant, because the event will have to match what
> >> the eject/change commands do with the tray. If what they do is messy, the
> >> event will turn out to be messy too.
> >> 
> >> Now, I don't doubt this can be all fixed and made clean. I'm just not sure
> >> if it's worth it.
> >
> > If a mess best describes to the outside what we're doing to the device,
> > then having a messy event is the best you can expect. Or in other words,
> > if you're doing messy things with the device and you straighten things
> > out in the event generation, then your events are lying to the
> > management tools.
> 
> Yup.  The event is merely a passive sensor.  If reality is messy, sensor
> data better reflect that.
> 
> Maybe it's easier to understand from a distance, so let's examine a more
> distant example: filesystem event monitoring with inotify(),
> specifically file permissions change.  The event is simple enough: you
> get it when file permissions change.  Now imagine chmod(2) was
> "improved" to refuse to set write bits unless read bits are also set,
> but only on Tuedays.  That's a messy chmod() indeed.  But the event
> remains as simple and clean as ever: you still get it when file
> permissions change.
> 
> Back to QMP.  In my opinion, the simple and clean event is "tray moved".
> Emit it when the tray moves, regardless of what made it move.

I'll give it a tray. Oh, I meant a try :)

> 
> Yes, the management app gets the event even when it itself directly
> triggered the move by commanding a media eject.  That's a feature.  It
> also gets the event when its media eject command actually becomes a
> polite request to the guest OS, because the tray happens to be locked,
> and the guest OS complies.  That's a feature, too.
> 
> [...]
>