| Message ID | 20210503150333.130310-6-hare@suse.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | scsi: enabled reserved commands for LLDDs | expand |
On 5/3/21 8:03 AM, Hannes Reinecke wrote: > Use dummy inquiry data when initialising devices and not just > some 'nullnullnull' string. Reviewed-by: Bart Van Assche <bvanassche@acm.org>
On Mon, May 03, 2021 at 05:03:20PM +0200, Hannes Reinecke wrote: > Use dummy inquiry data when initialising devices and not just > some 'nullnullnull' string. Why? > +/* > + * Dummy inquiry for virtual LUNs: > + * > + * standard INQUIRY: [qualifier indicates no connected LU] > + * PQual=1 Device_type=31 RMB=0 LU_CONG=0 version=0x05 [SPC-3] > + * [AERC=0] [TrmTsk=0] NormACA=0 HiSUP=0 Resp_data_format=2 > + * SCCS=0 ACC=0 TPGS=0 3PC=0 Protect=0 [BQue=0] > + * EncServ=0 MultiP=0 [MChngr=0] [ACKREQQ=0] Addr16=0 > + * [RelAdr=0] WBus16=0 Sync=0 [Linked=0] [TranDis=0] CmdQue=0 > + * length=36 (0x24) Peripheral device type: no physical device on this lu > + * Vendor identification: LINUX > + * Product identification: VIRTUALLUN > + * Product revision level: 1.0 > + */ You don't juse set this up for virtual Luns, but as a default for all scsi_devices before calling inquirty. I'd much helper with a helper to fill out fake inquiry data rather than having seemingly valid data for all devices before inquirty is called or if it fails.
On 5/4/21 11:55 AM, Christoph Hellwig wrote: > On Mon, May 03, 2021 at 05:03:20PM +0200, Hannes Reinecke wrote: >> Use dummy inquiry data when initialising devices and not just >> some 'nullnullnull' string. > > Why? > Because it's really weird if you start up scsi_debug with thousands of devices and then call 'lsscsi' repeatedly. That will print out several devices with 'nullnullnull', only to be replaced with the 'real' inquiry data during device discovery. I'd rather have a valid inquiry right from the start. >> +/* >> + * Dummy inquiry for virtual LUNs: >> + * >> + * standard INQUIRY: [qualifier indicates no connected LU] >> + * PQual=1 Device_type=31 RMB=0 LU_CONG=0 version=0x05 [SPC-3] >> + * [AERC=0] [TrmTsk=0] NormACA=0 HiSUP=0 Resp_data_format=2 >> + * SCCS=0 ACC=0 TPGS=0 3PC=0 Protect=0 [BQue=0] >> + * EncServ=0 MultiP=0 [MChngr=0] [ACKREQQ=0] Addr16=0 >> + * [RelAdr=0] WBus16=0 Sync=0 [Linked=0] [TranDis=0] CmdQue=0 >> + * length=36 (0x24) Peripheral device type: no physical device on this lu >> + * Vendor identification: LINUX >> + * Product identification: VIRTUALLUN >> + * Product revision level: 1.0 >> + */ > > You don't juse set this up for virtual Luns, but as a default for all > scsi_devices before calling inquirty. I'd much helper with a helper > to fill out fake inquiry data rather than having seemingly valid data > for all devices before inquirty is called or if it fails. > Right. Will be doing so. Cheers, Hannes
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c index 9f1b7f3c650a..a50abba41ac1 100644 --- a/drivers/scsi/scsi_scan.c +++ b/drivers/scsi/scsi_scan.c @@ -81,7 +81,27 @@ #define SCSI_SCAN_TARGET_PRESENT 1 #define SCSI_SCAN_LUN_PRESENT 2 -static const char *scsi_null_device_strs = "nullnullnullnull"; +/* + * Dummy inquiry for virtual LUNs: + * + * standard INQUIRY: [qualifier indicates no connected LU] + * PQual=1 Device_type=31 RMB=0 LU_CONG=0 version=0x05 [SPC-3] + * [AERC=0] [TrmTsk=0] NormACA=0 HiSUP=0 Resp_data_format=2 + * SCCS=0 ACC=0 TPGS=0 3PC=0 Protect=0 [BQue=0] + * EncServ=0 MultiP=0 [MChngr=0] [ACKREQQ=0] Addr16=0 + * [RelAdr=0] WBus16=0 Sync=0 [Linked=0] [TranDis=0] CmdQue=0 + * length=36 (0x24) Peripheral device type: no physical device on this lu + * Vendor identification: LINUX + * Product identification: VIRTUALLUN + * Product revision level: 1.0 + */ +static const unsigned char scsi_null_inquiry[36] = { + 0x3f, 0x00, 0x05, 0x02, 0x1f, 0x00, 0x00, 0x00, + 0x4c, 0x49, 0x4e, 0x55, 0x58, 0x20, 0x20, 0x20, + 0x56, 0x49, 0x52, 0x54, 0x55, 0x41, 0x4c, 0x4c, + 0x55, 0x4e, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, + 0x31, 0x2e, 0x30, 0x20 +}; #define MAX_SCSI_LUNS 512 @@ -225,9 +245,10 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget, if (!sdev) goto out; - sdev->vendor = scsi_null_device_strs; - sdev->model = scsi_null_device_strs; - sdev->rev = scsi_null_device_strs; + sdev->type = scsi_null_inquiry[0] & 0x1f; + sdev->vendor = scsi_null_inquiry + 8; + sdev->model = scsi_null_inquiry + 16; + sdev->rev = scsi_null_inquiry + 32; sdev->host = shost; sdev->queue_ramp_up_period = SCSI_DEFAULT_RAMP_UP_PERIOD; sdev->id = starget->id; @@ -254,11 +275,6 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget, * slave_configure function */ sdev->max_device_blocked = SCSI_DEFAULT_DEVICE_BLOCKED; - /* - * Some low level driver could use device->type - */ - sdev->type = -1; - /* * Assume that the device will have handshaking problems, * and then fix this field later if it turns out it
Use dummy inquiry data when initialising devices and not just some 'nullnullnull' string. Signed-off-by: Hannes Reinecke <hare@suse.de> --- drivers/scsi/scsi_scan.c | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-)