On 23.10.2015 15:54, Kevin Wolf wrote:
> Am 19.10.2015 um 17:53 hat Max Reitz geschrieben:
>> Implement 'eject' by calling blockdev-open-tray and
>> blockdev-remove-medium.
>>
>> Signed-off-by: Max Reitz
>> ---
>> blockdev.c | 11 +--
>> 1 file changed, 5 insertions(+), 6 deletions(-)
>>
>> diff --git a/blockdev.c b/blockdev.c
>> index a4c278f..0481686 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -1941,16 +1941,15 @@ out:
>>
>> void qmp_eject(const char *device, bool has_force, bool force, Error **errp)
>> {
>> -BlockBackend *blk;
>> +Error *local_err = NULL;
>>
>> -blk = blk_by_name(device);
>> -if (!blk) {
>> -error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
>> - "Device '%s' not found", device);
>> +qmp_blockdev_open_tray(device, has_force, force, _err);
>> +if (local_err) {
>> +error_propagate(errp, local_err);
>> return;
>> }
>
> This changes the behaviour, in the current patch series in two ways if
> the device is locked:
>
> 1. With force, the qmp_blockdev_remove_medium() call will fail because
>we only unlocked the device, but didn't actually open the tray (I
>commented on this in the qmp_blockdev_open_tray() patch). This breaks
>the API, obviously.
Yep, will fix.
> 2. Without force, eject previously sent an eject request and also
>returned a "Device is locked" error. Now it fails with "Tray of
>device is not open". Regression in the message quality, but not an
>API breakage because tools must not parse the message.
I think this should be fine. The previous message wasn't too good in my
opinion either, because the most likely scenario is this: User issues
eject, management tool reports qemu's message: "Device is locked!" and
then the tray opens. So that's strange, too. Maybe "Tray of device is
not open" is actually the better message here, I don't know. It better
describes the state, but it does not describe the reason...
But in addition to that, there is no easy way around this. I could put a
check into qmp_eject() which returns a "Device is locked" message if the
tray is still closed after a successful qmp_blockdev_open_tray(), but I
don't know whether that's worth it.
Max
>> -eject_device(blk, force, errp);
>> +qmp_blockdev_remove_medium(device, errp);
>> }
>
> Kevin
>
signature.asc
Description: OpenPGP digital signature