diff mbox

[1/2,scsi-bus] : Add PR-OUT and PR-IN case for SCSIRequest xfer and xfer_mode setup

Message ID 1275270181-10992-1-git-send-email-nab@linux-iscsi.org
State New
Headers show

Commit Message

Nicholas A. Bellinger May 31, 2010, 1:43 a.m. UTC 
From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch updates hw/scsi-bus.c to add PERSISTENT_RESERVE_OUT and PERSISTENT_RESERVE_IN
case in scsi_req_length() to extra the incoming buffer length into SCSIRequest->cmd.xfer,
and adds a second PERSISTENT_RESERVE_OUT case in scsi_req_xfer_mode() in order to properly
set SCSI_XFER_TO_DEV for WRITE data.

Tested with Linux KVM guests and Megasas 8708EM2 HBA emulation and TCM_Loop target ports.

Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
---
 hw/scsi-bus.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

Comments

Kevin Wolf June 4, 2010, 2:06 p.m. UTC | #1 
Am 31.05.2010 03:43, schrieb Nicholas A. Bellinger:
> From: Nicholas Bellinger <nab@linux-iscsi.org>
> 
> This patch updates hw/scsi-bus.c to add PERSISTENT_RESERVE_OUT and PERSISTENT_RESERVE_IN
> case in scsi_req_length() to extra the incoming buffer length into SCSIRequest->cmd.xfer,
> and adds a second PERSISTENT_RESERVE_OUT case in scsi_req_xfer_mode() in order to properly
> set SCSI_XFER_TO_DEV for WRITE data.
> 
> Tested with Linux KVM guests and Megasas 8708EM2 HBA emulation and TCM_Loop target ports.
> 
> Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
> ---
>  hw/scsi-bus.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
> index b8e4b71..75ec74e 100644
> --- a/hw/scsi-bus.c
> +++ b/hw/scsi-bus.c
> @@ -325,6 +325,10 @@ static int scsi_req_length(SCSIRequest *req, uint8_t *cmd)
>      case INQUIRY:
>          req->cmd.xfer = cmd[4] | (cmd[3] << 8);
>          break;
> +    case PERSISTENT_RESERVE_OUT:
> +    case PERSISTENT_RESERVE_IN:
> +        req->cmd.xfer = cmd[8] | (cmd[7] << 8);

Maybe I'm missing something, but isn't exactly the same value set in the
switch block above? (for cmd[0] >> 5 == 2)

Kevin
Kevin Wolf June 16, 2010, 12:13 p.m. UTC | #2 
Am 04.06.2010 16:06, schrieb Kevin Wolf:
> Am 31.05.2010 03:43, schrieb Nicholas A. Bellinger:
>> From: Nicholas Bellinger <nab@linux-iscsi.org>
>>
>> This patch updates hw/scsi-bus.c to add PERSISTENT_RESERVE_OUT and PERSISTENT_RESERVE_IN
>> case in scsi_req_length() to extra the incoming buffer length into SCSIRequest->cmd.xfer,
>> and adds a second PERSISTENT_RESERVE_OUT case in scsi_req_xfer_mode() in order to properly
>> set SCSI_XFER_TO_DEV for WRITE data.
>>
>> Tested with Linux KVM guests and Megasas 8708EM2 HBA emulation and TCM_Loop target ports.
>>
>> Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
>> ---
>>  hw/scsi-bus.c |    5 +++++
>>  1 files changed, 5 insertions(+), 0 deletions(-)
>>
>> diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
>> index b8e4b71..75ec74e 100644
>> --- a/hw/scsi-bus.c
>> +++ b/hw/scsi-bus.c
>> @@ -325,6 +325,10 @@ static int scsi_req_length(SCSIRequest *req, uint8_t *cmd)
>>      case INQUIRY:
>>          req->cmd.xfer = cmd[4] | (cmd[3] << 8);
>>          break;
>> +    case PERSISTENT_RESERVE_OUT:
>> +    case PERSISTENT_RESERVE_IN:
>> +        req->cmd.xfer = cmd[8] | (cmd[7] << 8);
> 
> Maybe I'm missing something, but isn't exactly the same value set in the
> switch block above? (for cmd[0] >> 5 == 2)

Nicholas? This isn't applied yet because I'm waiting for your answer.

Is there a reason why it makes sense to do it explicitly here instead
using the generic code a few lines above? I think the same applied to
patch 2/2.

Kevin
Nicholas A. Bellinger June 16, 2010, 1:03 p.m. UTC | #3 
On Wed, 2010-06-16 at 14:13 +0200, Kevin Wolf wrote:
> Am 04.06.2010 16:06, schrieb Kevin Wolf:
> > Am 31.05.2010 03:43, schrieb Nicholas A. Bellinger:
> >> From: Nicholas Bellinger <nab@linux-iscsi.org>
> >>
> >> This patch updates hw/scsi-bus.c to add PERSISTENT_RESERVE_OUT and PERSISTENT_RESERVE_IN
> >> case in scsi_req_length() to extra the incoming buffer length into SCSIRequest->cmd.xfer,
> >> and adds a second PERSISTENT_RESERVE_OUT case in scsi_req_xfer_mode() in order to properly
> >> set SCSI_XFER_TO_DEV for WRITE data.
> >>
> >> Tested with Linux KVM guests and Megasas 8708EM2 HBA emulation and TCM_Loop target ports.
> >>
> >> Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
> >> ---
> >>  hw/scsi-bus.c |    5 +++++
> >>  1 files changed, 5 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
> >> index b8e4b71..75ec74e 100644
> >> --- a/hw/scsi-bus.c
> >> +++ b/hw/scsi-bus.c
> >> @@ -325,6 +325,10 @@ static int scsi_req_length(SCSIRequest *req, uint8_t *cmd)
> >>      case INQUIRY:
> >>          req->cmd.xfer = cmd[4] | (cmd[3] << 8);
> >>          break;
> >> +    case PERSISTENT_RESERVE_OUT:
> >> +    case PERSISTENT_RESERVE_IN:
> >> +        req->cmd.xfer = cmd[8] | (cmd[7] << 8);
> > 
> > Maybe I'm missing something, but isn't exactly the same value set in the
> > switch block above? (for cmd[0] >> 5 == 2)
> 
> Nicholas? This isn't applied yet because I'm waiting for your answer.
> 
> Is there a reason why it makes sense to do it explicitly here instead
> using the generic code a few lines above? I think the same applied to
> patch 2/2.

Hi Kevin,

I just tested this again and you are correct, the reassignment of
req->cmd.xfer for PR and Maintence CDBs is unnecessary in
scsi_req_length().  I will go ahead and drop part this from my tree.

Please let me know if you would like me to resend the patch series.

Best,

--nab
Kevin Wolf June 16, 2010, 1:08 p.m. UTC | #4 
Am 16.06.2010 15:03, schrieb Nicholas A. Bellinger:
> On Wed, 2010-06-16 at 14:13 +0200, Kevin Wolf wrote:
>> Am 04.06.2010 16:06, schrieb Kevin Wolf:
>>> Am 31.05.2010 03:43, schrieb Nicholas A. Bellinger:
>>>> From: Nicholas Bellinger <nab@linux-iscsi.org>
>>>>
>>>> This patch updates hw/scsi-bus.c to add PERSISTENT_RESERVE_OUT and PERSISTENT_RESERVE_IN
>>>> case in scsi_req_length() to extra the incoming buffer length into SCSIRequest->cmd.xfer,
>>>> and adds a second PERSISTENT_RESERVE_OUT case in scsi_req_xfer_mode() in order to properly
>>>> set SCSI_XFER_TO_DEV for WRITE data.
>>>>
>>>> Tested with Linux KVM guests and Megasas 8708EM2 HBA emulation and TCM_Loop target ports.
>>>>
>>>> Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
>>>> ---
>>>>  hw/scsi-bus.c |    5 +++++
>>>>  1 files changed, 5 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
>>>> index b8e4b71..75ec74e 100644
>>>> --- a/hw/scsi-bus.c
>>>> +++ b/hw/scsi-bus.c
>>>> @@ -325,6 +325,10 @@ static int scsi_req_length(SCSIRequest *req, uint8_t *cmd)
>>>>      case INQUIRY:
>>>>          req->cmd.xfer = cmd[4] | (cmd[3] << 8);
>>>>          break;
>>>> +    case PERSISTENT_RESERVE_OUT:
>>>> +    case PERSISTENT_RESERVE_IN:
>>>> +        req->cmd.xfer = cmd[8] | (cmd[7] << 8);
>>>
>>> Maybe I'm missing something, but isn't exactly the same value set in the
>>> switch block above? (for cmd[0] >> 5 == 2)
>>
>> Nicholas? This isn't applied yet because I'm waiting for your answer.
>>
>> Is there a reason why it makes sense to do it explicitly here instead
>> using the generic code a few lines above? I think the same applied to
>> patch 2/2.
> 
> Hi Kevin,
> 
> I just tested this again and you are correct, the reassignment of
> req->cmd.xfer for PR and Maintence CDBs is unnecessary in
> scsi_req_length().  I will go ahead and drop part this from my tree.
> 
> Please let me know if you would like me to resend the patch series.

Sure, taking simpler code is always better, so I'd be happier with a v2.

Kevin
diff mbox

Patch

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index b8e4b71..75ec74e 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -325,6 +325,10 @@  static int scsi_req_length(SCSIRequest *req, uint8_t *cmd)
     case INQUIRY:
         req->cmd.xfer = cmd[4] | (cmd[3] << 8);
         break;
+    case PERSISTENT_RESERVE_OUT:
+    case PERSISTENT_RESERVE_IN:
+        req->cmd.xfer = cmd[8] | (cmd[7] << 8);
+        break;
     }
     return 0;
 }
@@ -389,6 +393,7 @@  static void scsi_req_xfer_mode(SCSIRequest *req)
     case MEDIUM_SCAN:
     case SEND_VOLUME_TAG:
     case WRITE_LONG_2:
+    case PERSISTENT_RESERVE_OUT:
         req->cmd.mode = SCSI_XFER_TO_DEV;
         break;
     default: