[Qemu-devel] [PATCH 0/2] scsi: enclosure support

[Qemu-devel] [PATCH 0/2] scsi: enclosure support

[Hannes Reinecke]

Hi all,

due to a customer issue I've added simple subenclosure support
to the SCSI emulation. The patch simply converts the current invisible
LUN0 into an enclosure device; existing setups using LUN0 as disks or
CD-ROMs will not be affected.

Hannes Reinecke (2):
  scsi: Make LUN 0 a simple enclosure
  scsi: use qemu_uuid to generate logical identifier for SES

 hw/scsi/scsi-bus.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 72 insertions(+), 1 deletion(-)

-- 
1.8.5.6

[Qemu-devel] [PATCH 1/2] scsi: Make LUN 0 a simple enclosure

[Hannes Reinecke]

Instead of having an 'invisible' LUN0 (in case LUN 0 is not connected)
this patch maks LUN0 a enclosure service, exposing it to the OS.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 hw/scsi/scsi-bus.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 55 insertions(+), 1 deletion(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 23c51de..c89e82d 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -493,10 +493,11 @@ static bool scsi_target_emulate_inquiry(SCSITargetReq *r)
     if (r->req.lun != 0) {
         r->buf[0] = TYPE_NO_LUN;
     } else {
-        r->buf[0] = TYPE_NOT_PRESENT | TYPE_INACTIVE;
+        r->buf[0] = TYPE_ENCLOSURE;
         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 */
         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);
@@ -505,6 +506,54 @@ static bool scsi_target_emulate_inquiry(SCSITargetReq *r)
     return true;
 }
 
+static bool scsi_target_emulate_receive_diagnostic(SCSITargetReq *r)
+{
+    uint8_t page_code = r->req.cmd.buf[2];
+    unsigned char *enc_desc, *type_desc;
+
+    assert(r->req.dev->lun != r->req.lun);
+
+    scsi_target_alloc_buf(&r->req, 0x38);
+
+    switch (page_code) {
+    case 0x00:
+        r->buf[r->len++] = page_code ; /* this page */
+        r->buf[r->len++] = 0x00;
+        r->buf[r->len++] = 0x00;
+        r->buf[r->len++] = 0x03;
+        r->buf[r->len++] = 0x00;
+        r->buf[r->len++] = 0x01;
+        r->buf[r->len++] = 0x08;
+        break;
+    case 0x01:
+        memset(r->buf, 0, 0x38);
+        r->buf[0] = page_code;
+        r->buf[3] = 0x30;
+        enc_desc = &r->buf[8];
+        enc_desc[0] = 0x09;
+        enc_desc[2] = 1;
+        enc_desc[3] = 0x24;
+        memcpy(&enc_desc[12], "QEMU    ", 8);
+        memcpy(&enc_desc[20], "QEMU TARGET     ", 16);
+        pstrcpy((char *)&enc_desc[36], 4, qemu_hw_version());
+        type_desc = &r->buf[48];
+        type_desc[1] = 1;
+        r->len = 0x38;
+        break;
+    case 0x08:
+        r->buf[0] = page_code;
+        r->buf[1] = 0x00;
+        r->buf[2] = 0x00;
+        r->buf[3] = 0x00;
+        r->len = 4;
+        break;
+    default:
+        return false;
+    }
+    r->len = MIN(r->req.cmd.xfer, r->len);
+    return true;
+}
+
 static size_t scsi_sense_len(SCSIRequest *req)
 {
     if (req->dev->type == TYPE_SCANNER)
@@ -528,6 +577,11 @@ static int32_t scsi_target_send_command(SCSIRequest *req, uint8_t *buf)
             goto illegal_request;
         }
         break;
+    case RECEIVE_DIAGNOSTIC:
+        if (!scsi_target_emulate_receive_diagnostic(r)) {
+            goto illegal_request;
+        }
+        break;
     case REQUEST_SENSE:
         scsi_target_alloc_buf(&r->req, scsi_sense_len(req));
         r->len = scsi_device_get_sense(r->req.dev, r->buf,
-- 
1.8.5.6

Re: [Qemu-devel] [PATCH 1/2] scsi: Make LUN 0 a simple enclosure

[Daniel P. Berrange]

On Thu, Aug 03, 2017 at 03:27:00PM +0200, Hannes Reinecke wrote:
> Instead of having an 'invisible' LUN0 (in case LUN 0 is not connected)
> this patch maks LUN0 a enclosure service, exposing it to the OS.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
>  hw/scsi/scsi-bus.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 55 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
> index 23c51de..c89e82d 100644
> --- a/hw/scsi/scsi-bus.c
> +++ b/hw/scsi/scsi-bus.c
> @@ -493,10 +493,11 @@ static bool scsi_target_emulate_inquiry(SCSITargetReq *r)
>      if (r->req.lun != 0) {
>          r->buf[0] = TYPE_NO_LUN;
>      } else {
> -        r->buf[0] = TYPE_NOT_PRESENT | TYPE_INACTIVE;
> +        r->buf[0] = TYPE_ENCLOSURE;
>          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 */
>          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);

I would think this needs to be tied into machine type version, otherwise
when you migrate old to new QEMU, LUN0 is suddenly going to change beneath
the running guest ?

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 1/2] scsi: Make LUN 0 a simple enclosure

[Hannes Reinecke]

On 08/03/2017 03:32 PM, Daniel P. Berrange wrote:
> On Thu, Aug 03, 2017 at 03:27:00PM +0200, Hannes Reinecke wrote:
>> Instead of having an 'invisible' LUN0 (in case LUN 0 is not connected)
>> this patch maks LUN0 a enclosure service, exposing it to the OS.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.com>
>> ---
>>  hw/scsi/scsi-bus.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 55 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
>> index 23c51de..c89e82d 100644
>> --- a/hw/scsi/scsi-bus.c
>> +++ b/hw/scsi/scsi-bus.c
>> @@ -493,10 +493,11 @@ static bool scsi_target_emulate_inquiry(SCSITargetReq *r)
>>      if (r->req.lun != 0) {
>>          r->buf[0] = TYPE_NO_LUN;
>>      } else {
>> -        r->buf[0] = TYPE_NOT_PRESENT | TYPE_INACTIVE;
>> +        r->buf[0] = TYPE_ENCLOSURE;
>>          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 */
>>          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);
> 
> I would think this needs to be tied into machine type version, otherwise
> when you migrate old to new QEMU, LUN0 is suddenly going to change beneath
> the running guest ?
> 
(I _knew_ this would be coming ...)

It will only change if LUN0 is _not_ assigned, ie if a LUN larger than 0
is the first LUN on that host.
In those cases the system would see an additional LUN, correct.
But as that LUN is trivially not used I don't really see a problem with
that.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.com			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[Qemu-devel] [PATCH 2/2] scsi: use qemu_uuid to generate logical identifier for SES

[Hannes Reinecke]

The SES enclosure descriptor requires a logical identifier,
so generate one using the qemu_uuid and the Qumranet OUI.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 hw/scsi/scsi-bus.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index c89e82d..8419c75 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -10,6 +10,7 @@
 #include "trace.h"
 #include "sysemu/dma.h"
 #include "qemu/cutils.h"
+#include "qemu/crc32c.h"
 
 static char *scsibus_get_dev_path(DeviceState *dev);
 static char *scsibus_get_fw_dev_path(DeviceState *dev);
@@ -533,6 +534,22 @@ static bool scsi_target_emulate_receive_diagnostic(SCSITargetReq *r)
         enc_desc[0] = 0x09;
         enc_desc[2] = 1;
         enc_desc[3] = 0x24;
+        if (qemu_uuid_set) {
+            uint32_t crc;
+
+            /*
+             * Make this a NAA IEEE Registered identifier
+             * using Qumranet OUI (0x001A4A) and the
+             * crc32 from the system UUID.
+             */
+            enc_desc[4] = 0x50;
+            enc_desc[5] = 0x01;
+            enc_desc[6] = 0xa4;
+            enc_desc[7] = 0xa0;
+            crc = crc32c(0xffffffff, qemu_uuid.data, 16);
+            cpu_to_le32s(&crc);
+            memcpy(&enc_desc[8], &crc, 4);
+        }
         memcpy(&enc_desc[12], "QEMU    ", 8);
         memcpy(&enc_desc[20], "QEMU TARGET     ", 16);
         pstrcpy((char *)&enc_desc[36], 4, qemu_hw_version());
-- 
1.8.5.6

Re: [Qemu-devel] [PATCH 0/2] scsi: enclosure support

[Paolo Bonzini]

On 03/08/2017 15:26, Hannes Reinecke wrote:
> Hi all,
> 
> due to a customer issue I've added simple subenclosure support
> to the SCSI emulation. The patch simply converts the current invisible
> LUN0 into an enclosure device; existing setups using LUN0 as disks or
> CD-ROMs will not be affected.

What is the issue exactly?  That is, for what is it necessary to have a
dummy enclosure?

I agree with Dan that this need machine type compatibility gunk.  For
example, could the new device affect /dev/sgN numbering?

Paolo

Re: [Qemu-devel] [PATCH 0/2] scsi: enclosure support

[Hannes Reinecke]

On 08/03/2017 05:10 PM, Paolo Bonzini wrote:
> On 03/08/2017 15:26, Hannes Reinecke wrote:
>> Hi all,
>>
>> due to a customer issue I've added simple subenclosure support
>> to the SCSI emulation. The patch simply converts the current invisible
>> LUN0 into an enclosure device; existing setups using LUN0 as disks or
>> CD-ROMs will not be affected.
> 
> What is the issue exactly?  That is, for what is it necessary to have a
> dummy enclosure?
> 
Well, stock linux displays some very interesting error messages for
these types of enclosures.
Which was the prime mover for doing this.

> I agree with Dan that this need machine type compatibility gunk.  For
> example, could the new device affect /dev/sgN numbering?
> 
Yes, indeed it would.

What about a new option to the scsi driver?
With that each user could selectively enable it, and we wouldn't need to
worry with machine type compability...

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [Qemu-devel] [PATCH 0/2] scsi: enclosure support

[Paolo Bonzini]

> On 08/03/2017 05:10 PM, Paolo Bonzini wrote:
> > On 03/08/2017 15:26, Hannes Reinecke wrote:
> >> Hi all,
> >>
> >> due to a customer issue I've added simple subenclosure support
> >> to the SCSI emulation. The patch simply converts the current invisible
> >> LUN0 into an enclosure device; existing setups using LUN0 as disks or
> >> CD-ROMs will not be affected.
> > 
> > What is the issue exactly?  That is, for what is it necessary to have a
> > dummy enclosure?
> > 
> Well, stock linux displays some very interesting error messages for
> these types of enclosures. Which was the prime mover for doing this.

--verbose?

> > I agree with Dan that this need machine type compatibility gunk.  For
> > example, could the new device affect /dev/sgN numbering?
>
> Yes, indeed it would.
> 
> What about a new option to the scsi driver?

If you do that, you've done 99% of the work to do compatibility so I
won't complain and do the 1% myself. :)

Paolo

Re: [Qemu-devel] [PATCH 0/2] scsi: enclosure support

[Hannes Reinecke]

On 08/04/2017 08:10 AM, Paolo Bonzini wrote:
> 
>> On 08/03/2017 05:10 PM, Paolo Bonzini wrote:
>>> On 03/08/2017 15:26, Hannes Reinecke wrote:
>>>> Hi all,
>>>>
>>>> due to a customer issue I've added simple subenclosure support
>>>> to the SCSI emulation. The patch simply converts the current invisible
>>>> LUN0 into an enclosure device; existing setups using LUN0 as disks or
>>>> CD-ROMs will not be affected.
>>>
>>> What is the issue exactly?  That is, for what is it necessary to have a
>>> dummy enclosure?
>>>
>> Well, stock linux displays some very interesting error messages for
>> these types of enclosures. Which was the prime mover for doing this.
> 
> --verbose?
> 
[   12.958454] scsi 1:0:0:254: Wrong diagnostic page; asked for 2 got 0
[   12.958456] scsi 1:0:0:254: Failed to get diagnostic page 0xffffffea
[   12.958457] scsi 1:0:0:254: Failed to bind enclosure -19
[   12.959392] ses 1:0:0:254: Attached Enclosure device

>>> I agree with Dan that this need machine type compatibility gunk.  For
>>> example, could the new device affect /dev/sgN numbering?
>>
>> Yes, indeed it would.
>>
>> What about a new option to the scsi driver?
> 
> If you do that, you've done 99% of the work to do compatibility so I
> won't complain and do the 1% myself. :)
> 
Okay, will be doing so.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)