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
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.
Paolo
