[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 32/39] blockdev: Implement eject with basic o
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH v7 32/39] blockdev: Implement eject with basic operations |
Date: |
Fri, 23 Oct 2015 16:42:14 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
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 <address@hidden>
>> ---
>> 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, &local_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
- Re: [Qemu-devel] [PATCH v7 31/39] blockdev: Add blockdev-insert-medium, (continued)
Re: [Qemu-devel] [PATCH v7 31/39] blockdev: Add blockdev-insert-medium, Kevin Wolf, 2015/10/23
[Qemu-devel] [PATCH v7 29/39] blockdev: Add blockdev-close-tray, Max Reitz, 2015/10/19
[Qemu-devel] [PATCH v7 32/39] blockdev: Implement eject with basic operations, Max Reitz, 2015/10/19
[Qemu-devel] [PATCH v7 33/39] blockdev: Implement change with basic operations, Max Reitz, 2015/10/19
[Qemu-devel] [PATCH v7 34/39] block: Inquire tray state before tray-moved events, Max Reitz, 2015/10/19
[Qemu-devel] [PATCH v7 35/39] qmp: Introduce blockdev-change-medium, Max Reitz, 2015/10/19
[Qemu-devel] [PATCH v7 37/39] blockdev: read-only-mode for blockdev-change-medium, Max Reitz, 2015/10/19