On Fri, 08/18 08:41, Paolo Bonzini wrote:
> On 18/08/2017 05:38, Fam Zheng wrote:
> > On Thu, 08/17 16:23, Paolo Bonzini wrote:
> >>> + if (scsi_sense_matches(r, SENSE_CODE(NO_MEDIUM))) {
> >>> + error = ENOMEDIUM;
> >>> + } else if (scsi_sense_matches(r, SENSE_CODE(TARGET_FAILURE))) {
> >>> + error = ENOMEM;
> >>> + } else if (scsi_sense_matches(r, SENSE_CODE(INVALID_FIELD))) {
> >>> + error = EINVAL;
> >>> + } else if (scsi_sense_matches(r,
> >>> SENSE_CODE(SPACE_ALLOC_FAILED))) {
> >>> + error = ENOSPC;
> >>> + } else {
> >>> + error = EIO;
> >>
> >> Nice touch :) and in fact the ENOSPC case is needed for correctness.
> >>
> >
> > Offline you mentioned iscsi_translate_sense(). Makes sense to unify both.
> > We can
> > create a util/scsi.c to move it there.
> >
> > About the error/non-error condition, is it enough to add a check "status ==
> > CHECK_CONDITION"?
>
> No, CHECK_CONDITION is for almost every error.
>
> Of those in iscsi_translate_sense:
>
> - DATA_PROTECTION should be an error
>
> - COMMAND_ABORTED should be passed down
>
> - of the various ILLEGAL_REQUEST cases, only ENOMEDIUM and ENOACCES
> should stop. LBA out of range should never happen (see
> scsi_disk_dma_command).
>
> - NOT_READY should also look at asc/ascq individually, like
> ILLEGAL_REQUEST, see below
And the same to DATA PROTECT.
>
> Of those in your list above, these are missing in iscsi_translate_sense:
>
> - SPACE_ALLOC_FAILED (which should stop the VM)
>
> - NOT_READY should also look at asc/ascq individually, like
> ILLEGAL_REQUEST. NO_MEDIUM is returning -EBUSY instead of -ENOMEDIUM.
>
> Among other sense keys, MEDIUM_ERROR and HARDWARE_ERROR probably should
> stop the VM.
Thanks, I'll work on a new version.
Fam
