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

[Markus Armbruster]

Paolo Bonzini <address@hidden> writes:

> On 02/16/2012 07:30 PM, Markus Armbruster wrote:
>> 3. Physical load/eject with pass-through device
>> 
>> The host_cdrom backend's tray handling is rudimentary.  But let me
>> sketch how a real implementation could work.
>> 
>> 3a. User closes tray, or opens unlocked tray
>> 
>> Backend gets notified, calls into block layer.  Block layer notifies
>> device model.  Device model notifies guest OS.
>> 
>> 3b. User presses button while tray is closed and locked
>> 
>> Backend gets notified, calls into block layer.  Block layer notifies
>> device model.  Device model notifies guest OS.  Guest OS may command
>> tray open.  Goto 1.
>> 
>> We should be able to emit suitable events in the block layer.
>
> Here is how I had implemented it; it is similar enough:
>
> 3a. User closes tray, or opens unlocked tray
>
> The device model polls the block layer that in turn asks the backend.
> Device model notifies guest OS.
>
> The block layer could look at the results of polling before passing them
> to the device model and emit events.
>
> 3b. User presses button while tray is closed and locked
>
> If the guest doesn't poll the device model, there is nothing to do.
>
> If the guest polls the device model, the device model passes the request
> to the block layer that in turn asks the backend.  But this doesn't
> generate any event unless the guest OS commands tray open.  If it does,
> goto 1.

Yes, that's just active polling instead of passive event reception,
i.e. similar enough.

For device models that can signal the guest (usually IRQ), polling
introduces latency, though.  Polling frequently enough to hide the
latency can waste a bit of power.  Anyway, no need to worry about it as
long as it doesn't bother users.

>> I figure Luiz's patch works.  But maybe it could be simplified some by
>> replacing bdrv_dev_change_media_cb() by a "open/close tray" callback
>> that returns whether it moved.  bdrv_dev_change_media_cb() would then
>> simply open the tray, emit event if it moved, close the tray, emit event
>> if it moved.
>
> The device models' change_media_cb is already effectively an open/close
> tray callback.  You do not need a check in the callback, you can simply
> call the is_tray_open callback.  bdrv_dev_change_media_cb can stop
> calling the callback unless the tray moved, and
> eject_device+qmp_bdrv_open_encrypted can call bdrv_dev_change_media_cb
> respectively to open and close the tray.  This would also fix the wart
> with no medium, IIUC.

I'd like to see this change, too.  Follow-up patch would be fine with
me.

>> [*] Can happen only in the warty case.  Or when the guest closes the
>> tray before us.  I'm not sure that works.
>
> The guest cannot beat us to this race because the monitor and MMIO are
> both protected by the global mutex.

Good to know, thanks!