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 <mre...@redhat.com> >> --- >> 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