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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.