* [Qemu-devel] [RFC PATCH 1/6] scsi: ignore LUN field in the CDB
2011-05-20 15:03 [Qemu-devel] [RFC PATCH 0/6] SCSI series part 2, rewrite LUN parsing Paolo Bonzini
@ 2011-05-20 15:03 ` Paolo Bonzini
2011-05-20 16:15 ` Christoph Hellwig
2011-05-20 15:03 ` [Qemu-devel] [RFC PATCH 2/6] scsi: support parsing of SAM logical unit numbers Paolo Bonzini
` (5 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Paolo Bonzini @ 2011-05-20 15:03 UTC (permalink / raw)
To: qemu-devel
The LUN field in the CDB is a historical relic. Ignore it as reserved,
which is what modern SCSI specifications actually say.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi-disk.c | 6 +++---
hw/scsi-generic.c | 5 ++---
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 4c7a53e..b14c32f 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -516,7 +516,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
memset(outbuf, 0, buflen);
- if (req->lun || req->cmd.buf[1] >> 5) {
+ if (req->lun) {
outbuf[0] = 0x7f; /* LUN not supported */
return buflen;
}
@@ -1022,9 +1022,9 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
}
#endif
- if (req->lun || buf[1] >> 5) {
+ if (req->lun) {
/* Only LUN 0 supported. */
- DPRINTF("Unimplemented LUN %d\n", req->lun ? req->lun : buf[1] >> 5);
+ DPRINTF("Unimplemented LUN %d\n", req->lun);
if (command != REQUEST_SENSE && command != INQUIRY) {
scsi_command_complete(r, CHECK_CONDITION,
SENSE_CODE(LUN_NOT_SUPPORTED));
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 0c04606..e6f0efd 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -335,9 +335,8 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *cmd)
SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
int ret;
- if (cmd[0] != REQUEST_SENSE &&
- (req->lun != s->lun || (cmd[1] >> 5) != s->lun)) {
- DPRINTF("Unimplemented LUN %d\n", req->lun ? req->lun : cmd[1] >> 5);
+ if (cmd[0] != REQUEST_SENSE && req->lun != s->lun) {
+ DPRINTF("Unimplemented LUN %d\n", req->lun);
scsi_set_sense(s, SENSE_CODE(LUN_NOT_SUPPORTED));
r->req.status = CHECK_CONDITION;
scsi_req_complete(&r->req);
--
1.7.4.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [Qemu-devel] [RFC PATCH 2/6] scsi: support parsing of SAM logical unit numbers
2011-05-20 15:03 [Qemu-devel] [RFC PATCH 0/6] SCSI series part 2, rewrite LUN parsing Paolo Bonzini
2011-05-20 15:03 ` [Qemu-devel] [RFC PATCH 1/6] scsi: ignore LUN field in the CDB Paolo Bonzini
@ 2011-05-20 15:03 ` Paolo Bonzini
2011-05-25 13:05 ` Christoph Hellwig
2011-05-20 15:03 ` [Qemu-devel] [RFC PATCH 3/6] scsi-generic: allow customization of the lun Paolo Bonzini
` (4 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Paolo Bonzini @ 2011-05-20 15:03 UTC (permalink / raw)
To: qemu-devel; +Cc: David Gibson
SAM logical unit numbers are complicated beasts that can address
multiple levels of buses and targets before finally reaching
logical units. Begin supporting them by correctly parsing vSCSI
LUNs.
Note that with the current (admittedly incorrect) code OpenFirmware
thought the devices were at "bus X, target 0, lun 0" (because OF
prefers access mode 0, which places bus numbers in the top byte),
while Linux thought it was "bus 0, target Y, lun 0" (because Linux
uses access mode 2, which places target numbers in the top byte).
With this patch, everything consistently uses the former notation.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi-bus.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
hw/scsi-defs.h | 22 +++++++++++
hw/scsi.h | 7 +++
hw/spapr_vscsi.c | 18 ++-------
4 files changed, 142 insertions(+), 14 deletions(-)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 2f0ffda..70b1092 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -718,3 +718,112 @@ static char *scsibus_get_fw_dev_path(DeviceState *dev)
return strdup(path);
}
+
+/* Decode the bus and level parts of a LUN, as defined in the SCSI architecture
+ model. If false is returned, the LUN could not be parsed. If true
+ is return, "*bus" and "*target" identify the next two steps in the
+ hierarchical LUN.
+
+ "*lun" can be used with scsi_get_lun to continue the parsing. */
+static bool scsi_decode_level(uint64_t sam_lun, int *bus, int *target,
+ uint64_t *lun)
+{
+ switch (sam_lun >> 62) {
+ case ADDR_PERIPHERAL_DEVICE:
+ *bus = (sam_lun >> 56) & 0x3f;
+ if (*bus) {
+ /* The TARGET OR LUN field selects a target; walk the next
+ 16-bits to find the LUN. */
+ *target = (sam_lun >> 48) & 0xff;
+ *lun = sam_lun << 16;
+ } else {
+ /* The TARGET OR LUN field selects a LUN on the current
+ node, identified by bus 0. */
+ *target = 0;
+ *lun = (sam_lun & 0xff000000000000LL) | (1LL << 62);
+ }
+ return true;
+ case ADDR_LOGICAL_UNIT:
+ *bus = (sam_lun >> 53) & 7;
+ *target = (sam_lun >> 56) & 0x3f;
+ *lun = (sam_lun & 0x1f000000000000LL) | (1LL << 62);
+ return true;
+ case ADDR_FLAT_SPACE:
+ *bus = 0;
+ *target = 0;
+ *lun = sam_lun;
+ return true;
+ case ADDR_LOGICAL_UNIT_EXT:
+ if ((sam_lun >> 56) == ADDR_WELL_KNOWN_LUN ||
+ (sam_lun >> 56) == ADDR_FLAT_SPACE_EXT) {
+ *bus = 0;
+ *target = 0;
+ *lun = sam_lun;
+ return true;
+ }
+ return false;
+ }
+ abort();
+}
+
+/* Extract a single-level LUN number from a LUN, as specified in the
+ SCSI architecture model. Return -1 if this is not possible because
+ the LUN includes a bus or target component. */
+static int scsi_get_lun(uint64_t sam_lun)
+{
+ int bus, target;
+
+retry:
+ switch (sam_lun >> 62) {
+ case ADDR_PERIPHERAL_DEVICE:
+ case ADDR_LOGICAL_UNIT:
+ scsi_decode_level(sam_lun, &bus, &target, &sam_lun);
+ if (bus || target) {
+ return LUN_INVALID;
+ }
+ goto retry;
+
+ case ADDR_FLAT_SPACE:
+ return (sam_lun >> 48) & 0x3fff;
+ case ADDR_LOGICAL_UNIT_EXT:
+ if ((sam_lun >> 56) == ADDR_WELL_KNOWN_LUN) {
+ return LUN_WLUN_BASE | ((sam_lun >> 48) & 0xff);
+ }
+ if ((sam_lun >> 56) == ADDR_FLAT_SPACE_EXT) {
+ return (sam_lun >> 32) & 0xffffff;
+ }
+ return LUN_INVALID;
+ }
+ abort();
+}
+
+/* Extract bus and target from the given LUN and use it to identify a
+ SCSIDevice from a SCSIBus. Right now, only 1 target per bus is
+ supported. In the future a SCSIDevice could host its own SCSIBus,
+ in an alternation of devices that select a bus (target ports) and
+ devices that select a target (initiator ports). */
+SCSIDevice *scsi_decode_lun(SCSIBus *sbus, uint64_t sam_lun, int *lun)
+{
+ int bus, target, decoded_lun;
+ uint64_t next_lun;
+
+ if (!scsi_decode_level(sam_lun, &bus, &target, &next_lun)) {
+ /* Unsupported LUN format. */
+ return NULL;
+ }
+ if (bus >= sbus->ndev || (bus == 0 && target > 0)) {
+ /* Out of range. */
+ return NULL;
+ }
+ if (target != 0) {
+ /* Only one target for now. */
+ return NULL;
+ }
+
+ decoded_lun = scsi_get_lun(next_lun);
+ if (decoded_lun != LUN_INVALID) {
+ *lun = decoded_lun;
+ return sbus->devs[bus];
+ }
+ return NULL;
+}
diff --git a/hw/scsi-defs.h b/hw/scsi-defs.h
index 413cce0..66dfd4a 100644
--- a/hw/scsi-defs.h
+++ b/hw/scsi-defs.h
@@ -164,3 +164,25 @@
#define TYPE_ENCLOSURE 0x0d /* Enclosure Services Device */
#define TYPE_NO_LUN 0x7f
+/*
+ * SCSI addressing methods (bits 62-63 of the LUN).
+ */
+#define ADDR_PERIPHERAL_DEVICE 0
+#define ADDR_FLAT_SPACE 1
+#define ADDR_LOGICAL_UNIT 2
+#define ADDR_LOGICAL_UNIT_EXT 3
+
+/*
+ * SCSI extended addressing methods (bits 56-63 of the LUN).
+ */
+#define ADDR_WELL_KNOWN_LUN 0xc1
+#define ADDR_FLAT_SPACE_EXT 0xd2
+
+/*
+ * SCSI well known LUNs (byte 1)
+ */
+#define WLUN_REPORT_LUNS 1
+#define WLUN_ACCESS_CONTROLS 2
+#define WLUN_TARGET_LOG_PAGES 3
+#define WLUN_SECURITY_PROTOCOL 4
+#define WLUN_MANAGEMENT_PROTOCOL 5
diff --git a/hw/scsi.h b/hw/scsi.h
index 93e9d35..aa75b82 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -21,6 +21,12 @@ enum SCSIXferMode {
SCSI_XFER_TO_DEV, /* WRITE, MODE_SELECT, ... */
};
+enum SCSILun {
+ LUN_INVALID = -256,
+ LUN_WLUN_BASE = -256,
+ LUN_WLUN_MASK = 255
+};
+
typedef struct SCSISense {
uint8_t key;
uint8_t asc;
@@ -137,6 +143,7 @@ extern const struct SCSISense sense_code_LUN_FAILURE;
int scsi_build_sense(SCSISense sense, uint8_t *buf, int len, int fixed);
int scsi_sense_valid(SCSISense sense);
+SCSIDevice *scsi_decode_lun(SCSIBus *sbus, uint64_t sam_lun, int *lun);
SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t lun);
SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun);
diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index b195e06..ee88ff6 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -139,13 +139,6 @@ static vscsi_req *vscsi_find_req(VSCSIState *s, SCSIRequest *req)
return &s->reqs[tag];
}
-static void vscsi_decode_id_lun(uint64_t srp_lun, int *id, int *lun)
-{
- /* XXX Figure that one out properly ! This is crackpot */
- *id = (srp_lun >> 56) & 0x7f;
- *lun = (srp_lun >> 48) & 0xff;
-}
-
static int vscsi_send_iu(VSCSIState *s, vscsi_req *req,
uint64_t length, uint8_t format)
{
@@ -645,20 +638,17 @@ static int vscsi_queue_cmd(VSCSIState *s, vscsi_req *req)
{
union srp_iu *srp = &req->iu.srp;
SCSIDevice *sdev;
- int n, id, lun;
+ int n, lun;
- vscsi_decode_id_lun(be64_to_cpu(srp->cmd.lun), &id, &lun);
-
- /* Qemu vs. linux issue with LUNs to be sorted out ... */
- sdev = (id < 8 && lun < 16) ? s->bus.devs[id] : NULL;
+ sdev = scsi_decode_lun(&s->bus, be64_to_cpu(srp->cmd.lun), &lun);
if (!sdev) {
- dprintf("VSCSI: Command for id %d with no drive\n", id);
if (srp->cmd.cdb[0] == INQUIRY) {
vscsi_inquiry_no_target(s, req);
} else {
vscsi_makeup_sense(s, req, ILLEGAL_REQUEST, 0x24, 0x00);
vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
- } return 1;
+ }
+ return 1;
}
req->lun = lun;
--
1.7.4.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [RFC PATCH 2/6] scsi: support parsing of SAM logical unit numbers
2011-05-20 15:03 ` [Qemu-devel] [RFC PATCH 2/6] scsi: support parsing of SAM logical unit numbers Paolo Bonzini
@ 2011-05-25 13:05 ` Christoph Hellwig
0 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2011-05-25 13:05 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, David Gibson
On Fri, May 20, 2011 at 05:03:33PM +0200, Paolo Bonzini wrote:
> SAM logical unit numbers are complicated beasts that can address
> multiple levels of buses and targets before finally reaching
> logical units. Begin supporting them by correctly parsing vSCSI
> LUNs.
>
> Note that with the current (admittedly incorrect) code OpenFirmware
> thought the devices were at "bus X, target 0, lun 0" (because OF
> prefers access mode 0, which places bus numbers in the top byte),
> while Linux thought it was "bus 0, target Y, lun 0" (because Linux
> uses access mode 2, which places target numbers in the top byte).
> With this patch, everything consistently uses the former notation.
Do we actually need the complete LUN encoding scheme? What speaks
against always presenting the flat LUN addressing scheme to guests?
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Qemu-devel] [RFC PATCH 3/6] scsi-generic: allow customization of the lun
2011-05-20 15:03 [Qemu-devel] [RFC PATCH 0/6] SCSI series part 2, rewrite LUN parsing Paolo Bonzini
2011-05-20 15:03 ` [Qemu-devel] [RFC PATCH 1/6] scsi: ignore LUN field in the CDB Paolo Bonzini
2011-05-20 15:03 ` [Qemu-devel] [RFC PATCH 2/6] scsi: support parsing of SAM logical unit numbers Paolo Bonzini
@ 2011-05-20 15:03 ` Paolo Bonzini
2011-05-25 13:10 ` Christoph Hellwig
2011-05-20 15:03 ` [Qemu-devel] [RFC PATCH 4/6] scsi-disk: " Paolo Bonzini
` (3 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Paolo Bonzini @ 2011-05-20 15:03 UTC (permalink / raw)
To: qemu-devel
This allows passthrough of devices with LUN != 0, by redirecting them to
LUN0 in the emulated target.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi-generic.c | 38 +++++++++++++++++++++++++++++++++-----
1 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index e6f0efd..fb38934 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -230,8 +230,11 @@ static void scsi_read_data(SCSIRequest *req)
return;
}
- if (r->req.cmd.buf[0] == REQUEST_SENSE && s->driver_status & SG_ERR_DRIVER_SENSE)
- {
+ switch (r->req.cmd.buf[0]) {
+ case REQUEST_SENSE:
+ if (!(s->driver_status & SG_ERR_DRIVER_SENSE)) {
+ break;
+ }
s->senselen = MIN(r->len, s->senselen);
memcpy(r->buf, s->sensebuf, s->senselen);
r->io_header.driver_status = 0;
@@ -246,6 +249,32 @@ static void scsi_read_data(SCSIRequest *req)
/* Clear sensebuf after REQUEST_SENSE */
scsi_clear_sense(s);
return;
+
+ case REPORT_LUNS:
+ assert(!s->lun);
+ if (r->req.cmd.xfer < 16) {
+ scsi_command_complete(r, -EINVAL);
+ return;
+ }
+ r->io_header.driver_status = 0;
+ r->io_header.status = 0;
+ r->io_header.dxfer_len = 16;
+ r->len = -1;
+ r->buf[3] = 8;
+ scsi_req_data(&r->req, 16);
+ scsi_command_complete(r, 0);
+ return;
+
+ case INQUIRY:
+ if (req->lun != s->lun) {
+ if (r->req.cmd.xfer < 1) {
+ scsi_command_complete(r, -EINVAL);
+ return;
+ }
+ outbuf[0] = 0x7f;
+ return MIN(req->cmd.xfer, SCSI_MAX_INQUIRY_LEN);
+ }
+ break;
}
ret = execute_command(s->bs, r, SG_DXFER_FROM_DEV, scsi_read_complete);
@@ -335,7 +364,7 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *cmd)
SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
int ret;
- if (cmd[0] != REQUEST_SENSE && req->lun != s->lun) {
+ if (cmd[0] != REQUEST_SENSE && cmd[0] != INQUIRY && req->lun != s->lun) {
DPRINTF("Unimplemented LUN %d\n", req->lun);
scsi_set_sense(s, SENSE_CODE(LUN_NOT_SUPPORTED));
r->req.status = CHECK_CONDITION;
@@ -510,8 +539,6 @@ static int scsi_generic_initfn(SCSIDevice *dev)
}
/* define device state */
- s->lun = scsiid.lun;
- DPRINTF("LUN %d\n", s->lun);
s->qdev.type = scsiid.scsi_type;
DPRINTF("device type %d\n", s->qdev.type);
if (s->qdev.type == TYPE_TAPE) {
@@ -552,6 +579,7 @@ static SCSIDeviceInfo scsi_generic_info = {
.get_sense = scsi_get_sense,
.qdev.props = (Property[]) {
DEFINE_BLOCK_PROPERTIES(SCSIGenericState, qdev.conf),
+ DEFINE_PROP_UINT32("lun", SCSIDiskState, lun, 0),
DEFINE_PROP_END_OF_LIST(),
},
};
--
1.7.4.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [RFC PATCH 3/6] scsi-generic: allow customization of the lun
2011-05-20 15:03 ` [Qemu-devel] [RFC PATCH 3/6] scsi-generic: allow customization of the lun Paolo Bonzini
@ 2011-05-25 13:10 ` Christoph Hellwig
2011-05-25 15:20 ` Paolo Bonzini
0 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2011-05-25 13:10 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
On Fri, May 20, 2011 at 05:03:34PM +0200, Paolo Bonzini wrote:
> This allows passthrough of devices with LUN != 0, by redirecting them to
> LUN0 in the emulated target.
I'm not quite sure what this code is for. Each /dev/sg device reresents
a LUN. So if we want to suport multiple LUNs in qemu for devices that
are backed by scsi-generic devices we need to take REPORT_LUNs emulation
into the core scsi code, as any qemu target is completely independent
of the underlying scsi device topology. In fact we could easily
mix generic and scsi-disk LUNs on a single target.
> + case INQUIRY:
> + if (req->lun != s->lun) {
This seems odd. I'd expect the SCSI core to handle the LUN addressing.
For now that is just rejecting wrongs ones, and if multiple LUN
support is added dispatching it to the correct drivers instance.
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [RFC PATCH 3/6] scsi-generic: allow customization of the lun
2011-05-25 13:10 ` Christoph Hellwig
@ 2011-05-25 15:20 ` Paolo Bonzini
2011-05-27 13:04 ` Christoph Hellwig
0 siblings, 1 reply; 18+ messages in thread
From: Paolo Bonzini @ 2011-05-25 15:20 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2487 bytes --]
On 05/25/2011 03:10 PM, Christoph Hellwig wrote:
> On Fri, May 20, 2011 at 05:03:34PM +0200, Paolo Bonzini wrote:
>> This allows passthrough of devices with LUN != 0, by redirecting them to
>> LUN0 in the emulated target.
>
> I'm not quite sure what this code is for. Each /dev/sg device reresents
> a LUN. So if we want to suport multiple LUNs in qemu for devices that
> are backed by scsi-generic devices we need to take REPORT_LUNs emulation
> into the core scsi code, as any qemu target is completely independent
> of the underlying scsi device topology.
Yes, I did that.
The problem this patch solves is as follows. scsi-generic rejects
requests whose LUN does not match the host device's LUN. However, since
right now each scsi-disk and scsi-generic instance _must_ implement the
whole target, this basically makes passthrough impossible when the
device has a LUN that is not 0.
That said, this patch has now a completely different subject and meaning
in my work branch. I attach it FYI.
>> + case INQUIRY:
>> + if (req->lun != s->lun) {
>
> This seems odd. I'd expect the SCSI core to handle the LUN addressing.
> For now that is just rejecting wrongs ones, and if multiple LUN
> support is added dispatching it to the correct drivers instance.
I agree. This case of INQUIRY is needed because (for simplicity and
backwards compatibility) you can hang a scsi-disk or scsi-generic device
directly off the HBA, without the intermediate pseudo-device that
handles dispatching commands and reporting LUNs. In this case, the
scsi-disk and scsi-generic devices see requests for other LUN than
theirs. In the case of INQUIRY and REQUEST SENSE, they must reply too.
A similar case happens with REPORT LUNS. If a device's LUN is non-zero,
the device will be attached to the intermediate pseudo-device, which
will handle REPORT LUNS for it. The simple REPORT LUNS code in
scsi-disk and scsi-generic is only for the case in which the target has
a single LUN. That must be LUN 0 so the reply is hardcoded, and I'm
asserting that the hardcoded reply is correct. Note that I'm not
asserting that the message is _sent_ to LUN 0; I'm asserting that the
device itself is on LUN 0.
I thought about making REPORT LUNS really handled by the core, but it's
a mess because I must put the answer in the buffer that scsi_req_get_buf
will report. The devices currently do not expose to the core a way to
allocate that buffer, or to query its size.
Paolo
[-- Attachment #2: 0001-scsi-generic-fix-passthrough-of-devices-with-LUN-0.patch --]
[-- Type: text/x-patch, Size: 2969 bytes --]
>From 925ced58d6b4a3163e720f5a6a7f9fda12f1f6eb Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Tue, 19 Apr 2011 07:34:07 +0200
Subject: [PATCH] scsi-generic: fix passthrough of devices with LUN != 0
This allows redirecting them to LUN0 in the emulated target.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi-generic.c | 39 +++++++++++++++++++++++++++++++++------
1 files changed, 33 insertions(+), 6 deletions(-)
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 94aca18..1611023 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -60,7 +60,6 @@ struct SCSIGenericState
{
SCSIDevice qdev;
BlockDriverState *bs;
- int lun;
int driver_status;
uint8_t sensebuf[SCSI_SENSE_BUF_SIZE];
uint8_t senselen;
@@ -232,8 +231,11 @@ static void scsi_read_data(SCSIRequest *req)
return;
}
- if (r->req.cmd.buf[0] == REQUEST_SENSE && s->driver_status & SG_ERR_DRIVER_SENSE)
- {
+ switch (r->req.cmd.buf[0]) {
+ case REQUEST_SENSE:
+ if (!(s->driver_status & SG_ERR_DRIVER_SENSE)) {
+ break;
+ }
s->senselen = MIN(r->len, s->senselen);
memcpy(r->buf, s->sensebuf, s->senselen);
r->io_header.driver_status = 0;
@@ -248,6 +250,32 @@ static void scsi_read_data(SCSIRequest *req)
/* Clear sensebuf after REQUEST_SENSE */
scsi_clear_sense(s);
return;
+
+ case REPORT_LUNS:
+ assert(!s->qdev.lun);
+ if (r->req.cmd.xfer < 16) {
+ scsi_command_complete(r, -EINVAL);
+ return;
+ }
+ r->io_header.driver_status = 0;
+ r->io_header.status = 0;
+ r->io_header.dxfer_len = 16;
+ r->len = -1;
+ r->buf[3] = 8;
+ scsi_req_data(&r->req, 16);
+ scsi_command_complete(r, 0);
+ return;
+
+ case INQUIRY:
+ if (req->lun != s->qdev.lun) {
+ if (r->req.cmd.xfer < 1) {
+ scsi_command_complete(r, -EINVAL);
+ return;
+ }
+ r->buf[0] = 0x7f;
+ return;
+ }
+ break;
}
ret = execute_command(s->bs, r, SG_DXFER_FROM_DEV, scsi_read_complete);
@@ -338,7 +366,8 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *cmd)
SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
int ret;
- if (cmd[0] != REQUEST_SENSE && req->lun != s->lun) {
+ if (cmd[0] != REQUEST_SENSE && cmd[0] != INQUIRY &&
+ req->lun != s->qdev.lun) {
DPRINTF("Unimplemented LUN %d\n", req->lun);
scsi_set_sense(s, SENSE_CODE(LUN_NOT_SUPPORTED));
r->req.status = CHECK_CONDITION;
@@ -505,8 +534,6 @@ static int scsi_generic_initfn(SCSIDevice *dev)
}
/* define device state */
- s->lun = scsiid.lun;
- DPRINTF("LUN %d\n", s->lun);
s->qdev.type = scsiid.scsi_type;
DPRINTF("device type %d\n", s->qdev.type);
if (s->qdev.type == TYPE_TAPE) {
--
1.7.4.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [RFC PATCH 3/6] scsi-generic: allow customization of the lun
2011-05-25 15:20 ` Paolo Bonzini
@ 2011-05-27 13:04 ` Christoph Hellwig
2011-05-27 13:31 ` Paolo Bonzini
0 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2011-05-27 13:04 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
On Wed, May 25, 2011 at 05:20:59PM +0200, Paolo Bonzini wrote:
> I agree. This case of INQUIRY is needed because (for simplicity and
> backwards compatibility) you can hang a scsi-disk or scsi-generic device
> directly off the HBA, without the intermediate pseudo-device that handles
> dispatching commands and reporting LUNs. In this case, the scsi-disk and
> scsi-generic devices see requests for other LUN than theirs. In the case
> of INQUIRY and REQUEST SENSE, they must reply too.
Requiring this code in the scsi drivers is a really bad idea. Not only
does it mean duplicating the implementation of REPORT LUNS and the illegal
LUN version of INQUIRY in every scsi LUN handler and the target driver,
but also an inconsitent topology of the qemu-internal objects representing
the SCSI implementation, which is a pretty clear path to all kinds of nast
bugs only showing up for the legacy case some time down the road.
The right way to solve this is to make sure we always have the proper
target object by creating it under the hood for the legacy case.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 3/6] scsi-generic: allow customization of the lun
2011-05-27 13:04 ` Christoph Hellwig
@ 2011-05-27 13:31 ` Paolo Bonzini
0 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2011-05-27 13:31 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: qemu-devel
On 05/27/2011 03:04 PM, Christoph Hellwig wrote:
> Requiring this code in the scsi drivers is a really bad idea. Not only
> does it mean duplicating the implementation of REPORT LUNS and the illegal
> LUN version of INQUIRY in every scsi LUN handler and the target driver,
> but also an inconsitent topology of the qemu-internal objects representing
> the SCSI implementation, which is a pretty clear path to all kinds of nast
> bugs only showing up for the legacy case some time down the road.
>
> The right way to solve this is to make sure we always have the proper
> target object by creating it under the hood for the legacy case.
I know, but this requires changes to the basic qdev layer so I planned
to do this later. Also because for bisectability, to avoid dropping a
huge patch, I first need to work around the legacy cases and then kill them.
Paolo
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Qemu-devel] [RFC PATCH 4/6] scsi-disk: allow customization of the lun
2011-05-20 15:03 [Qemu-devel] [RFC PATCH 0/6] SCSI series part 2, rewrite LUN parsing Paolo Bonzini
` (2 preceding siblings ...)
2011-05-20 15:03 ` [Qemu-devel] [RFC PATCH 3/6] scsi-generic: allow customization of the lun Paolo Bonzini
@ 2011-05-20 15:03 ` Paolo Bonzini
2011-05-25 13:13 ` Christoph Hellwig
2011-05-20 15:03 ` [Qemu-devel] [RFC PATCH 5/6] scsi: let a SCSIDevice have children devices Paolo Bonzini
` (2 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Paolo Bonzini @ 2011-05-20 15:03 UTC (permalink / raw)
To: qemu-devel
This will not work until there is a device that can answer REPORT LUNS
for disks with LUN != 0. However, it provides the infrastructure.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi-disk.c | 17 +++++++++--------
1 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index b14c32f..f41550a 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -66,6 +66,7 @@ struct SCSIDiskState
/* The qemu block layer uses a fixed 512 byte sector size.
This is the number of 512 byte blocks in a single scsi sector. */
int cluster_size;
+ uint32_t lun;
uint32_t removable;
uint64_t max_lba;
QEMUBH *bh;
@@ -516,7 +517,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
memset(outbuf, 0, buflen);
- if (req->lun) {
+ if (req->lun != s->lun) {
outbuf[0] = 0x7f; /* LUN not supported */
return buflen;
}
@@ -955,6 +956,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf)
DPRINTF("Unsupported Service Action In\n");
goto illegal_request;
case REPORT_LUNS:
+ assert(!s->lun);
if (req->cmd.xfer < 16)
goto illegal_request;
memset(outbuf, 0, 16);
@@ -1022,14 +1024,12 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
}
#endif
- if (req->lun) {
- /* Only LUN 0 supported. */
+ if (command != REQUEST_SENSE && command != INQUIRY && req->lun != s->lun) {
+ /* Only one LUN supported. */
DPRINTF("Unimplemented LUN %d\n", req->lun);
- if (command != REQUEST_SENSE && command != INQUIRY) {
- scsi_command_complete(r, CHECK_CONDITION,
- SENSE_CODE(LUN_NOT_SUPPORTED));
- return 0;
- }
+ scsi_command_complete(r, CHECK_CONDITION,
+ SENSE_CODE(LUN_NOT_SUPPORTED));
+ return 0;
}
switch (command) {
case TEST_UNIT_READY:
@@ -1247,6 +1247,7 @@ static SCSIDeviceInfo scsi_disk_info = {
.get_sense = scsi_get_sense,
.qdev.props = (Property[]) {
DEFINE_BLOCK_PROPERTIES(SCSIDiskState, qdev.conf),
+ DEFINE_PROP_UINT32("lun", SCSIDiskState, lun, 0),
DEFINE_PROP_STRING("ver", SCSIDiskState, version),
DEFINE_PROP_STRING("serial", SCSIDiskState, serial),
DEFINE_PROP_BIT("removable", SCSIDiskState, removable, 0, false),
--
1.7.4.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [RFC PATCH 4/6] scsi-disk: allow customization of the lun
2011-05-20 15:03 ` [Qemu-devel] [RFC PATCH 4/6] scsi-disk: " Paolo Bonzini
@ 2011-05-25 13:13 ` Christoph Hellwig
0 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2011-05-25 13:13 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
> case REPORT_LUNS:
> + assert(!s->lun);
Besides REPORT_LUNS really belonging into the core code as mentioned before
the assert seems dangerous to me. What protects a guest from issuing a
REPORT LUNS for a non-zero LUN and hitting this assert? Note that SPC
explicitly allows sending REPORT LUNS to either LUN 0 or the well known
LUN if it exists, even if at least Linux doesn't make use of the latter
yet.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Qemu-devel] [RFC PATCH 5/6] scsi: let a SCSIDevice have children devices
2011-05-20 15:03 [Qemu-devel] [RFC PATCH 0/6] SCSI series part 2, rewrite LUN parsing Paolo Bonzini
` (3 preceding siblings ...)
2011-05-20 15:03 ` [Qemu-devel] [RFC PATCH 4/6] scsi-disk: " Paolo Bonzini
@ 2011-05-20 15:03 ` Paolo Bonzini
2011-05-20 15:03 ` [Qemu-devel] [RFC PATCH 6/6] scsi: add walking of hierarchical LUNs Paolo Bonzini
2011-05-20 16:14 ` [Qemu-devel] [RFC PATCH 0/6] SCSI series part 2, rewrite LUN parsing Christoph Hellwig
6 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2011-05-20 15:03 UTC (permalink / raw)
To: qemu-devel
This provides the infrastructure for simple devices to pick LUNs.
Of course, this will not do anything until there is a device that
can report the existence of those LUNs.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/esp.c | 4 +++-
hw/lsi53c895a.c | 2 +-
hw/scsi-bus.c | 14 ++++++++++++++
hw/scsi.h | 3 +++
4 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/hw/esp.c b/hw/esp.c
index 5a33c67..e5bab76 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -239,12 +239,14 @@ static uint32_t get_cmd(ESPState *s, uint8_t *buf)
static void do_busid_cmd(ESPState *s, uint8_t *buf, uint8_t busid)
{
+ SCSIDevice *dev;
int32_t datalen;
int lun;
DPRINTF("do_busid_cmd: busid 0x%x\n", busid);
lun = busid & 7;
- s->current_req = scsi_req_new(s->current_dev, 0, lun);
+ dev = scsi_find_lun(s->current_dev, lun, buf);
+ s->current_req = scsi_req_new(dev, 0, lun);
datalen = scsi_req_enqueue(s->current_req, buf);
s->ti_size = datalen;
if (datalen != 0) {
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index f291283..c549955 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -780,7 +780,7 @@ static void lsi_do_command(LSIState *s)
s->command_complete = 0;
id = (s->select_tag >> 8) & 0xf;
- dev = s->bus.devs[id];
+ dev = scsi_find_lun(s->bus.devs[id], s->current_lun, buf);
if (!dev) {
lsi_bad_selection(s, id);
return;
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 70b1092..4d46831 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -719,6 +719,20 @@ static char *scsibus_get_fw_dev_path(DeviceState *dev)
return strdup(path);
}
+/* Simplified walk of the SCSI bus hierarchy, for devices that only support
+ one bus and only flat-space LUNs (typically 3-bit ones!). */
+SCSIDevice *scsi_find_lun(SCSIDevice *sdev, int lun, uint8_t *cdb)
+{
+ SCSIBus *sbus = sdev->children;
+ if (!sbus ||
+ (lun == 0 && cdb[1] == REPORT_LUNS) ||
+ lun >= sbus->ndev || sbus->devs[lun] == NULL) {
+ return sdev;
+ } else {
+ return sbus->devs[lun];
+ }
+}
+
/* Decode the bus and level parts of a LUN, as defined in the SCSI architecture
model. If false is returned, the LUN could not be parsed. If true
is return, "*bus" and "*target" identify the next two steps in the
diff --git a/hw/scsi.h b/hw/scsi.h
index aa75b82..438dd89 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -58,6 +58,7 @@ struct SCSIDevice
uint32_t id;
BlockConf conf;
SCSIDeviceInfo *info;
+ SCSIBus *children;
QTAILQ_HEAD(, SCSIRequest) requests;
int blocksize;
int type;
@@ -143,7 +144,9 @@ extern const struct SCSISense sense_code_LUN_FAILURE;
int scsi_build_sense(SCSISense sense, uint8_t *buf, int len, int fixed);
int scsi_sense_valid(SCSISense sense);
+
SCSIDevice *scsi_decode_lun(SCSIBus *sbus, uint64_t sam_lun, int *lun);
+SCSIDevice *scsi_find_lun(SCSIDevice *sdev, int lun, uint8_t *cdb);
SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t lun);
SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun);
--
1.7.4.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* [Qemu-devel] [RFC PATCH 6/6] scsi: add walking of hierarchical LUNs
2011-05-20 15:03 [Qemu-devel] [RFC PATCH 0/6] SCSI series part 2, rewrite LUN parsing Paolo Bonzini
` (4 preceding siblings ...)
2011-05-20 15:03 ` [Qemu-devel] [RFC PATCH 5/6] scsi: let a SCSIDevice have children devices Paolo Bonzini
@ 2011-05-20 15:03 ` Paolo Bonzini
2011-05-20 16:14 ` [Qemu-devel] [RFC PATCH 0/6] SCSI series part 2, rewrite LUN parsing Christoph Hellwig
6 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2011-05-20 15:03 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi-bus.c | 79 +++++++++++++++++++++++++++++++++++++++++++-----------
hw/scsi.h | 9 +++++-
hw/spapr_vscsi.c | 6 +++-
3 files changed, 75 insertions(+), 19 deletions(-)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 4d46831..2037da3 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -811,33 +811,80 @@ retry:
abort();
}
-/* Extract bus and target from the given LUN and use it to identify a
- SCSIDevice from a SCSIBus. Right now, only 1 target per bus is
- supported. In the future a SCSIDevice could host its own SCSIBus,
- in an alternation of devices that select a bus (target ports) and
- devices that select a target (initiator ports). */
-SCSIDevice *scsi_decode_lun(SCSIBus *sbus, uint64_t sam_lun, int *lun)
+/* Reusable implementation of the decode_lun entry in SCSIBusOps. */
+SCSIDevice *scsi_decode_bus_from_lun(SCSIBus *sbus, uint64_t sam_lun,
+ uint64_t *next_lun)
{
- int bus, target, decoded_lun;
- uint64_t next_lun;
+ int bus, target;
+ uint64_t my_next_lun;
+ SCSIDevice *sdev;
- if (!scsi_decode_level(sam_lun, &bus, &target, &next_lun)) {
+ if (!scsi_decode_level(sam_lun, &bus, &target, &my_next_lun)) {
/* Unsupported LUN format. */
return NULL;
}
- if (bus >= sbus->ndev || (bus == 0 && target > 0)) {
+ if (bus >= sbus->ndev) {
/* Out of range. */
return NULL;
}
- if (target != 0) {
- /* Only one target for now. */
+
+ sdev = sbus->devs[bus];
+ if (!sdev) {
+ return NULL;
+ } else if (bus == 0 || !sdev->children) {
+ return target ? NULL : sdev;
+ } else {
+ /* Next we'll decode the target, so pass down the same LUN we got. */
+ return sdev->children->ops.decode_lun(sbus, sam_lun, next_lun);
+ }
+}
+
+SCSIDevice *scsi_decode_target_from_lun(SCSIBus *sbus, uint64_t sam_lun,
+ uint64_t *next_lun)
+{
+ int bus, target;
+ SCSIDevice *sdev;
+
+ if (!scsi_decode_level(sam_lun, &bus, &target, next_lun)) {
+ /* Unsupported LUN format. */
+ return NULL;
+ }
+ if (target >= sbus->ndev) {
+ /* Out of range. */
return NULL;
}
+ sdev = sbus->devs[target];
+ if (!sdev || !sdev->children || (*next_lun >> 56) == ADDR_WELL_KNOWN_LUN) {
+ return sdev;
+ } else {
+ return sdev->children->ops.decode_lun(sbus, *next_lun, next_lun);
+ }
+}
+
+/* Extract bus and target from the given LUN and use it to identify a
+ SCSIDevice from a SCSIBus. Right now, only 1 target per bus is
+ supported. In the future a SCSIDevice could host its own SCSIBus,
+ in an alternation of devices that select a bus (target ports) and
+ devices that select a target (initiator ports). */
+SCSIDevice *scsi_decode_lun(SCSIBus *sbus, uint64_t sam_lun,
+ uint8_t *cdb, int *lun)
+{
+ int decoded_lun;
+ uint64_t next_lun;
+ SCSIDevice *sdev;
+
+ sdev = sbus->ops.decode_lun(sbus, sam_lun, &next_lun);
+ if (!sdev) {
+ return NULL;
+ }
decoded_lun = scsi_get_lun(next_lun);
- if (decoded_lun != LUN_INVALID) {
- *lun = decoded_lun;
- return sbus->devs[bus];
+ if (decoded_lun == LUN_INVALID) {
+ return NULL;
+ }
+ if ((decoded_lun & ~LUN_WLUN_MASK) == LUN_WLUN_BASE) {
+ return sdev;
}
- return NULL;
+ *lun = decoded_lun;
+ return scsi_find_lun(sdev, decoded_lun, cdb);
}
diff --git a/hw/scsi.h b/hw/scsi.h
index 438dd89..c4cca0b 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -88,6 +88,8 @@ struct SCSIBusOps {
void (*transfer_data)(SCSIRequest *req, uint32_t arg);
void (*complete)(SCSIRequest *req, uint32_t arg);
void (*cancel)(SCSIRequest *req);
+ SCSIDevice *(*decode_lun)(SCSIBus *sbus, uint64_t sam_lun,
+ uint64_t *next_lun);
};
struct SCSIBus {
@@ -145,7 +147,12 @@ extern const struct SCSISense sense_code_LUN_FAILURE;
int scsi_build_sense(SCSISense sense, uint8_t *buf, int len, int fixed);
int scsi_sense_valid(SCSISense sense);
-SCSIDevice *scsi_decode_lun(SCSIBus *sbus, uint64_t sam_lun, int *lun);
+SCSIDevice *scsi_decode_lun(SCSIBus *sbus, uint64_t sam_lun, uint8_t *cdb,
+ int *lun);
+SCSIDevice *scsi_decode_bus_from_lun(SCSIBus *sbus, uint64_t sam_lun,
+ uint64_t *next_lun);
+SCSIDevice *scsi_decode_target_from_lun(SCSIBus *sbus, uint64_t sam_lun,
+ uint64_t *next_lun);
SCSIDevice *scsi_find_lun(SCSIDevice *sdev, int lun, uint8_t *cdb);
SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t lun);
diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index ee88ff6..d46ab30 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -640,7 +640,8 @@ static int vscsi_queue_cmd(VSCSIState *s, vscsi_req *req)
SCSIDevice *sdev;
int n, lun;
- sdev = scsi_decode_lun(&s->bus, be64_to_cpu(srp->cmd.lun), &lun);
+ sdev = scsi_decode_lun(&s->bus, be64_to_cpu(srp->cmd.lun),
+ srp->cmd.cdb, &lun);
if (!sdev) {
if (srp->cmd.cdb[0] == INQUIRY) {
vscsi_inquiry_no_target(s, req);
@@ -918,7 +919,8 @@ static int vscsi_do_crq(struct VIOsPAPRDevice *dev, uint8_t *crq_data)
static struct SCSIBusOps vscsi_scsi_ops = {
.transfer_data = vscsi_transfer_data,
.complete = vscsi_command_complete,
- .cancel = vscsi_request_cancelled
+ .cancel = vscsi_request_cancelled,
+ .decode_lun = scsi_decode_bus_from_lun
};
static int spapr_vscsi_init(VIOsPAPRDevice *dev)
--
1.7.4.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [RFC PATCH 0/6] SCSI series part 2, rewrite LUN parsing
2011-05-20 15:03 [Qemu-devel] [RFC PATCH 0/6] SCSI series part 2, rewrite LUN parsing Paolo Bonzini
` (5 preceding siblings ...)
2011-05-20 15:03 ` [Qemu-devel] [RFC PATCH 6/6] scsi: add walking of hierarchical LUNs Paolo Bonzini
@ 2011-05-20 16:14 ` Christoph Hellwig
2011-05-20 17:37 ` Paolo Bonzini
6 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2011-05-20 16:14 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
On Fri, May 20, 2011 at 05:03:31PM +0200, Paolo Bonzini wrote:
> This is the second part of my SCSI work. The first is still pending
> and this one is incomplete, but I still would like to get opinions
> early enough because this design directly affects the UI.
>
> This series is half of the work that is necessary to support multiple
> LUNs behind a target. The idea is to have two devices, "scsi-path"
> and "scsi-target", each of which provides both a SCSIDevice and a
> SCSIBus.
I don't quite understand what you mean with path here. It doesn't
seem to map to any SAM concept, nor does it seem to be related
to traditional multipathing.
Can you explain what a path is supposed to be, and why it's called a "path"?
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [RFC PATCH 0/6] SCSI series part 2, rewrite LUN parsing
2011-05-20 16:14 ` [Qemu-devel] [RFC PATCH 0/6] SCSI series part 2, rewrite LUN parsing Christoph Hellwig
@ 2011-05-20 17:37 ` Paolo Bonzini
2011-05-25 13:17 ` Christoph Hellwig
0 siblings, 1 reply; 18+ messages in thread
From: Paolo Bonzini @ 2011-05-20 17:37 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: qemu-devel
On 05/20/2011 06:14 PM, Christoph Hellwig wrote:
> I don't quite understand what you mean with path here. It doesn't
> seem to map to any SAM concept, nor does it seem to be related
> to traditional multipathing.
It's what SAM calls a "bus identifier" in the description of LUN
addressing modes.
> Can you explain what a path is supposed to be, and why it's called a "path"?
It's a SAM "BUS IDENTIFIER", but bus was too confusing with respect to
qdev's BusState (which represents either a SAM bus identifier or
target). I think the term "path" comes from Windows, see for example
http://msdn.microsoft.com/en-us/library/ff564699%28v=vs.85%29.aspx:
PathId [in, optional]
Indicates the SCSI port or bus for the request. This parameter is
optional.
... but I chose it because I found it also in SAM: "The BUS IDENTIFIER
field identifies the bus or path that the SCSI device shall use to relay
the received command or task management function".
I might also call it scsi-initiator which is consistent from the idea
that it sits between two targets ports.
Paolo
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [RFC PATCH 0/6] SCSI series part 2, rewrite LUN parsing
2011-05-20 17:37 ` Paolo Bonzini
@ 2011-05-25 13:17 ` Christoph Hellwig
2011-05-25 15:08 ` Paolo Bonzini
0 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2011-05-25 13:17 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
On Fri, May 20, 2011 at 07:37:30PM +0200, Paolo Bonzini wrote:
> On 05/20/2011 06:14 PM, Christoph Hellwig wrote:
>> I don't quite understand what you mean with path here. It doesn't
>> seem to map to any SAM concept, nor does it seem to be related
>> to traditional multipathing.
>
> It's what SAM calls a "bus identifier" in the description of LUN addressing
> modes.
Ok, so it more or less translates to the concept of a "channel" in
the Linux SCSI subsystem. It would help if you could explain the concept
in a bit more detail in a comment. Or in fact not bother with it at all,
which should be easy if we never present hierachial LUNs.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 0/6] SCSI series part 2, rewrite LUN parsing
2011-05-25 13:17 ` Christoph Hellwig
@ 2011-05-25 15:08 ` Paolo Bonzini
0 siblings, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2011-05-25 15:08 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: qemu-devel
On 05/25/2011 03:17 PM, Christoph Hellwig wrote:
> On Fri, May 20, 2011 at 07:37:30PM +0200, Paolo Bonzini wrote:
>> On 05/20/2011 06:14 PM, Christoph Hellwig wrote:
>>> I don't quite understand what you mean with path here. It doesn't
>>> seem to map to any SAM concept, nor does it seem to be related
>>> to traditional multipathing.
>>
>> It's what SAM calls a "bus identifier" in the description of LUN addressing
>> modes.
>
> Ok, so it more or less translates to the concept of a "channel" in
> the Linux SCSI subsystem.
Yes.
> It would help if you could explain the concept
> in a bit more detail in a comment. Or in fact not bother with it at all,
> which should be easy if we never present hierachial LUNs.
Unfortunately spapr_vscsi requires hierarchical LUNs. The OpenFirmware
code starts by sending out INQUIRY messages to channel 0/1/2/3/4/5/6/7
target 0 LUN 0 (it doesn't recognize other targets or LUNs as far as I
can see). So if you want to have a CD-ROM and a HD on your virtual
machine, and you want your HD to keep its name both during the
installation process and afterwards, you pretty much have to use channel
0 for the HD and another channel for the CD-ROM..
Paolo
^ permalink raw reply [flat|nested] 18+ messages in thread