* [Qemu-devel] [PATCH] scsi: provide general-purpose functions to manage sense data
@ 2017-12-17 5:29 Paolo Bonzini
2017-12-18 10:47 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 2+ messages in thread
From: Paolo Bonzini @ 2017-12-17 5:29 UTC (permalink / raw)
To: qemu-devel; +Cc: dgilbert
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>
---
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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [Qemu-devel] [PATCH] scsi: provide general-purpose functions to manage sense data
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
0 siblings, 0 replies; 2+ messages in thread
From: Dr. David Alan Gilbert @ 2017-12-18 10:47 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
* 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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-12-18 10:47 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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 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).