diff mbox

[PATCHv2,4/4] scsi: Add 'enclosure' option for scsi devices

Message ID 1501835795-92331-5-git-send-email-hare@suse.de (mailing list archive)
State New, archived
Headers show

Commit Message

Hannes Reinecke Aug. 4, 2017, 8:36 a.m. UTC 
Make enclosure support optional with the 'enclosure' argument to
the scsi device.
Adding 'enclosure=on' as option to the SCSI device will present
an enclosure service device on LUN0, either as a stand-alone
LUN (in case LUN0 is not assigned) or by setting the EncServ bit
int the inquiry data if LUN0 is assigned to a block devices.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 hw/scsi/scsi-bus.c     | 11 ++++++++---
 hw/scsi/scsi-disk.c    |  4 +++-
 include/hw/scsi/scsi.h |  1 +
 3 files changed, 12 insertions(+), 4 deletions(-)

Comments

Paolo Bonzini Aug. 4, 2017, 10:48 a.m. UTC | #1 
On 04/08/2017 10:36, Hannes Reinecke wrote:
> Make enclosure support optional with the 'enclosure' argument to
> the scsi device.
> Adding 'enclosure=on' as option to the SCSI device will present
> an enclosure service device on LUN0, either as a stand-alone
> LUN (in case LUN0 is not assigned) or by setting the EncServ bit
> int the inquiry data if LUN0 is assigned to a block devices.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
>  hw/scsi/scsi-bus.c     | 11 ++++++++---
>  hw/scsi/scsi-disk.c    |  4 +++-
>  include/hw/scsi/scsi.h |  1 +
>  3 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
> index 79a222f..c11422b 100644
> --- a/hw/scsi/scsi-bus.c
> +++ b/hw/scsi/scsi-bus.c
> @@ -22,6 +22,7 @@ static Property scsi_props[] = {
>      DEFINE_PROP_UINT32("channel", SCSIDevice, channel, 0),
>      DEFINE_PROP_UINT32("scsi-id", SCSIDevice, id, -1),
>      DEFINE_PROP_UINT32("lun", SCSIDevice, lun, -1),
> +    DEFINE_PROP_BOOL("enclosure", SCSIDevice, enclosure, false),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> @@ -494,11 +495,14 @@ static bool scsi_target_emulate_inquiry(SCSITargetReq *r)
>      if (r->req.lun != 0) {
>          r->buf[0] = TYPE_NO_LUN;
>      } else {
> -        r->buf[0] = TYPE_ENCLOSURE;
> +        r->buf[0] = r->req.dev->enclosure ?
> +            TYPE_ENCLOSURE : TYPE_NOT_PRESENT | TYPE_INACTIVE;
>          r->buf[2] = 5; /* Version */
>          r->buf[3] = 2 | 0x10; /* HiSup, response data format */
>          r->buf[4] = r->len - 5; /* Additional Length = (Len - 1) - 4 */
> -        r->buf[6] = 0x40; /* Enclosure service */
> +        if (r->req.dev->enclosure) {
> +            r->buf[6] = 0x40; /* Enclosure service */
> +        }
>          r->buf[7] = 0x10 | (r->req.bus->info->tcq ? 0x02 : 0); /* Sync, TCQ.  */
>          memcpy(&r->buf[8], "QEMU    ", 8);
>          memcpy(&r->buf[16], "QEMU TARGET     ", 16);
> @@ -600,7 +604,8 @@ static int32_t scsi_target_send_command(SCSIRequest *req, uint8_t *buf)
>          }
>          break;
>      case RECEIVE_DIAGNOSTIC:
> -        if (!scsi_target_emulate_receive_diagnostic(r)) {
> +        if (!r->req.dev->enclosure ||
> +            !scsi_target_emulate_receive_diagnostic(r)) {
>              goto illegal_request;
>          }
>          break;
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index 5f1e5e8..153d97d 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -792,7 +792,9 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
>                 the additional length is not adjusted */
>          outbuf[4] = 36 - 5;
>      }
> -
> +    if (s->qdev.lun == 0 && s->qdev.enclosure) {
> +            outbuf[6] = 0x40; /* Enclosure service */
> +    }

Should this really be set on disks even if you do have a LUN0?

Why don't you instead add a very simple scsi-enclosure device that you
can specify as the LUN0?

Thanks,

Paolo

>      /* Sync data transfer and TCQ.  */
>      outbuf[7] = 0x10 | (req->bus->info->tcq ? 0x02 : 0);
>      return buflen;
> diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
> index 6b85786..243c185 100644
> --- a/include/hw/scsi/scsi.h
> +++ b/include/hw/scsi/scsi.h
> @@ -100,6 +100,7 @@ struct SCSIDevice
>      BlockConf conf;
>      SCSISense unit_attention;
>      bool sense_is_ua;
> +    bool enclosure;
>      uint8_t sense[SCSI_SENSE_BUF_SIZE];
>      uint32_t sense_len;
>      QTAILQ_HEAD(, SCSIRequest) requests;
>
Hannes Reinecke Aug. 4, 2017, 11:54 a.m. UTC | #2 
On 08/04/2017 12:48 PM, Paolo Bonzini wrote:
> On 04/08/2017 10:36, Hannes Reinecke wrote:
>> Make enclosure support optional with the 'enclosure' argument to
>> the scsi device.
>> Adding 'enclosure=on' as option to the SCSI device will present
>> an enclosure service device on LUN0, either as a stand-alone
>> LUN (in case LUN0 is not assigned) or by setting the EncServ bit
>> int the inquiry data if LUN0 is assigned to a block devices.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.com>
>> ---
>>  hw/scsi/scsi-bus.c     | 11 ++++++++---
>>  hw/scsi/scsi-disk.c    |  4 +++-
>>  include/hw/scsi/scsi.h |  1 +
>>  3 files changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
>> index 79a222f..c11422b 100644
>> --- a/hw/scsi/scsi-bus.c
>> +++ b/hw/scsi/scsi-bus.c
>> @@ -22,6 +22,7 @@ static Property scsi_props[] = {
>>      DEFINE_PROP_UINT32("channel", SCSIDevice, channel, 0),
>>      DEFINE_PROP_UINT32("scsi-id", SCSIDevice, id, -1),
>>      DEFINE_PROP_UINT32("lun", SCSIDevice, lun, -1),
>> +    DEFINE_PROP_BOOL("enclosure", SCSIDevice, enclosure, false),
>>      DEFINE_PROP_END_OF_LIST(),
>>  };
>>  
>> @@ -494,11 +495,14 @@ static bool scsi_target_emulate_inquiry(SCSITargetReq *r)
>>      if (r->req.lun != 0) {
>>          r->buf[0] = TYPE_NO_LUN;
>>      } else {
>> -        r->buf[0] = TYPE_ENCLOSURE;
>> +        r->buf[0] = r->req.dev->enclosure ?
>> +            TYPE_ENCLOSURE : TYPE_NOT_PRESENT | TYPE_INACTIVE;
>>          r->buf[2] = 5; /* Version */
>>          r->buf[3] = 2 | 0x10; /* HiSup, response data format */
>>          r->buf[4] = r->len - 5; /* Additional Length = (Len - 1) - 4 */
>> -        r->buf[6] = 0x40; /* Enclosure service */
>> +        if (r->req.dev->enclosure) {
>> +            r->buf[6] = 0x40; /* Enclosure service */
>> +        }
>>          r->buf[7] = 0x10 | (r->req.bus->info->tcq ? 0x02 : 0); /* Sync, TCQ.  */
>>          memcpy(&r->buf[8], "QEMU    ", 8);
>>          memcpy(&r->buf[16], "QEMU TARGET     ", 16);
>> @@ -600,7 +604,8 @@ static int32_t scsi_target_send_command(SCSIRequest *req, uint8_t *buf)
>>          }
>>          break;
>>      case RECEIVE_DIAGNOSTIC:
>> -        if (!scsi_target_emulate_receive_diagnostic(r)) {
>> +        if (!r->req.dev->enclosure ||
>> +            !scsi_target_emulate_receive_diagnostic(r)) {
>>              goto illegal_request;
>>          }
>>          break;
>> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
>> index 5f1e5e8..153d97d 100644
>> --- a/hw/scsi/scsi-disk.c
>> +++ b/hw/scsi/scsi-disk.c
>> @@ -792,7 +792,9 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
>>                 the additional length is not adjusted */
>>          outbuf[4] = 36 - 5;
>>      }
>> -
>> +    if (s->qdev.lun == 0 && s->qdev.enclosure) {
>> +            outbuf[6] = 0x40; /* Enclosure service */
>> +    }
> 
> Should this really be set on disks even if you do have a LUN0?
> 
Why not? You _can_ have embedded enclosure devices; that's what this bit
is for...

> Why don't you instead add a very simple scsi-enclosure device that you
> can specify as the LUN0?
> 
You don't have to.
Once you leave LUN0 free (eg by assigning your first disk LUN1) you'll
get this device by virtue of the current LUN0 emulation.

But yeah, maybe I can be doing a separate enclosure device.
Will be checking if and how I can make it to work.

Cheers,

Hannes
diff mbox

Patch

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 79a222f..c11422b 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -22,6 +22,7 @@  static Property scsi_props[] = {
     DEFINE_PROP_UINT32("channel", SCSIDevice, channel, 0),
     DEFINE_PROP_UINT32("scsi-id", SCSIDevice, id, -1),
     DEFINE_PROP_UINT32("lun", SCSIDevice, lun, -1),
+    DEFINE_PROP_BOOL("enclosure", SCSIDevice, enclosure, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -494,11 +495,14 @@  static bool scsi_target_emulate_inquiry(SCSITargetReq *r)
     if (r->req.lun != 0) {
         r->buf[0] = TYPE_NO_LUN;
     } else {
-        r->buf[0] = TYPE_ENCLOSURE;
+        r->buf[0] = r->req.dev->enclosure ?
+            TYPE_ENCLOSURE : TYPE_NOT_PRESENT | TYPE_INACTIVE;
         r->buf[2] = 5; /* Version */
         r->buf[3] = 2 | 0x10; /* HiSup, response data format */
         r->buf[4] = r->len - 5; /* Additional Length = (Len - 1) - 4 */
-        r->buf[6] = 0x40; /* Enclosure service */
+        if (r->req.dev->enclosure) {
+            r->buf[6] = 0x40; /* Enclosure service */
+        }
         r->buf[7] = 0x10 | (r->req.bus->info->tcq ? 0x02 : 0); /* Sync, TCQ.  */
         memcpy(&r->buf[8], "QEMU    ", 8);
         memcpy(&r->buf[16], "QEMU TARGET     ", 16);
@@ -600,7 +604,8 @@  static int32_t scsi_target_send_command(SCSIRequest *req, uint8_t *buf)
         }
         break;
     case RECEIVE_DIAGNOSTIC:
-        if (!scsi_target_emulate_receive_diagnostic(r)) {
+        if (!r->req.dev->enclosure ||
+            !scsi_target_emulate_receive_diagnostic(r)) {
             goto illegal_request;
         }
         break;
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 5f1e5e8..153d97d 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -792,7 +792,9 @@  static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
                the additional length is not adjusted */
         outbuf[4] = 36 - 5;
     }
-
+    if (s->qdev.lun == 0 && s->qdev.enclosure) {
+            outbuf[6] = 0x40; /* Enclosure service */
+    }
     /* Sync data transfer and TCQ.  */
     outbuf[7] = 0x10 | (req->bus->info->tcq ? 0x02 : 0);
     return buflen;
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 6b85786..243c185 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -100,6 +100,7 @@  struct SCSIDevice
     BlockConf conf;
     SCSISense unit_attention;
     bool sense_is_ua;
+    bool enclosure;
     uint8_t sense[SCSI_SENSE_BUF_SIZE];
     uint32_t sense_len;
     QTAILQ_HEAD(, SCSIRequest) requests;