qemu-trivial
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-trivial] [PATCH] spapr_vscsi: remove duplicate condition check


From: Prasad Joshi
Subject: Re: [Qemu-trivial] [PATCH] spapr_vscsi: remove duplicate condition check
Date: Mon, 24 Mar 2014 22:49:26 +0530

On Mon, Mar 24, 2014 at 9:15 PM, Paolo Bonzini <address@hidden> wrote:
> Il 24/03/2014 16:44, Prasad Joshi ha scritto:
>
>> Signed-off-by: Prasad Joshi <address@hidden>
>> ---
>>  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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]