Re: [Qemu-block] [PATCH] Report error when opening device with locked tray

[Kevin Wolf]

Am 07.06.2016 um 22:00 hat John Snow geschrieben:
> 
> 
> On 06/07/2016 06:28 AM, Kevin Wolf wrote:
> > Am 06.06.2016 um 21:40 hat Colin Lord geschrieben:
> >> This commit causes qmp_blockdev_change_medium to report an error if an
> >> attempt is made to open a device with a locked tray.
> > 
> > The old behaviour is that the command seemingly succeeds, but the medium
> > isn't actually changed. Correct?
> > 
> 
> Close. Old "change" command also fails, but with a confusing error.
> 
> > Should this be mentioned in the commit message? You just describe what
> > you change, but not why.
> > 
> 
> Old behavior:
> 
> - Change uses qmp_blockdev_open_tray, which "succeeds."
> - Change then tries to use qmp_x_blockdev_remove_medium, but receives
> potentially confusing error "Tray is locked."
> - Moments later, the tray is likely now open.

Ah, yes, makes sense. This and that we want to have a less confusing
error message is the part that is missing in the commit message.

Kevin

> New behavior:
> 
> - Change uses do_open_tray, which returns -EINPROGRESS.
> - Change can propagate this error upwards without attempting to remove
> the medium.
> - User gets "Device <foo> is locked and force was not specified, wait
> for tray to open and try again" error.
> 
> Why: "The new error tries to inform the user that there is an action
> pending and that the command, if run again, may succeed."
> 
> >> Signed-off-by: Colin Lord <address@hidden>
> >> This is based off my previous patch regarding the do_open_tray function
> >> (currently at v3). Probably should have been submitted as a patch set
> >> but I wasn't thinking that far ahead when I submitted the first patch.
> >> ---
> >>  blockdev.c | 7 +++++--
> >>  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > Yes, would probably have made sense as a series, but as long as it's
> > only two patches, it's not really a problem.
> > 
> > Please make sure to put such comments below the "---" line, though, i.e.
> > comments that make sense for the review, but not as part of the commit
> > log. Then git-am automatically removes that part from the commit message
> > while applying the patch. I did it manually for this one now.
> > 
> > Kevin
> > 
> 
> -- 
> —js