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
>From 925ced58d6b4a3163e720f5a6a7f9fda12f1f6eb Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonz...@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 <pbonz...@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
