diff mbox series

[2/2] scsi-generic: Fix HM-zoned device scan

Message ID 20200811225122.17342-3-dmitry.fomichev@wdc.com
State New
Headers show
Series block;scsi-generic: Fix max transfer size calculation | expand

Commit Message

Dmitry Fomichev Aug. 11, 2020, 10:51 p.m. UTC
Several important steps during device scan depend on SCSI type of the
device. For example, max_transfer property is only determined and
assigned if the device has the type of TYPE_DISK.

Host-managed ZBC disks retain most of the properties of regular SCSI
drives, but they have their own SCSI device type, 0x14. This prevents
the proper assignment of max_transfer property for HM-zoned devices in
scsi-generic driver leading to I/O errors if the maximum i/o size
calculated at the guest exceeds the host value.

To fix this, define TYPE_ZBC to have the standard value from SCSI ZBC
standard spec. Several scan steps that were previously done only for
TYPE_DISK devices, are now performed for the SCSI devices having
TYPE_ZBC too.

Reported-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
---
 hw/scsi/scsi-generic.c   | 10 ++++++----
 include/scsi/constants.h |  1 +
 2 files changed, 7 insertions(+), 4 deletions(-)

Comments

Alistair Francis Aug. 17, 2020, 3:58 p.m. UTC | #1
On Tue, Aug 11, 2020 at 3:52 PM Dmitry Fomichev <dmitry.fomichev@wdc.com> wrote:
>
> Several important steps during device scan depend on SCSI type of the
> device. For example, max_transfer property is only determined and
> assigned if the device has the type of TYPE_DISK.
>
> Host-managed ZBC disks retain most of the properties of regular SCSI
> drives, but they have their own SCSI device type, 0x14. This prevents
> the proper assignment of max_transfer property for HM-zoned devices in
> scsi-generic driver leading to I/O errors if the maximum i/o size
> calculated at the guest exceeds the host value.
>
> To fix this, define TYPE_ZBC to have the standard value from SCSI ZBC
> standard spec. Several scan steps that were previously done only for
> TYPE_DISK devices, are now performed for the SCSI devices having
> TYPE_ZBC too.
>
> Reported-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/scsi/scsi-generic.c   | 10 ++++++----
>  include/scsi/constants.h |  1 +
>  2 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
> index 86ed0a3822..2cb23ca891 100644
> --- a/hw/scsi/scsi-generic.c
> +++ b/hw/scsi/scsi-generic.c
> @@ -162,7 +162,8 @@ static void scsi_handle_inquiry_reply(SCSIGenericReq *r, SCSIDevice *s)
>          }
>      }
>
> -    if (s->type == TYPE_DISK && (r->req.cmd.buf[1] & 0x01)) {
> +    if ((s->type == TYPE_DISK || s->type == TYPE_ZBC) &&
> +        (r->req.cmd.buf[1] & 0x01)) {
>          page = r->req.cmd.buf[2];
>          if (page == 0xb0) {
>              uint32_t max_transfer =
> @@ -299,10 +300,11 @@ static void scsi_read_complete(void * opaque, int ret)
>      }
>      blk_set_guest_block_size(s->conf.blk, s->blocksize);
>
> -    /* Patch MODE SENSE device specific parameters if the BDS is opened
> +    /*
> +     * Patch MODE SENSE device specific parameters if the BDS is opened
>       * readonly.
>       */
> -    if ((s->type == TYPE_DISK || s->type == TYPE_TAPE) &&
> +    if ((s->type == TYPE_DISK || s->type == TYPE_TAPE || s->type == TYPE_ZBC) &&
>          blk_is_read_only(s->conf.blk) &&
>          (r->req.cmd.buf[0] == MODE_SENSE ||
>           r->req.cmd.buf[0] == MODE_SENSE_10) &&
> @@ -617,7 +619,7 @@ static void scsi_generic_read_device_identification(SCSIDevice *s)
>  void scsi_generic_read_device_inquiry(SCSIDevice *s)
>  {
>      scsi_generic_read_device_identification(s);
> -    if (s->type == TYPE_DISK) {
> +    if (s->type == TYPE_DISK || s->type == TYPE_ZBC) {
>          scsi_generic_set_vpd_bl_emulation(s);
>      } else {
>          s->needs_vpd_bl_emulation = false;
> diff --git a/include/scsi/constants.h b/include/scsi/constants.h
> index 874176019e..2a32c08b5e 100644
> --- a/include/scsi/constants.h
> +++ b/include/scsi/constants.h
> @@ -218,6 +218,7 @@
>  #define TYPE_ENCLOSURE      0x0d    /* Enclosure Services Device */
>  #define TYPE_RBC            0x0e    /* Simplified Direct-Access Device */
>  #define TYPE_OSD            0x11    /* Object-storage Device */
> +#define TYPE_ZBC            0x14    /* Host-managed Zoned SCSI Device */
>  #define TYPE_WLUN           0x1e    /* Well known LUN */
>  #define TYPE_NOT_PRESENT    0x1f
>  #define TYPE_INACTIVE       0x20
> --
> 2.21.0
>
>
diff mbox series

Patch

diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 86ed0a3822..2cb23ca891 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -162,7 +162,8 @@  static void scsi_handle_inquiry_reply(SCSIGenericReq *r, SCSIDevice *s)
         }
     }
 
-    if (s->type == TYPE_DISK && (r->req.cmd.buf[1] & 0x01)) {
+    if ((s->type == TYPE_DISK || s->type == TYPE_ZBC) &&
+        (r->req.cmd.buf[1] & 0x01)) {
         page = r->req.cmd.buf[2];
         if (page == 0xb0) {
             uint32_t max_transfer =
@@ -299,10 +300,11 @@  static void scsi_read_complete(void * opaque, int ret)
     }
     blk_set_guest_block_size(s->conf.blk, s->blocksize);
 
-    /* Patch MODE SENSE device specific parameters if the BDS is opened
+    /*
+     * Patch MODE SENSE device specific parameters if the BDS is opened
      * readonly.
      */
-    if ((s->type == TYPE_DISK || s->type == TYPE_TAPE) &&
+    if ((s->type == TYPE_DISK || s->type == TYPE_TAPE || s->type == TYPE_ZBC) &&
         blk_is_read_only(s->conf.blk) &&
         (r->req.cmd.buf[0] == MODE_SENSE ||
          r->req.cmd.buf[0] == MODE_SENSE_10) &&
@@ -617,7 +619,7 @@  static void scsi_generic_read_device_identification(SCSIDevice *s)
 void scsi_generic_read_device_inquiry(SCSIDevice *s)
 {
     scsi_generic_read_device_identification(s);
-    if (s->type == TYPE_DISK) {
+    if (s->type == TYPE_DISK || s->type == TYPE_ZBC) {
         scsi_generic_set_vpd_bl_emulation(s);
     } else {
         s->needs_vpd_bl_emulation = false;
diff --git a/include/scsi/constants.h b/include/scsi/constants.h
index 874176019e..2a32c08b5e 100644
--- a/include/scsi/constants.h
+++ b/include/scsi/constants.h
@@ -218,6 +218,7 @@ 
 #define TYPE_ENCLOSURE      0x0d    /* Enclosure Services Device */
 #define TYPE_RBC            0x0e    /* Simplified Direct-Access Device */
 #define TYPE_OSD            0x11    /* Object-storage Device */
+#define TYPE_ZBC            0x14    /* Host-managed Zoned SCSI Device */
 #define TYPE_WLUN           0x1e    /* Well known LUN */
 #define TYPE_NOT_PRESENT    0x1f
 #define TYPE_INACTIVE       0x20