Re: [Qemu-devel] [Qemu-ppc] [PATCH] spapr_vscsi: Fix REPORT_LUNS handling
On 01/03/14 08:27, Paolo Bonzini wrote: > Il 02/01/2014 19:23, Nathan Whitehorn ha scritto: Let me try to grasp what you're doing here. You're trying to figure out how many devices there are attached to the bus. For every device you reserve a buffer block. Lun0 is mandatory, all others are optional. First off, I think the code would be easier to grasp if you'd count "number of entries" rather than "number of bytes". That way we don't have to mentally deal with the 8 byte block granularity. Then IIUC you're jumping through a lot of hoops to count lun0 if it's there, but keep it reserved when it's not there. Why don't you just always reserve entry 0 for lun0? In the loop where you're actually filling in data you just skip lun0. Or is lun0 a terminator and always has to come last? > This is simply because you should not report lun 0 twice; even if it is > not defined, LUN 0 is there as a dummy device that only answers a > handful of commands (including INQUIRY and REPORT LUNS). There are many > ways to write it, but unless you use GArray or something like that, it > will look very much like Nathan and hw/scsi/scsi-bus.c's code. > > Paolo I've sent a new version of this patch reflecting the discussion here. Please let me know if I have missed anything. -Nathan
Re: [Qemu-devel] [Qemu-ppc] [PATCH] spapr_vscsi: Fix REPORT_LUNS handling
Il 02/01/2014 19:23, Nathan Whitehorn ha scritto: >>> Let me try to grasp what you're doing here. You're trying to >>> figure out how many devices there are attached to the bus. For >>> every device you reserve a buffer block. Lun0 is mandatory, all >>> others are optional. >>> >>> First off, I think the code would be easier to grasp if you'd >>> count "number of entries" rather than "number of bytes". That way >>> we don't have to mentally deal with the 8 byte block >>> granularity. >>> >>> Then IIUC you're jumping through a lot of hoops to count lun0 if >>> it's there, but keep it reserved when it's not there. Why don't >>> you just always reserve entry 0 for lun0? In the loop where >>> you're actually filling in data you just skip lun0. Or is lun0 a >>> terminator and always has to come last? This is simply because you should not report lun 0 twice; even if it is not defined, LUN 0 is there as a dummy device that only answers a handful of commands (including INQUIRY and REPORT LUNS). There are many ways to write it, but unless you use GArray or something like that, it will look very much like Nathan and hw/scsi/scsi-bus.c's code. Paolo
Re: [Qemu-devel] [Qemu-ppc] [PATCH] spapr_vscsi: Fix REPORT_LUNS handling
Il 02/01/2014 19:14, Nathan Whitehorn ha scritto: >> > He should do something like : >> > resp_data[i] |= dev->id & 0x3f; >> > here to avoid a dev->id > 63 from spilling into the address method field. >> > >> > Or probably should have a check for >> > if dev->id > 3 then fail > OK. No need for that: .max_channel = 7, /* logical unit addressing format */ .max_target = 63, .max_lun = 31, dev->id is thus bounded to 0..63. Paolo
Re: [Qemu-devel] [Qemu-ppc] [PATCH] spapr_vscsi: Fix REPORT_LUNS handling
On 01/02/14 10:41, Alexander Graf wrote:
> On 02.01.2014, at 16:31, Alexander Graf wrote:
>
>> On 18.10.2013, at 14:33, Nathan Whitehorn wrote:
>>
>>> Intercept REPORT_LUNS commands addressed either to SRP LUN 0 or the
>>> well-known
>>> LUN for REPORT_LUNS commands. This is required to implement the SAM and SPC
>>> specifications.
>>>
>>> Since SRP implements only a single SCSI target port per connection, the SRP
>>> target is required to report all available LUNs in response to a REPORT_LUNS
>>> command addressed either to LUN 0 or the well-known LUN. Instead, QEMU was
>>> forwarding such requests to the first QEMU SCSI target, with the result that
>>> initiators that relied on this feature would only see LUNs on the first QEMU
>>> SCSI target.
>>>
>>> Behavior for REPORT_LUNS commands addressed to any other LUN is not
>>> specified
>>> by the standard and so is left unchanged. This preserves behavior under
>>> Linux
>>> and SLOF, which enumerate possible LUNs by hand and so address no commands
>>> either to LUN 0 or the well-known REPORT_LUNS LUN.
>>>
>>> Signed-off-by: Nathan Whitehorn
>> This patch fails on checkpatch.pl. Please fix those warnings up :).
>>
>> WARNING: braces {} are necessary for all arms of this statement
>> #65: FILE: hw/scsi/spapr_vscsi.c:738:
>> +if (dev->channel == 0 && dev->id == 0 && dev->lun == 0)
>> [...]
>>
>> WARNING: braces {} are necessary for all arms of this statement
>> #81: FILE: hw/scsi/spapr_vscsi.c:754:
>> +if (dev->id == 0 && dev->channel == 0)
>> [...]
>> +else
>> [...]
>>
>> WARNING: line over 80 characters
>> #108: FILE: hw/scsi/spapr_vscsi.c:781:
>> +if ((srp->cmd.lun == 0 || be64_to_cpu(srp->cmd.lun) ==
>> SRP_REPORT_LUNS_WLUN) && srp->cmd.cdb[0] == REPORT_LUNS) {
>>
>> total: 0 errors, 3 warnings, 75 lines checked
>>
>> Your patch has style problems, please review. If any of these errors
>> are false positives report them to the maintainer, see
>> CHECKPATCH in MAINTAINERS.
>>
>>> ---
>>> hw/scsi/spapr_vscsi.c | 57
>>> +++
>>> 1 file changed, 57 insertions(+)
>>>
>>> diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
>>> index 2a26042..87e0fb3 100644
>>> --- a/hw/scsi/spapr_vscsi.c
>>> +++ b/hw/scsi/spapr_vscsi.c
>>> @@ -63,6 +63,8 @@
>>> #define SCSI_SENSE_BUF_SIZE 96
>>> #define SRP_RSP_SENSE_DATA_LEN 18
>>>
>>> +#define SRP_REPORT_LUNS_WLUN0xc101000
>>> +
>>> typedef union vscsi_crq {
>>>struct viosrp_crq s;
>>>uint8_t raw[16];
>>> @@ -720,12 +722,67 @@ static void vscsi_inquiry_no_target(VSCSIState *s,
>>> vscsi_req *req)
>>>}
>>> }
>>>
>>> +static void vscsi_report_luns(VSCSIState *s, vscsi_req *req)
>>> +{
>>> +BusChild *kid;
>>> +int i, len, n, rc;
>>> +uint8_t *resp_data;
>>> +bool found_lun0;
>>> +
>>> +n = 0;
>>> +found_lun0 = false;
>>> +QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) {
>>> +SCSIDevice *dev = SCSI_DEVICE(kid->child);
>>> +
>>> +n += 8;
>>> +if (dev->channel == 0 && dev->id == 0 && dev->lun == 0)
>>> +found_lun0 = true;
>>> +}
>>> +if (!found_lun0) {
>>> +n += 8;
>>> +}
>>> +len = n+8;
>> Let me try to grasp what you're doing here. You're trying to figure out how
>> many devices there are attached to the bus. For every device you reserve a
>> buffer block. Lun0 is mandatory, all others are optional.
>>
>> First off, I think the code would be easier to grasp if you'd count "number
>> of entries" rather than "number of bytes". That way we don't have to
>> mentally deal with the 8 byte block granularity.
>>
>> Then IIUC you're jumping through a lot of hoops to count lun0 if it's there,
>> but keep it reserved when it's not there. Why don't you just always reserve
>> entry 0 for lun0? In the loop where you're actually filling in data you just
>> skip lun0. Or is lun0 a terminator and always has to come last?
>>
>>
>>> +
>>> +resp_data = malloc(len);
>> g_malloc0
>>
>>> +memset(resp_data, 0, len);
>>> +stl_be_p(resp_data, n);
>>> +i = found_lun0 ? 8 : 16;
>>> +QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) {
>>> +DeviceState *qdev = kid->child;
>>> +SCSIDevice *dev = SCSI_DEVICE(qdev);
>>> +
>>> +if (dev->id == 0 && dev->channel == 0)
>>> +resp_data[i] = 0;
>>> +else
>>> +resp_data[i] = (2 << 6);
> Ah, I almost forgot this one. Please convert that into something more verbose
> through a define. Whatever that bit means ... :).
I've tried to maintain the existing (questionable) style here. See
vscsi_device_find() for instance. This seemed like a bad occasion for a
global style cleanup.
>>> +resp_data[i] |= dev->id;
>>> +resp_data[i+1] = (dev->channel << 5);
>>> +resp_data[i+1] |= dev->lun;
> What are the other 6 bytes reserved for?
It's the hierarchical LUN fields. vscsi_device_find() defines the LUN
format u
Re: [Qemu-devel] [Qemu-ppc] [PATCH] spapr_vscsi: Fix REPORT_LUNS handling
On 01/02/14 10:56, ronnie sahlberg wrote:
> On Thu, Jan 2, 2014 at 7:41 AM, Alexander Graf wrote:
>> On 02.01.2014, at 16:31, Alexander Graf wrote:
>>
>>> On 18.10.2013, at 14:33, Nathan Whitehorn wrote:
>>>
Intercept REPORT_LUNS commands addressed either to SRP LUN 0 or the
well-known
LUN for REPORT_LUNS commands. This is required to implement the SAM and SPC
specifications.
Since SRP implements only a single SCSI target port per connection, the SRP
target is required to report all available LUNs in response to a
REPORT_LUNS
command addressed either to LUN 0 or the well-known LUN. Instead, QEMU was
forwarding such requests to the first QEMU SCSI target, with the result
that
initiators that relied on this feature would only see LUNs on the first
QEMU
SCSI target.
Behavior for REPORT_LUNS commands addressed to any other LUN is not
specified
by the standard and so is left unchanged. This preserves behavior under
Linux
and SLOF, which enumerate possible LUNs by hand and so address no commands
either to LUN 0 or the well-known REPORT_LUNS LUN.
Signed-off-by: Nathan Whitehorn
>>> This patch fails on checkpatch.pl. Please fix those warnings up :).
>>>
>>> WARNING: braces {} are necessary for all arms of this statement
>>> #65: FILE: hw/scsi/spapr_vscsi.c:738:
>>> +if (dev->channel == 0 && dev->id == 0 && dev->lun == 0)
>>> [...]
>>>
>>> WARNING: braces {} are necessary for all arms of this statement
>>> #81: FILE: hw/scsi/spapr_vscsi.c:754:
>>> +if (dev->id == 0 && dev->channel == 0)
>>> [...]
>>> +else
>>> [...]
>>>
>>> WARNING: line over 80 characters
>>> #108: FILE: hw/scsi/spapr_vscsi.c:781:
>>> +if ((srp->cmd.lun == 0 || be64_to_cpu(srp->cmd.lun) ==
>>> SRP_REPORT_LUNS_WLUN) && srp->cmd.cdb[0] == REPORT_LUNS) {
>>>
>>> total: 0 errors, 3 warnings, 75 lines checked
>>>
>>> Your patch has style problems, please review. If any of these errors
>>> are false positives report them to the maintainer, see
>>> CHECKPATCH in MAINTAINERS.
>>>
---
hw/scsi/spapr_vscsi.c | 57
+++
1 file changed, 57 insertions(+)
diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index 2a26042..87e0fb3 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -63,6 +63,8 @@
#define SCSI_SENSE_BUF_SIZE 96
#define SRP_RSP_SENSE_DATA_LEN 18
+#define SRP_REPORT_LUNS_WLUN0xc101000
+
typedef union vscsi_crq {
struct viosrp_crq s;
uint8_t raw[16];
@@ -720,12 +722,67 @@ static void vscsi_inquiry_no_target(VSCSIState *s,
vscsi_req *req)
}
}
+static void vscsi_report_luns(VSCSIState *s, vscsi_req *req)
+{
+BusChild *kid;
+int i, len, n, rc;
+uint8_t *resp_data;
+bool found_lun0;
+
+n = 0;
+found_lun0 = false;
+QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) {
+SCSIDevice *dev = SCSI_DEVICE(kid->child);
+
+n += 8;
+if (dev->channel == 0 && dev->id == 0 && dev->lun == 0)
+found_lun0 = true;
+}
+if (!found_lun0) {
+n += 8;
+}
+len = n+8;
>>> Let me try to grasp what you're doing here. You're trying to figure out how
>>> many devices there are attached to the bus. For every device you reserve a
>>> buffer block. Lun0 is mandatory, all others are optional.
>>>
>>> First off, I think the code would be easier to grasp if you'd count "number
>>> of entries" rather than "number of bytes". That way we don't have to
>>> mentally deal with the 8 byte block granularity.
>>>
>>> Then IIUC you're jumping through a lot of hoops to count lun0 if it's
>>> there, but keep it reserved when it's not there. Why don't you just always
>>> reserve entry 0 for lun0? In the loop where you're actually filling in data
>>> you just skip lun0. Or is lun0 a terminator and always has to come last?
>>>
>>>
+
+resp_data = malloc(len);
>>> g_malloc0
>>>
+memset(resp_data, 0, len);
+stl_be_p(resp_data, n);
+i = found_lun0 ? 8 : 16;
+QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) {
+DeviceState *qdev = kid->child;
+SCSIDevice *dev = SCSI_DEVICE(qdev);
+
+if (dev->id == 0 && dev->channel == 0)
+resp_data[i] = 0;
+else
+resp_data[i] = (2 << 6);
> This looks odd.
> Shouldn't this rather be
> resp_data[i] = (1 << 6);
> to set the LUN address method to 01b meaning Single Level LUN structure.
> (SAM5 4.7.3)
>
> He is setting the address method to 10b but there is no such address
> method afaik
There is. This uses the "Logical unit addressing" method of SAM5 section
4.7.7.4, following the re
Re: [Qemu-devel] [Qemu-ppc] [PATCH] spapr_vscsi: Fix REPORT_LUNS handling
On Thu, Jan 2, 2014 at 7:41 AM, Alexander Graf wrote:
>
> On 02.01.2014, at 16:31, Alexander Graf wrote:
>
>>
>> On 18.10.2013, at 14:33, Nathan Whitehorn wrote:
>>
>>> Intercept REPORT_LUNS commands addressed either to SRP LUN 0 or the
>>> well-known
>>> LUN for REPORT_LUNS commands. This is required to implement the SAM and SPC
>>> specifications.
>>>
>>> Since SRP implements only a single SCSI target port per connection, the SRP
>>> target is required to report all available LUNs in response to a REPORT_LUNS
>>> command addressed either to LUN 0 or the well-known LUN. Instead, QEMU was
>>> forwarding such requests to the first QEMU SCSI target, with the result that
>>> initiators that relied on this feature would only see LUNs on the first QEMU
>>> SCSI target.
>>>
>>> Behavior for REPORT_LUNS commands addressed to any other LUN is not
>>> specified
>>> by the standard and so is left unchanged. This preserves behavior under
>>> Linux
>>> and SLOF, which enumerate possible LUNs by hand and so address no commands
>>> either to LUN 0 or the well-known REPORT_LUNS LUN.
>>>
>>> Signed-off-by: Nathan Whitehorn
>>
>> This patch fails on checkpatch.pl. Please fix those warnings up :).
>>
>> WARNING: braces {} are necessary for all arms of this statement
>> #65: FILE: hw/scsi/spapr_vscsi.c:738:
>> +if (dev->channel == 0 && dev->id == 0 && dev->lun == 0)
>> [...]
>>
>> WARNING: braces {} are necessary for all arms of this statement
>> #81: FILE: hw/scsi/spapr_vscsi.c:754:
>> +if (dev->id == 0 && dev->channel == 0)
>> [...]
>> +else
>> [...]
>>
>> WARNING: line over 80 characters
>> #108: FILE: hw/scsi/spapr_vscsi.c:781:
>> +if ((srp->cmd.lun == 0 || be64_to_cpu(srp->cmd.lun) ==
>> SRP_REPORT_LUNS_WLUN) && srp->cmd.cdb[0] == REPORT_LUNS) {
>>
>> total: 0 errors, 3 warnings, 75 lines checked
>>
>> Your patch has style problems, please review. If any of these errors
>> are false positives report them to the maintainer, see
>> CHECKPATCH in MAINTAINERS.
>>
>>> ---
>>> hw/scsi/spapr_vscsi.c | 57
>>> +++
>>> 1 file changed, 57 insertions(+)
>>>
>>> diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
>>> index 2a26042..87e0fb3 100644
>>> --- a/hw/scsi/spapr_vscsi.c
>>> +++ b/hw/scsi/spapr_vscsi.c
>>> @@ -63,6 +63,8 @@
>>> #define SCSI_SENSE_BUF_SIZE 96
>>> #define SRP_RSP_SENSE_DATA_LEN 18
>>>
>>> +#define SRP_REPORT_LUNS_WLUN0xc101000
>>> +
>>> typedef union vscsi_crq {
>>>struct viosrp_crq s;
>>>uint8_t raw[16];
>>> @@ -720,12 +722,67 @@ static void vscsi_inquiry_no_target(VSCSIState *s,
>>> vscsi_req *req)
>>>}
>>> }
>>>
>>> +static void vscsi_report_luns(VSCSIState *s, vscsi_req *req)
>>> +{
>>> +BusChild *kid;
>>> +int i, len, n, rc;
>>> +uint8_t *resp_data;
>>> +bool found_lun0;
>>> +
>>> +n = 0;
>>> +found_lun0 = false;
>>> +QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) {
>>> +SCSIDevice *dev = SCSI_DEVICE(kid->child);
>>> +
>>> +n += 8;
>>> +if (dev->channel == 0 && dev->id == 0 && dev->lun == 0)
>>> +found_lun0 = true;
>>> +}
>>> +if (!found_lun0) {
>>> +n += 8;
>>> +}
>>> +len = n+8;
>>
>> Let me try to grasp what you're doing here. You're trying to figure out how
>> many devices there are attached to the bus. For every device you reserve a
>> buffer block. Lun0 is mandatory, all others are optional.
>>
>> First off, I think the code would be easier to grasp if you'd count "number
>> of entries" rather than "number of bytes". That way we don't have to
>> mentally deal with the 8 byte block granularity.
>>
>> Then IIUC you're jumping through a lot of hoops to count lun0 if it's there,
>> but keep it reserved when it's not there. Why don't you just always reserve
>> entry 0 for lun0? In the loop where you're actually filling in data you just
>> skip lun0. Or is lun0 a terminator and always has to come last?
>>
>>
>>> +
>>> +resp_data = malloc(len);
>>
>> g_malloc0
>>
>>> +memset(resp_data, 0, len);
>>> +stl_be_p(resp_data, n);
>>> +i = found_lun0 ? 8 : 16;
>>> +QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) {
>>> +DeviceState *qdev = kid->child;
>>> +SCSIDevice *dev = SCSI_DEVICE(qdev);
>>> +
>>> +if (dev->id == 0 && dev->channel == 0)
>>> +resp_data[i] = 0;
>>> +else
>>> +resp_data[i] = (2 << 6);
This looks odd.
Shouldn't this rather be
resp_data[i] = (1 << 6);
to set the LUN address method to 01b meaning Single Level LUN structure.
(SAM5 4.7.3)
He is setting the address method to 10b but there is no such address
method afaik.
>
> Ah, I almost forgot this one. Please convert that into something more verbose
> through a define. Whatever that bit means ... :).
>
>>> +resp_data[i] |= dev->id;
He should do something like :
resp_data[i] |= dev->id & 0x3f;
here to avoid a dev->id > 63 from spi
Re: [Qemu-devel] [Qemu-ppc] [PATCH] spapr_vscsi: Fix REPORT_LUNS handling
On 02.01.2014, at 16:31, Alexander Graf wrote:
>
> On 18.10.2013, at 14:33, Nathan Whitehorn wrote:
>
>> Intercept REPORT_LUNS commands addressed either to SRP LUN 0 or the
>> well-known
>> LUN for REPORT_LUNS commands. This is required to implement the SAM and SPC
>> specifications.
>>
>> Since SRP implements only a single SCSI target port per connection, the SRP
>> target is required to report all available LUNs in response to a REPORT_LUNS
>> command addressed either to LUN 0 or the well-known LUN. Instead, QEMU was
>> forwarding such requests to the first QEMU SCSI target, with the result that
>> initiators that relied on this feature would only see LUNs on the first QEMU
>> SCSI target.
>>
>> Behavior for REPORT_LUNS commands addressed to any other LUN is not specified
>> by the standard and so is left unchanged. This preserves behavior under Linux
>> and SLOF, which enumerate possible LUNs by hand and so address no commands
>> either to LUN 0 or the well-known REPORT_LUNS LUN.
>>
>> Signed-off-by: Nathan Whitehorn
>
> This patch fails on checkpatch.pl. Please fix those warnings up :).
>
> WARNING: braces {} are necessary for all arms of this statement
> #65: FILE: hw/scsi/spapr_vscsi.c:738:
> +if (dev->channel == 0 && dev->id == 0 && dev->lun == 0)
> [...]
>
> WARNING: braces {} are necessary for all arms of this statement
> #81: FILE: hw/scsi/spapr_vscsi.c:754:
> +if (dev->id == 0 && dev->channel == 0)
> [...]
> +else
> [...]
>
> WARNING: line over 80 characters
> #108: FILE: hw/scsi/spapr_vscsi.c:781:
> +if ((srp->cmd.lun == 0 || be64_to_cpu(srp->cmd.lun) ==
> SRP_REPORT_LUNS_WLUN) && srp->cmd.cdb[0] == REPORT_LUNS) {
>
> total: 0 errors, 3 warnings, 75 lines checked
>
> Your patch has style problems, please review. If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
>
>> ---
>> hw/scsi/spapr_vscsi.c | 57
>> +++
>> 1 file changed, 57 insertions(+)
>>
>> diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
>> index 2a26042..87e0fb3 100644
>> --- a/hw/scsi/spapr_vscsi.c
>> +++ b/hw/scsi/spapr_vscsi.c
>> @@ -63,6 +63,8 @@
>> #define SCSI_SENSE_BUF_SIZE 96
>> #define SRP_RSP_SENSE_DATA_LEN 18
>>
>> +#define SRP_REPORT_LUNS_WLUN0xc101000
>> +
>> typedef union vscsi_crq {
>>struct viosrp_crq s;
>>uint8_t raw[16];
>> @@ -720,12 +722,67 @@ static void vscsi_inquiry_no_target(VSCSIState *s,
>> vscsi_req *req)
>>}
>> }
>>
>> +static void vscsi_report_luns(VSCSIState *s, vscsi_req *req)
>> +{
>> +BusChild *kid;
>> +int i, len, n, rc;
>> +uint8_t *resp_data;
>> +bool found_lun0;
>> +
>> +n = 0;
>> +found_lun0 = false;
>> +QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) {
>> +SCSIDevice *dev = SCSI_DEVICE(kid->child);
>> +
>> +n += 8;
>> +if (dev->channel == 0 && dev->id == 0 && dev->lun == 0)
>> +found_lun0 = true;
>> +}
>> +if (!found_lun0) {
>> +n += 8;
>> +}
>> +len = n+8;
>
> Let me try to grasp what you're doing here. You're trying to figure out how
> many devices there are attached to the bus. For every device you reserve a
> buffer block. Lun0 is mandatory, all others are optional.
>
> First off, I think the code would be easier to grasp if you'd count "number
> of entries" rather than "number of bytes". That way we don't have to mentally
> deal with the 8 byte block granularity.
>
> Then IIUC you're jumping through a lot of hoops to count lun0 if it's there,
> but keep it reserved when it's not there. Why don't you just always reserve
> entry 0 for lun0? In the loop where you're actually filling in data you just
> skip lun0. Or is lun0 a terminator and always has to come last?
>
>
>> +
>> +resp_data = malloc(len);
>
> g_malloc0
>
>> +memset(resp_data, 0, len);
>> +stl_be_p(resp_data, n);
>> +i = found_lun0 ? 8 : 16;
>> +QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) {
>> +DeviceState *qdev = kid->child;
>> +SCSIDevice *dev = SCSI_DEVICE(qdev);
>> +
>> +if (dev->id == 0 && dev->channel == 0)
>> +resp_data[i] = 0;
>> +else
>> +resp_data[i] = (2 << 6);
Ah, I almost forgot this one. Please convert that into something more verbose
through a define. Whatever that bit means ... :).
>> +resp_data[i] |= dev->id;
>> +resp_data[i+1] = (dev->channel << 5);
>> +resp_data[i+1] |= dev->lun;
What are the other 6 bytes reserved for?
Alex
