Re: [Qemu-devel] [PATCH v7 28/39] blockdev: Add blockdev-open-tray

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3450 bytes --]

On 23.10.2015 15:22, Kevin Wolf wrote:
> Am 19.10.2015 um 17:53 hat Max Reitz geschrieben:
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>  blockdev.c           | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  qapi/block-core.json | 23 +++++++++++++++++++++++
>>  qmp-commands.hx      | 39 +++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 111 insertions(+)
> 
>> +
>> +    if (blk_dev_is_medium_locked(blk)) {
>> +        blk_dev_eject_request(blk, force);
>> +    } else {
>> +        blk_dev_change_media_cb(blk, false);
>> +    }
> 
> This seems to be inconsistent with the command documentation: In the
> case of a forced eject request, the tray is just unlocked, but not
> opened.

OK. I assumed that since the guest would be expected to open the tray by
itself if @force was false, the emulated device would do the same if
@force was true.

So I'll call blk_dev_change_media_cb() after blk_dev_eject_request(),
too, if @force is true.

>> +##
>> +# @blockdev-open-tray:
>> +#
>> +# Opens a block device's tray. If there is a block driver state tree inserted as
>> +# a medium, it will become inaccessible to the guest (but it will remain
>> +# associated to the block device, so closing the tray will make it accessible
>> +# again).
>> +#
>> +# If the tray was already open before, this will be a no-op.
>> +#
>> +# @device: block device name
>> +#
>> +# @force:  #optional if false (the default), an eject request will be sent to
>> +#          the guest if it has locked the tray (and the tray will not be opened
>> +#          immediately); if true, the tray will be opened regardless of whether
>> +#          it is locked
>> +#
>> +# Since: 2.5
>> +##
>> +{ 'command': 'blockdev-open-tray',
>> +  'data': { 'device': 'str',
>> +            '*force': 'bool' } }
> 
> This API makes it impossible for the management application to know
> whether the tray has actually been opened or just an eject request has
> been made.

Yes, and I faintly remember having asked you and Markus on your opinion
on this topic in last December. A related question I was even more
concerned about was "If the tray is locked and an eject request has been
sent, the eject issued by the guest may do something different than
blockdev-open-tray would have done with an unlocked tray", with the
issue being that the management application cannot prevent
blockdev-open-tray from sending the unlock request.

I considered actually making blockdev-open-tray and this
"send-eject-request" two different commands, so that blockdev-open-tray
would never send an eject request and just return an error if the tray
is locked. As far as I remember, you (both?) convinced me to keep it a
single command which would do what "pressing the button on a physical CD
drive does".

So I did that, and thus it's the same. Just when pressing that button,
after sending this command you'll have to wait until the tray is open
and you won't know whether the OS actually has any intention of doing so
until you've waited for too long.

> Is the expected use that management sets a timeout and waits for a
> tray opened event? If so, worth documenting?

That's probably what management software should do, yes, just like it is
with the "eject" command right now. I considered the "the tray will not
be opened immediately" enough, but feel free to tell me it is not.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]