From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=36377 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PV0kt-0007Lp-FD for qemu-devel@nongnu.org; Tue, 21 Dec 2010 06:51:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PV0ks-0000Dk-0k for qemu-devel@nongnu.org; Tue, 21 Dec 2010 06:51:47 -0500 Received: from cantor.suse.de ([195.135.220.2]:43919 helo=mx1.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PV0kr-0000DZ-LM for qemu-devel@nongnu.org; Tue, 21 Dec 2010 06:51:45 -0500 Message-ID: <4D1095ED.30903@suse.de> Date: Tue, 21 Dec 2010 12:56:29 +0100 From: Hannes Reinecke MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 06/15] scsi: Update sense code handling References: <1290597370-21365-1-git-send-email-hare@suse.de> <1290597370-21365-7-git-send-email-hare@suse.de> <4CEE73D5.1000207@redhat.com> In-Reply-To: <4CEE73D5.1000207@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: stefanha@gmail.com, qemu-devel@nongnu.org, nab@linux-iscsi.org, kraxel@redhat.com On 11/25/2010 03:33 PM, Kevin Wolf wrote: > Am 24.11.2010 12:16, schrieb Hannes Reinecke: >> The SCSI spec has a quite detailed list of sense codes available. >> It even mandates the use of specific ones for some failure cases. >> The current implementation just has one type of 'generic' error >> which is actually a violation of the spec in certain cases. >> This patch introduces various predefined sense codes to have the >> sense code reporting more in line with the spec. >> >> Signed-off-by: Hannes Reinecke >> Acked-by: Christoph Hellwig >> --- >> hw/scsi-bus.c | 92 ++++++++++++++++++++++++++++++++++++++++++++ >> hw/scsi-disk.c | 109 +++++++++++++++++++++++++++-----------------= --------- >> hw/scsi-generic.c | 76 ++++++++++++++++++++++++++----------- >> hw/scsi.h | 38 ++++++++++++++++++ >> 4 files changed, 239 insertions(+), 76 deletions(-) >> >> diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c >> index 93f0e9a..afdf0ad 100644 >> --- a/hw/scsi-bus.c >> +++ b/hw/scsi-bus.c >> @@ -388,6 +388,98 @@ int scsi_req_parse(SCSIRequest *req, uint8_t *buf= ) >> return 0; >> } >> =20 >> +/* >> + * Predefined sense codes >> + */ >> + >> +/* No sense data available */ >> +const struct SCSISense sense_code_NO_SENSE =3D { >> + .key =3D NO_SENSE , .asc =3D 0x00 , .ascq =3D 0x00 >> +}; >> + >> +/* LUN not ready, Manual intervention required */ >> +const struct SCSISense sense_code_LUN_NOT_READY =3D { >> + .key =3D NOT_READY, .asc =3D 0x04, .ascq =3D 0x03 >> +}; >> + >> +/* LUN not ready, Medium not present */ >> +const struct SCSISense sense_code_NO_MEDIUM =3D { >> + .key =3D NOT_READY, .asc =3D 0x3a, .ascq =3D 0x00 >> +}; >> + >> +/* Hardware error, internal target failure */ >> +const struct SCSISense sense_code_TARGET_FAILURE =3D { >> + .key =3D HARDWARE_ERROR, .asc =3D 0x44, .ascq =3D 0x00 >> +}; >> + >> +/* Illegal request, invalid command operation code */ >> +const struct SCSISense sense_code_INVALID_OPCODE =3D { >> + .key =3D ILLEGAL_REQUEST, .asc =3D 0x20, .ascq =3D 0x00 >> +}; >> + >> +/* Illegal request, LBA out of range */ >> +const struct SCSISense sense_code_LBA_OUT_OF_RANGE =3D { >> + .key =3D ILLEGAL_REQUEST, .asc =3D 0x21, .ascq =3D 0x00 >> +}; >> + >> +/* Illegal request, Invalid field in CDB */ >> +const struct SCSISense sense_code_INVALID_FIELD =3D { >> + .key =3D ILLEGAL_REQUEST, .asc =3D 0x24, .ascq =3D 0x00 >> +}; >> + >> +/* Illegal request, LUN not supported */ >> +const struct SCSISense sense_code_LUN_NOT_SUPPORTED =3D { >> + .key =3D ILLEGAL_REQUEST, .asc =3D 0x25, .ascq =3D 0x00 >> +}; >> + >> +/* Command aborted, I/O process terminated */ >> +const struct SCSISense sense_code_IO_ERROR =3D { >> + .key =3D ABORTED_COMMAND, .asc =3D 0x00, .ascq =3D 0x06 >> +}; >> + >> +/* Command aborted, I_T Nexus loss occurred */ >> +const struct SCSISense sense_code_I_T_NEXUS_LOSS =3D { >> + .key =3D ABORTED_COMMAND, .asc =3D 0x29, .ascq =3D 0x07 >> +}; >> + >> +/* Command aborted, Logical Unit failure */ >> +const struct SCSISense sense_code_LUN_FAILURE =3D { >> + .key =3D ABORTED_COMMAND, .asc =3D 0x3e, .ascq =3D 0x01 >> +}; >> + >> +/* >> + * scsi_build_sense >> + * >> + * Build a sense buffer >> + */ >> +int scsi_build_sense(SCSISense sense, uint8_t *buf, int len, int fixe= d) >> +{ >> + if (len < 8) >> + return 0; >> + if (fixed && len < 14) >> + return 0; >> + >> + memset(buf, 0, len); >> + if (fixed) { >> + /* Return fixed format sense buffer */ >> + buf[0] =3D 0xf0; >> + buf[2] =3D sense.key; >> + buf[7] =3D 7; >> + buf[12] =3D sense.asc; >> + buf[13] =3D sense.ascq; >> + len =3D 14; >=20 > My spec says: "Device servers shall return at least 18 bytes of > parameter data in response to a REQUEST SENSE command if the allocation > length is 18 or greater and the DESC bit is set to zero." >=20 > So should this be MIN(len, 18) instead? >=20 Yes, you are correct. And we should actually always return sense data, even if the length is smaller than the minimum. Fixed in my megasas git tree; will be included in the next round of patches. Cheers, Hannes --=20 Dr. Hannes Reinecke zSeries & Storage hare@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg GF: Markus Rex, HRB 16746 (AG N=FCrnberg)