From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] scsi: provide general-purpose functions to manage sense data
Date: Mon, 18 Dec 2017 10:47:14 +0000 [thread overview]
Message-ID: <20171218104713.GD2440@work-vm> (raw)
In-Reply-To: <20171217052945.12782-1-pbonzini@redhat.com>
* Paolo Bonzini (pbonzini@redhat.com) wrote:
> Extract the common parts of scsi_sense_buf_to_errno, scsi_convert_sense
> and scsi_target_send_command's REQUEST SENSE handling into two new
> functions scsi_parse_sense_buf and scsi_build_sense_buf.
>
> Fix a bug in scsi_target_send_command along the way; the length was
> written in buf[10] rather than buf[7].
>
> Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Fixes: b07fbce634 ("scsi-bus: correct responses for INQUIRY and REQUEST SENSE")
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> hw/scsi/scsi-bus.c | 16 +-----
> include/scsi/utils.h | 3 ++
> scsi/utils.c | 139 +++++++++++++++++++++++++--------------------------
> 3 files changed, 72 insertions(+), 86 deletions(-)
>
> diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
> index 977f7bce1f..965becf31f 100644
> --- a/hw/scsi/scsi-bus.c
> +++ b/hw/scsi/scsi-bus.c
> @@ -540,20 +540,8 @@ static int32_t scsi_target_send_command(SCSIRequest *req, uint8_t *buf)
> if (req->lun != 0) {
> const struct SCSISense sense = SENSE_CODE(LUN_NOT_SUPPORTED);
>
> - if (fixed_sense) {
> - r->buf[0] = 0x70;
> - r->buf[2] = sense.key;
> - r->buf[10] = 10;
> - r->buf[12] = sense.asc;
> - r->buf[13] = sense.ascq;
> - r->len = MIN(req->cmd.xfer, SCSI_SENSE_LEN);
> - } else {
> - r->buf[0] = 0x72;
> - r->buf[1] = sense.key;
> - r->buf[2] = sense.asc;
> - r->buf[3] = sense.ascq;
> - r->len = 8;
> - }
> + r->len = scsi_build_sense_buf(r->buf, req->cmd.xfer,
> + sense, fixed_sense);
> } else {
> r->len = scsi_device_get_sense(r->req.dev, r->buf,
> MIN(req->cmd.xfer, r->buf_len),
> diff --git a/include/scsi/utils.h b/include/scsi/utils.h
> index eb07e474ee..4b705f5e0f 100644
> --- a/include/scsi/utils.h
> +++ b/include/scsi/utils.h
> @@ -31,6 +31,9 @@ typedef struct SCSISense {
> } SCSISense;
>
> int scsi_build_sense(uint8_t *buf, SCSISense sense);
> +SCSISense scsi_parse_sense_buf(const uint8_t *in_buf, int in_len);
> +int scsi_build_sense_buf(uint8_t *buf, size_t max_size, SCSISense sense,
> + bool fixed_sense);
>
> /*
> * Predefined sense codes
> diff --git a/scsi/utils.c b/scsi/utils.c
> index e4182a9b09..61bc1a8e2b 100644
> --- a/scsi/utils.c
> +++ b/scsi/utils.c
> @@ -96,15 +96,60 @@ int scsi_cdb_length(uint8_t *buf)
> return cdb_len;
> }
>
> +SCSISense scsi_parse_sense_buf(const uint8_t *in_buf, int in_len)
> +{
> + bool fixed_in;
> + SCSISense sense;
> +
> + assert(in_len > 0);
> + fixed_in = (in_buf[0] & 2) == 0;
> + if (fixed_in) {
> + if (in_len < 14) {
> + return SENSE_CODE(IO_ERROR);
> + }
> + sense.key = in_buf[2];
> + sense.asc = in_buf[12];
> + sense.ascq = in_buf[13];
> + } else {
> + if (in_len < 4) {
> + return SENSE_CODE(IO_ERROR);
> + }
> + sense.key = in_buf[1];
> + sense.asc = in_buf[2];
> + sense.ascq = in_buf[3];
> + }
> +
> + return sense;
> +}
> +
> +int scsi_build_sense_buf(uint8_t *out_buf, size_t size, SCSISense sense,
> + bool fixed_sense)
> +{
> + int len;
> + uint8_t buf[SCSI_SENSE_LEN] = { 0 };
> +
> + if (fixed_sense) {
> + buf[0] = 0x70;
> + buf[2] = sense.key;
> + buf[7] = 10;
> + buf[12] = sense.asc;
> + buf[13] = sense.ascq;
> + len = 18;
> + } else {
> + buf[0] = 0x72;
> + buf[1] = sense.key;
> + buf[2] = sense.asc;
> + buf[3] = sense.ascq;
> + len = 8;
> + }
> + len = MIN(len, size);
> + memcpy(out_buf, buf, len);
> + return len;
> +}
> +
> int scsi_build_sense(uint8_t *buf, SCSISense sense)
> {
> - memset(buf, 0, 18);
> - buf[0] = 0x70;
> - buf[2] = sense.key;
> - buf[7] = 10;
> - buf[12] = sense.asc;
> - buf[13] = sense.ascq;
> - return 18;
> + return scsi_build_sense_buf(buf, SCSI_SENSE_LEN, sense, true);
> }
>
> /*
> @@ -274,52 +319,21 @@ const struct SCSISense sense_code_SPACE_ALLOC_FAILED = {
> int scsi_convert_sense(uint8_t *in_buf, int in_len,
> uint8_t *buf, int len, bool fixed)
> {
> - bool fixed_in;
> SCSISense sense;
> - if (!fixed && len < 8) {
> - return 0;
> - }
> -
> - if (in_len == 0) {
> - sense.key = NO_SENSE;
> - sense.asc = 0;
> - sense.ascq = 0;
> - } else {
> - fixed_in = (in_buf[0] & 2) == 0;
> -
> - if (fixed == fixed_in) {
> - memcpy(buf, in_buf, MIN(len, in_len));
> - return MIN(len, in_len);
> - }
> + bool fixed_in;
>
> - if (fixed_in) {
> - sense.key = in_buf[2];
> - sense.asc = in_buf[12];
> - sense.ascq = in_buf[13];
> - } else {
> - sense.key = in_buf[1];
> - sense.asc = in_buf[2];
> - sense.ascq = in_buf[3];
> - }
> + fixed_in = (in_buf[0] & 2) == 0;
> + if (in_len && fixed == fixed_in) {
> + memcpy(buf, in_buf, MIN(len, in_len));
> + return MIN(len, in_len);
> }
>
> - memset(buf, 0, len);
> - if (fixed) {
> - /* Return fixed format sense buffer */
> - buf[0] = 0x70;
> - buf[2] = sense.key;
> - buf[7] = 10;
> - buf[12] = sense.asc;
> - buf[13] = sense.ascq;
> - return MIN(len, SCSI_SENSE_LEN);
> + if (in_len == 0) {
> + sense = SENSE_CODE(NO_SENSE);
> } else {
> - /* Return descriptor format sense buffer */
> - buf[0] = 0x72;
> - buf[1] = sense.key;
> - buf[2] = sense.asc;
> - buf[3] = sense.ascq;
> - return 8;
> + sense = scsi_parse_sense_buf(in_buf, in_len);
> }
> + return scsi_build_sense_buf(buf, len, sense, fixed);
> }
>
> int scsi_sense_to_errno(int key, int asc, int ascq)
> @@ -366,34 +380,15 @@ int scsi_sense_to_errno(int key, int asc, int ascq)
> }
> }
>
> -int scsi_sense_buf_to_errno(const uint8_t *sense, size_t sense_size)
> +int scsi_sense_buf_to_errno(const uint8_t *in_buf, size_t in_len)
> {
> - int key, asc, ascq;
> - if (sense_size < 1) {
> - return EIO;
> - }
> - switch (sense[0]) {
> - case 0x70: /* Fixed format sense data. */
> - if (sense_size < 14) {
> - return EIO;
> - }
> - key = sense[2] & 0xF;
> - asc = sense[12];
> - ascq = sense[13];
> - break;
> - case 0x72: /* Descriptor format sense data. */
> - if (sense_size < 4) {
> - return EIO;
> - }
> - key = sense[1] & 0xF;
> - asc = sense[2];
> - ascq = sense[3];
> - break;
> - default:
> + SCSISense sense;
> + if (in_len < 1) {
> return EIO;
> - break;
> }
> - return scsi_sense_to_errno(key, asc, ascq);
> +
> + sense = scsi_parse_sense_buf(in_buf, in_len);
> + return scsi_sense_to_errno(sense.key, sense.asc, sense.ascq);
> }
>
> const char *scsi_command_name(uint8_t cmd)
> --
> 2.14.3
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
prev parent reply other threads:[~2017-12-18 10:47 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-17 5:29 [Qemu-devel] [PATCH] scsi: provide general-purpose functions to manage sense data Paolo Bonzini
2017-12-18 10:47 ` Dr. David Alan Gilbert [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171218104713.GD2440@work-vm \
--to=dgilbert@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).