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

[Markus Armbruster]

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.

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.

[...]