* [Qemu-trivial] [PATCH] spapr_vscsi: remove duplicate condition check
@ 2014-03-24 15:44 ` Prasad Joshi
0 siblings, 0 replies; 8+ messages in thread
From: Prasad Joshi @ 2014-03-24 15:44 UTC (permalink / raw)
To: prasadjoshi.linux
Cc: qemu-trivial, qemu-devel, agraf, qemu-ppc, stefanha, pbonzini
Signed-off-by: Prasad Joshi <prasadjoshi.linux@gmail.com>
---
hw/scsi/spapr_vscsi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index 34478f0..d4ada4f 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -690,7 +690,7 @@ static void vscsi_inquiry_no_target(VSCSIState *s, vscsi_req *req)
int rc, len, alen;
/* We dont do EVPD. Also check that page_code is 0 */
- if ((cdb[1] & 0x01) || (cdb[1] & 0x01) || cdb[2] != 0) {
+ if ((cdb[1] & 0x01) || cdb[2] != 0) {
/* Send INVALID FIELD IN CDB */
vscsi_makeup_sense(s, req, ILLEGAL_REQUEST, 0x24, 0);
vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
--
1.8.1.2
^ permalink raw reply related [flat|nested] 8+ messages in thread* [Qemu-devel] [PATCH] spapr_vscsi: remove duplicate condition check
@ 2014-03-24 15:44 ` Prasad Joshi
0 siblings, 0 replies; 8+ messages in thread
From: Prasad Joshi @ 2014-03-24 15:44 UTC (permalink / raw)
To: prasadjoshi.linux
Cc: qemu-trivial, qemu-devel, agraf, qemu-ppc, stefanha, pbonzini
Signed-off-by: Prasad Joshi <prasadjoshi.linux@gmail.com>
---
hw/scsi/spapr_vscsi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index 34478f0..d4ada4f 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -690,7 +690,7 @@ static void vscsi_inquiry_no_target(VSCSIState *s, vscsi_req *req)
int rc, len, alen;
/* We dont do EVPD. Also check that page_code is 0 */
- if ((cdb[1] & 0x01) || (cdb[1] & 0x01) || cdb[2] != 0) {
+ if ((cdb[1] & 0x01) || cdb[2] != 0) {
/* Send INVALID FIELD IN CDB */
vscsi_makeup_sense(s, req, ILLEGAL_REQUEST, 0x24, 0);
vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
--
1.8.1.2
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [Qemu-trivial] [PATCH] spapr_vscsi: remove duplicate condition check
2014-03-24 15:44 ` [Qemu-devel] " Prasad Joshi
@ 2014-03-24 15:45 ` Paolo Bonzini
-1 siblings, 0 replies; 8+ messages in thread
From: Paolo Bonzini @ 2014-03-24 15:45 UTC (permalink / raw)
To: Prasad Joshi; +Cc: qemu-trivial, qemu-ppc, qemu-devel, stefanha, agraf
Il 24/03/2014 16:44, Prasad Joshi ha scritto:
> Signed-off-by: Prasad Joshi <prasadjoshi.linux@gmail.com>
> ---
> hw/scsi/spapr_vscsi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
> index 34478f0..d4ada4f 100644
> --- a/hw/scsi/spapr_vscsi.c
> +++ b/hw/scsi/spapr_vscsi.c
> @@ -690,7 +690,7 @@ static void vscsi_inquiry_no_target(VSCSIState *s, vscsi_req *req)
> int rc, len, alen;
>
> /* We dont do EVPD. Also check that page_code is 0 */
> - if ((cdb[1] & 0x01) || (cdb[1] & 0x01) || cdb[2] != 0) {
> + if ((cdb[1] & 0x01) || cdb[2] != 0) {
> /* Send INVALID FIELD IN CDB */
> vscsi_makeup_sense(s, req, ILLEGAL_REQUEST, 0x24, 0);
> vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
>
Not trivial---I have no idea if something else was meant to be checked.
Paolo
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH] spapr_vscsi: remove duplicate condition check
@ 2014-03-24 15:45 ` Paolo Bonzini
0 siblings, 0 replies; 8+ messages in thread
From: Paolo Bonzini @ 2014-03-24 15:45 UTC (permalink / raw)
To: Prasad Joshi; +Cc: qemu-trivial, qemu-ppc, qemu-devel, stefanha, agraf
Il 24/03/2014 16:44, Prasad Joshi ha scritto:
> Signed-off-by: Prasad Joshi <prasadjoshi.linux@gmail.com>
> ---
> hw/scsi/spapr_vscsi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
> index 34478f0..d4ada4f 100644
> --- a/hw/scsi/spapr_vscsi.c
> +++ b/hw/scsi/spapr_vscsi.c
> @@ -690,7 +690,7 @@ static void vscsi_inquiry_no_target(VSCSIState *s, vscsi_req *req)
> int rc, len, alen;
>
> /* We dont do EVPD. Also check that page_code is 0 */
> - if ((cdb[1] & 0x01) || (cdb[1] & 0x01) || cdb[2] != 0) {
> + if ((cdb[1] & 0x01) || cdb[2] != 0) {
> /* Send INVALID FIELD IN CDB */
> vscsi_makeup_sense(s, req, ILLEGAL_REQUEST, 0x24, 0);
> vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
>
Not trivial---I have no idea if something else was meant to be checked.
Paolo
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-trivial] [PATCH] spapr_vscsi: remove duplicate condition check
2014-03-24 15:45 ` [Qemu-devel] " Paolo Bonzini
@ 2014-03-24 17:19 ` Prasad Joshi
-1 siblings, 0 replies; 8+ messages in thread
From: Prasad Joshi @ 2014-03-24 17:19 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-trivial, qemu-ppc, qemu-devel, Stefan Hajnoczi, agraf
On Mon, Mar 24, 2014 at 9:15 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 24/03/2014 16:44, Prasad Joshi ha scritto:
>
>> Signed-off-by: Prasad Joshi <prasadjoshi.linux@gmail.com>
>> ---
>> hw/scsi/spapr_vscsi.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
>> index 34478f0..d4ada4f 100644
>> --- a/hw/scsi/spapr_vscsi.c
>> +++ b/hw/scsi/spapr_vscsi.c
>> @@ -690,7 +690,7 @@ static void vscsi_inquiry_no_target(VSCSIState *s,
>> vscsi_req *req)
>> int rc, len, alen;
>>
>> /* We dont do EVPD. Also check that page_code is 0 */
>> - if ((cdb[1] & 0x01) || (cdb[1] & 0x01) || cdb[2] != 0) {
>> + if ((cdb[1] & 0x01) || cdb[2] != 0) {
>> /* Send INVALID FIELD IN CDB */
>> vscsi_makeup_sense(s, req, ILLEGAL_REQUEST, 0x24, 0);
>> vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
>>
>
> Not trivial---I have no idea if something else was meant to be checked.
Considering the code intends to follow the comment mentioned above if
condition. I think the patch is correct.
Here is reference from SCSI manual.
<manual>
INQUIRY Command
================
EVPD (Enable Vital Product Data) bit
----------------------------------------------
An enable vital product data (EVPD) bit set to one specifies that the
device server shall return the vital product data specified by the
PAGE CODE field (see 3.6.4).
0 ==> If the EVPD bit is set to zero, the device server shall return
the standard INQUIRY data (see 3.6.2). If the PAGE CODE field is not
set to zero when the EVPD bit is set to zero, the command shall be
terminated with CHECK CONDITION status, with the sense key set to
ILLEGAL REQUEST, and the additional sense code set to INVALID FIELD IN
CDB.
1 ==> When the EVPD bit is set to one, the PAGE CODE field specifies
which page of vital product data information the device server shall
return (see 4.4).
</manual>
- According to comment EVPD is not supported. (First part of if condition)
- According to manual if EVPD is 0, PAGE CODE must not be zero. PAGE
CODE is 3rd byte of INQUIRY command (i.e. cmd[2])
Therefore, in my opinion, patch is correct.
Thanks and Regards,
Prasad
>
> Paolo
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH] spapr_vscsi: remove duplicate condition check
@ 2014-03-24 17:19 ` Prasad Joshi
0 siblings, 0 replies; 8+ messages in thread
From: Prasad Joshi @ 2014-03-24 17:19 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-trivial, qemu-ppc, qemu-devel, Stefan Hajnoczi, agraf
On Mon, Mar 24, 2014 at 9:15 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 24/03/2014 16:44, Prasad Joshi ha scritto:
>
>> Signed-off-by: Prasad Joshi <prasadjoshi.linux@gmail.com>
>> ---
>> hw/scsi/spapr_vscsi.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
>> index 34478f0..d4ada4f 100644
>> --- a/hw/scsi/spapr_vscsi.c
>> +++ b/hw/scsi/spapr_vscsi.c
>> @@ -690,7 +690,7 @@ static void vscsi_inquiry_no_target(VSCSIState *s,
>> vscsi_req *req)
>> int rc, len, alen;
>>
>> /* We dont do EVPD. Also check that page_code is 0 */
>> - if ((cdb[1] & 0x01) || (cdb[1] & 0x01) || cdb[2] != 0) {
>> + if ((cdb[1] & 0x01) || cdb[2] != 0) {
>> /* Send INVALID FIELD IN CDB */
>> vscsi_makeup_sense(s, req, ILLEGAL_REQUEST, 0x24, 0);
>> vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
>>
>
> Not trivial---I have no idea if something else was meant to be checked.
Considering the code intends to follow the comment mentioned above if
condition. I think the patch is correct.
Here is reference from SCSI manual.
<manual>
INQUIRY Command
================
EVPD (Enable Vital Product Data) bit
----------------------------------------------
An enable vital product data (EVPD) bit set to one specifies that the
device server shall return the vital product data specified by the
PAGE CODE field (see 3.6.4).
0 ==> If the EVPD bit is set to zero, the device server shall return
the standard INQUIRY data (see 3.6.2). If the PAGE CODE field is not
set to zero when the EVPD bit is set to zero, the command shall be
terminated with CHECK CONDITION status, with the sense key set to
ILLEGAL REQUEST, and the additional sense code set to INVALID FIELD IN
CDB.
1 ==> When the EVPD bit is set to one, the PAGE CODE field specifies
which page of vital product data information the device server shall
return (see 4.4).
</manual>
- According to comment EVPD is not supported. (First part of if condition)
- According to manual if EVPD is 0, PAGE CODE must not be zero. PAGE
CODE is 3rd byte of INQUIRY command (i.e. cmd[2])
Therefore, in my opinion, patch is correct.
Thanks and Regards,
Prasad
>
> Paolo
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-trivial] [PATCH] spapr_vscsi: remove duplicate condition check
2014-03-24 17:19 ` [Qemu-devel] " Prasad Joshi
@ 2014-03-24 17:50 ` Paolo Bonzini
-1 siblings, 0 replies; 8+ messages in thread
From: Paolo Bonzini @ 2014-03-24 17:50 UTC (permalink / raw)
To: Prasad Joshi; +Cc: qemu-trivial, qemu-ppc, qemu-devel, Stefan Hajnoczi, agraf
Il 24/03/2014 18:19, Prasad Joshi ha scritto:
> On Mon, Mar 24, 2014 at 9:15 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 24/03/2014 16:44, Prasad Joshi ha scritto:
>>
>>> Signed-off-by: Prasad Joshi <prasadjoshi.linux@gmail.com>
>>> ---
>>> hw/scsi/spapr_vscsi.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
>>> index 34478f0..d4ada4f 100644
>>> --- a/hw/scsi/spapr_vscsi.c
>>> +++ b/hw/scsi/spapr_vscsi.c
>>> @@ -690,7 +690,7 @@ static void vscsi_inquiry_no_target(VSCSIState *s,
>>> vscsi_req *req)
>>> int rc, len, alen;
>>>
>>> /* We dont do EVPD. Also check that page_code is 0 */
>>> - if ((cdb[1] & 0x01) || (cdb[1] & 0x01) || cdb[2] != 0) {
>>> + if ((cdb[1] & 0x01) || cdb[2] != 0) {
>>> /* Send INVALID FIELD IN CDB */
>>> vscsi_makeup_sense(s, req, ILLEGAL_REQUEST, 0x24, 0);
>>> vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
>>>
>>
>> Not trivial---I have no idea if something else was meant to be checked.
>
> Considering the code intends to follow the comment mentioned above if
> condition. I think the patch is correct.
>
> Here is reference from SCSI manual.
>
> <manual>
> INQUIRY Command
> ================
> EVPD (Enable Vital Product Data) bit
> ----------------------------------------------
> An enable vital product data (EVPD) bit set to one specifies that the
> device server shall return the vital product data specified by the
> PAGE CODE field (see 3.6.4).
>
> 0 ==> If the EVPD bit is set to zero, the device server shall return
> the standard INQUIRY data (see 3.6.2). If the PAGE CODE field is not
> set to zero when the EVPD bit is set to zero, the command shall be
> terminated with CHECK CONDITION status, with the sense key set to
> ILLEGAL REQUEST, and the additional sense code set to INVALID FIELD IN
> CDB.
>
> 1 ==> When the EVPD bit is set to one, the PAGE CODE field specifies
> which page of vital product data information the device server shall
> return (see 4.4).
> </manual>
>
> - According to comment EVPD is not supported. (First part of if condition)
> - According to manual if EVPD is 0, PAGE CODE must not be zero. PAGE
> CODE is 3rd byte of INQUIRY command (i.e. cmd[2])
More important, there's no other field in the CDB that matters (except
for an obsolete on in byte 1/bit 1).
Applied to scsi-next branch, thanks.
Paolo
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH] spapr_vscsi: remove duplicate condition check
@ 2014-03-24 17:50 ` Paolo Bonzini
0 siblings, 0 replies; 8+ messages in thread
From: Paolo Bonzini @ 2014-03-24 17:50 UTC (permalink / raw)
To: Prasad Joshi; +Cc: qemu-trivial, qemu-ppc, qemu-devel, Stefan Hajnoczi, agraf
Il 24/03/2014 18:19, Prasad Joshi ha scritto:
> On Mon, Mar 24, 2014 at 9:15 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 24/03/2014 16:44, Prasad Joshi ha scritto:
>>
>>> Signed-off-by: Prasad Joshi <prasadjoshi.linux@gmail.com>
>>> ---
>>> hw/scsi/spapr_vscsi.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
>>> index 34478f0..d4ada4f 100644
>>> --- a/hw/scsi/spapr_vscsi.c
>>> +++ b/hw/scsi/spapr_vscsi.c
>>> @@ -690,7 +690,7 @@ static void vscsi_inquiry_no_target(VSCSIState *s,
>>> vscsi_req *req)
>>> int rc, len, alen;
>>>
>>> /* We dont do EVPD. Also check that page_code is 0 */
>>> - if ((cdb[1] & 0x01) || (cdb[1] & 0x01) || cdb[2] != 0) {
>>> + if ((cdb[1] & 0x01) || cdb[2] != 0) {
>>> /* Send INVALID FIELD IN CDB */
>>> vscsi_makeup_sense(s, req, ILLEGAL_REQUEST, 0x24, 0);
>>> vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
>>>
>>
>> Not trivial---I have no idea if something else was meant to be checked.
>
> Considering the code intends to follow the comment mentioned above if
> condition. I think the patch is correct.
>
> Here is reference from SCSI manual.
>
> <manual>
> INQUIRY Command
> ================
> EVPD (Enable Vital Product Data) bit
> ----------------------------------------------
> An enable vital product data (EVPD) bit set to one specifies that the
> device server shall return the vital product data specified by the
> PAGE CODE field (see 3.6.4).
>
> 0 ==> If the EVPD bit is set to zero, the device server shall return
> the standard INQUIRY data (see 3.6.2). If the PAGE CODE field is not
> set to zero when the EVPD bit is set to zero, the command shall be
> terminated with CHECK CONDITION status, with the sense key set to
> ILLEGAL REQUEST, and the additional sense code set to INVALID FIELD IN
> CDB.
>
> 1 ==> When the EVPD bit is set to one, the PAGE CODE field specifies
> which page of vital product data information the device server shall
> return (see 4.4).
> </manual>
>
> - According to comment EVPD is not supported. (First part of if condition)
> - According to manual if EVPD is 0, PAGE CODE must not be zero. PAGE
> CODE is 3rd byte of INQUIRY command (i.e. cmd[2])
More important, there's no other field in the CDB that matters (except
for an obsolete on in byte 1/bit 1).
Applied to scsi-next branch, thanks.
Paolo
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-03-24 17:51 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-24 15:44 [Qemu-trivial] [PATCH] spapr_vscsi: remove duplicate condition check Prasad Joshi
2014-03-24 15:44 ` [Qemu-devel] " Prasad Joshi
2014-03-24 15:45 ` [Qemu-trivial] " Paolo Bonzini
2014-03-24 15:45 ` [Qemu-devel] " Paolo Bonzini
2014-03-24 17:19 ` [Qemu-trivial] " Prasad Joshi
2014-03-24 17:19 ` [Qemu-devel] " Prasad Joshi
2014-03-24 17:50 ` [Qemu-trivial] " Paolo Bonzini
2014-03-24 17:50 ` [Qemu-devel] " Paolo Bonzini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.