From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=42959 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PLB7b-0006lV-00 for qemu-devel@nongnu.org; Wed, 24 Nov 2010 03:54:37 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PLB7Z-0001IP-Qv for qemu-devel@nongnu.org; Wed, 24 Nov 2010 03:54:34 -0500 Received: from cantor2.suse.de ([195.135.220.15]:48839 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PLB7Z-0001ID-Cf for qemu-devel@nongnu.org; Wed, 24 Nov 2010 03:54:33 -0500 Message-ID: <4CECD36E.50401@suse.de> Date: Wed, 24 Nov 2010 09:57:18 +0100 From: Hannes Reinecke MIME-Version: 1.0 References: <1290586723-8724-1-git-send-email-nab@linux-iscsi.org> In-Reply-To: <1290586723-8724-1-git-send-email-nab@linux-iscsi.org> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: [Qemu-devel] Re: [PATCH] scsi-generic: bugfixes for 'SCSIRequest' conversion List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: linux-iscsi-target-dev@googlegroups.com Cc: Kevin Wolf , Stefan Hajnoczi , qemu-devel , "Nicholas A. Bellinger" , Gerd Hoffmann , Paolo Bonzini On 11/24/2010 09:18 AM, Nicholas A. Bellinger wrote: > From: Nicholas Bellinger >=20 > This patch adds a handful of bugfixes for scsi-generic > that where added into: >=20 > commit a4194b3f79a85e111f000788ddec05d465748851 > Author: Hannes Reinecke > Date: Mon Nov 22 15:39:33 2010 -0800 >=20 > scsi: Use 'SCSIRequest' directly >=20 > this includes: >=20 > *) Fix incorrect errno usage in switch() statement within > scsi_command_complete() >=20 > *) Remove bogus scsi_command_complete() for residual case > within scsi_read_complete() >=20 > *) Remove incorrect '-' sign from return in scsi_send_command() >=20 > Tested with .37-rc2 TCM_Loop FILEIO backstores on KVM host into > Debian Lenny v2.6.26 KVM guest with an xfs filesystem mount. >=20 > Signed-off-by: Nicholas A. Bellinger nab@linux-iscsi.org> > --- > hw/scsi-generic.c | 14 ++++++-------- > 1 files changed, 6 insertions(+), 8 deletions(-) >=20 > diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c > index 7d30115..dc277cc 100644 > --- a/hw/scsi-generic.c > +++ b/hw/scsi-generic.c > @@ -146,13 +146,13 @@ static void scsi_command_complete(void *opaque, i= nt ret) > =20 > if (ret !=3D 0) { > switch(ret) { > - case ENODEV: > + case -ENODEV: > s->senselen =3D scsi_set_sense(s, SENSE_CODE(LUN_NOT_S= UPPORTED)); > break; > - case EINVAL: > + case -EINVAL: > s->senselen =3D scsi_set_sense(s, SENSE_CODE(INVALID_F= IELD)); > break; > - case EBADR: > + case -EBADR: > s->senselen =3D scsi_set_sense(s, SENSE_CODE(TARGET_FA= ILURE)); > break; > default: Oh. Correct. Although we could do a 'switch (-ret)' here. > @@ -230,12 +230,10 @@ static void scsi_read_complete(void * opaque, int= ret) > return; > } > len =3D r->io_header.dxfer_len - r->io_header.resid; > - DPRINTF("Data ready tag=3D0x%x len=3D%d\n", r->req.tag, len); > + DPRINTF("Data ready tag=3D0x%x remaining len=3D%d\n", r->req.tag, = len); > =20 > r->len =3D -1; > r->req.bus->complete(&r->req, SCSI_REASON_DATA, len); > - if (len =3D=3D 0) > - scsi_command_complete(r, 0); > } > =20 > /* Read more data from scsi device into buffer. */ Yes, that's correct (after a fashion). Difference is that we're having to do one more callback into scsi_read_data() with this hunk. But ok, ack. > @@ -375,7 +373,7 @@ static int32_t scsi_send_command(SCSIRequest *req, = uint8_t *cmd) > } > scsi_req_fixup(&r->req); > =20 > - DPRINTF("Command: lun=3D%d tag=3D0x%x len %zd data=3D0x%02x", lun,= tag, > + DPRINTF("Command: lun=3D%d tag=3D0x%x len %zd data=3D0x%02x", req-= >lun, req->tag, > r->req.cmd.xfer, cmd[0]); > =20 > #ifdef DEBUG_SCSI > @@ -414,7 +412,7 @@ static int32_t scsi_send_command(SCSIRequest *req, = uint8_t *cmd) > r->len =3D r->req.cmd.xfer; > if (r->req.cmd.mode =3D=3D SCSI_XFER_TO_DEV) { > r->len =3D 0; > - return -r->req.cmd.xfer; > + return r->req.cmd.xfer; > } > =20 > return r->req.cmd.xfer; NACK. Returning a negative value here means we're about to execute a write. Cf comment at the start of the function: /* Execute a scsi command. Returns the length of the data expected by the command. This will be Positive for data transfers from the device (eg. disk reads), negative for transfers to the device (eg. disk writes), and zero if the command does not transfer any data. */ Cheers, Hannes --=20 Dr. Hannes Reinecke zSeries & Storage hare@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg GF: Markus Rex, HRB 16746 (AG N=FCrnberg)