diff mbox

virtio-scsi: Change sense buffer size to 252

Message ID 1394095660-15075-1-git-send-email-famz@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Fam Zheng March 6, 2014, 8:47 a.m. UTC
According to SPC-4, section 4.5.2.1, 252 is the limit of sense data. So
increase the value.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 include/linux/virtio_scsi.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Paolo Bonzini March 6, 2014, 10:09 a.m. UTC | #1
Il 06/03/2014 09:47, Fam Zheng ha scritto:
> According to SPC-4, section 4.5.2.1, 252 is the limit of sense data. So
> increase the value.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  include/linux/virtio_scsi.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/virtio_scsi.h b/include/linux/virtio_scsi.h
> index 4195b97..a437f7f 100644
> --- a/include/linux/virtio_scsi.h
> +++ b/include/linux/virtio_scsi.h
> @@ -28,7 +28,7 @@
>  #define _LINUX_VIRTIO_SCSI_H
>
>  #define VIRTIO_SCSI_CDB_SIZE   32
> -#define VIRTIO_SCSI_SENSE_SIZE 96
> +#define VIRTIO_SCSI_SENSE_SIZE 252
>
>  /* SCSI command request, followed by data-out */
>  struct virtio_scsi_cmd_req {
>

Hi Fam, how did you test this?

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hannes Reinecke March 6, 2014, 11:22 a.m. UTC | #2
On 03/06/2014 11:09 AM, Paolo Bonzini wrote:
> Il 06/03/2014 09:47, Fam Zheng ha scritto:
>> According to SPC-4, section 4.5.2.1, 252 is the limit of sense
>> data. So
>> increase the value.
>>
>> Signed-off-by: Fam Zheng <famz@redhat.com>
>> ---
>>  include/linux/virtio_scsi.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/linux/virtio_scsi.h
>> b/include/linux/virtio_scsi.h
>> index 4195b97..a437f7f 100644
>> --- a/include/linux/virtio_scsi.h
>> +++ b/include/linux/virtio_scsi.h
>> @@ -28,7 +28,7 @@
>>  #define _LINUX_VIRTIO_SCSI_H
>>
>>  #define VIRTIO_SCSI_CDB_SIZE   32
>> -#define VIRTIO_SCSI_SENSE_SIZE 96
>> +#define VIRTIO_SCSI_SENSE_SIZE 252
>>
>>  /* SCSI command request, followed by data-out */
>>  struct virtio_scsi_cmd_req {
>>
> 
> Hi Fam, how did you test this?
> 
Is there a specific reason _not_ to use the linux default?
The SCSI stack typically limits the sense code to
SCSI_SENSE_BUFFERSIZE, so using other values have a
limited sense.
Literally :-)

Cheers,

Hannes
Paolo Bonzini March 6, 2014, 11:55 a.m. UTC | #3
Il 06/03/2014 12:22, Hannes Reinecke ha scritto:
> On 03/06/2014 11:09 AM, Paolo Bonzini wrote:
>> Il 06/03/2014 09:47, Fam Zheng ha scritto:
>>> According to SPC-4, section 4.5.2.1, 252 is the limit of sense
>>> data. So
>>> increase the value.
>>>
>>> Signed-off-by: Fam Zheng <famz@redhat.com>
>>> ---
>>>  include/linux/virtio_scsi.h | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/include/linux/virtio_scsi.h
>>> b/include/linux/virtio_scsi.h
>>> index 4195b97..a437f7f 100644
>>> --- a/include/linux/virtio_scsi.h
>>> +++ b/include/linux/virtio_scsi.h
>>> @@ -28,7 +28,7 @@
>>>  #define _LINUX_VIRTIO_SCSI_H
>>>
>>>  #define VIRTIO_SCSI_CDB_SIZE   32
>>> -#define VIRTIO_SCSI_SENSE_SIZE 96
>>> +#define VIRTIO_SCSI_SENSE_SIZE 252
>>>
>>>  /* SCSI command request, followed by data-out */
>>>  struct virtio_scsi_cmd_req {
>>>
>>
>> Hi Fam, how did you test this?
>
> Is there a specific reason _not_ to use the linux default?
> The SCSI stack typically limits the sense code to
> SCSI_SENSE_BUFFERSIZE, so using other values have a
> limited sense.
> Literally :-)

Indeed I don't think this patch makes a difference.  Though I asked not 
from the SCSI stack perspective, but because right now both virtio-scsi 
targets (QEMU and vhost-scsi) are also truncating at 96.

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Fam Zheng March 7, 2014, 5:56 a.m. UTC | #4
On Thu, 03/06 11:09, Paolo Bonzini wrote:
> Il 06/03/2014 09:47, Fam Zheng ha scritto:
> >According to SPC-4, section 4.5.2.1, 252 is the limit of sense data. So
> >increase the value.
> >
> >Signed-off-by: Fam Zheng <famz@redhat.com>
> >---
> > include/linux/virtio_scsi.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/include/linux/virtio_scsi.h b/include/linux/virtio_scsi.h
> >index 4195b97..a437f7f 100644
> >--- a/include/linux/virtio_scsi.h
> >+++ b/include/linux/virtio_scsi.h
> >@@ -28,7 +28,7 @@
> > #define _LINUX_VIRTIO_SCSI_H
> >
> > #define VIRTIO_SCSI_CDB_SIZE   32
> >-#define VIRTIO_SCSI_SENSE_SIZE 96
> >+#define VIRTIO_SCSI_SENSE_SIZE 252
> >
> > /* SCSI command request, followed by data-out */
> > struct virtio_scsi_cmd_req {
> >
> 
> Hi Fam, how did you test this?
> 

I only tested the basic functionality of virtio-scsi. I'm doing more testing on
this and your fix on QEMU side now.

Thanks,
Fam
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Fam Zheng March 7, 2014, 5:59 a.m. UTC | #5
On Thu, 03/06 12:55, Paolo Bonzini wrote:
> Il 06/03/2014 12:22, Hannes Reinecke ha scritto:
> >On 03/06/2014 11:09 AM, Paolo Bonzini wrote:
> >>Il 06/03/2014 09:47, Fam Zheng ha scritto:
> >>>According to SPC-4, section 4.5.2.1, 252 is the limit of sense
> >>>data. So
> >>>increase the value.
> >>>
> >>>Signed-off-by: Fam Zheng <famz@redhat.com>
> >>>---
> >>> include/linux/virtio_scsi.h | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>>diff --git a/include/linux/virtio_scsi.h
> >>>b/include/linux/virtio_scsi.h
> >>>index 4195b97..a437f7f 100644
> >>>--- a/include/linux/virtio_scsi.h
> >>>+++ b/include/linux/virtio_scsi.h
> >>>@@ -28,7 +28,7 @@
> >>> #define _LINUX_VIRTIO_SCSI_H
> >>>
> >>> #define VIRTIO_SCSI_CDB_SIZE   32
> >>>-#define VIRTIO_SCSI_SENSE_SIZE 96
> >>>+#define VIRTIO_SCSI_SENSE_SIZE 252
> >>>
> >>> /* SCSI command request, followed by data-out */
> >>> struct virtio_scsi_cmd_req {
> >>>
> >>
> >>Hi Fam, how did you test this?
> >
> >Is there a specific reason _not_ to use the linux default?
> >The SCSI stack typically limits the sense code to
> >SCSI_SENSE_BUFFERSIZE, so using other values have a
> >limited sense.
> >Literally :-)

OK. So, do we need to lift the limit on other parts of SCSI stack as well?

> 
> Indeed I don't think this patch makes a difference.  Though I asked not from
> the SCSI stack perspective, but because right now both virtio-scsi targets
> (QEMU and vhost-scsi) are also truncating at 96.
> 

Oh, I missed that, I'll do a more thorough review and testing. Thank you for
pointing out.

Thanks,
Fam
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/linux/virtio_scsi.h b/include/linux/virtio_scsi.h
index 4195b97..a437f7f 100644
--- a/include/linux/virtio_scsi.h
+++ b/include/linux/virtio_scsi.h
@@ -28,7 +28,7 @@ 
 #define _LINUX_VIRTIO_SCSI_H
 
 #define VIRTIO_SCSI_CDB_SIZE   32
-#define VIRTIO_SCSI_SENSE_SIZE 96
+#define VIRTIO_SCSI_SENSE_SIZE 252
 
 /* SCSI command request, followed by data-out */
 struct virtio_scsi_cmd_req {