From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:60795) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QPFtV-00019T-5f for qemu-devel@nongnu.org; Wed, 25 May 2011 11:21:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QPFtO-0004gG-JW for qemu-devel@nongnu.org; Wed, 25 May 2011 11:21:09 -0400 Received: from mail-ww0-f53.google.com ([74.125.82.53]:60596) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QPFtO-0004g1-9a for qemu-devel@nongnu.org; Wed, 25 May 2011 11:21:02 -0400 Received: by wwj40 with SMTP id 40so7705597wwj.10 for ; Wed, 25 May 2011 08:21:01 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <4DDD1E5B.4010900@redhat.com> Date: Wed, 25 May 2011 17:20:59 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1305903817-25476-1-git-send-email-pbonzini@redhat.com> <1305903817-25476-4-git-send-email-pbonzini@redhat.com> <20110525131049.GB2283@lst.de> In-Reply-To: <20110525131049.GB2283@lst.de> Content-Type: multipart/mixed; boundary="------------080002050003030502000206" Subject: Re: [Qemu-devel] [RFC PATCH 3/6] scsi-generic: allow customization of the lun List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Christoph Hellwig Cc: qemu-devel@nongnu.org This is a multi-part message in MIME format. --------------080002050003030502000206 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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 --------------080002050003030502000206 Content-Type: text/x-patch; name="0001-scsi-generic-fix-passthrough-of-devices-with-LUN-0.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0001-scsi-generic-fix-passthrough-of-devices-with-LUN-0.patc"; filename*1="h" >>From 925ced58d6b4a3163e720f5a6a7f9fda12f1f6eb Mon Sep 17 00:00:00 2001 From: Paolo Bonzini 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 --- 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 --------------080002050003030502000206--