[Qemu-devel] Re: [PATCH] scsi-generic: bugfixes for 'SCSIRequest' conversion

[Nicholas A. Bellinger]

On Wed, 2010-11-24 at 11:16 +0100, Hannes Reinecke wrote:
> On 11/24/2010 10:04 AM, Kevin Wolf wrote:
> > Am 24.11.2010 09:57, schrieb Hannes Reinecke:
> >> On 11/24/2010 09:18 AM, Nicholas A. Bellinger wrote:
> >>> From: Nicholas Bellinger <address@hidden>
> >>>
> >>> This patch adds a handful of bugfixes for scsi-generic
> >>> that where added into:
> >>>
> >>> commit a4194b3f79a85e111f000788ddec05d465748851
> >>> Author: Hannes Reinecke <address@hidden>
> >>> Date:   Mon Nov 22 15:39:33 2010 -0800
> >>>
> >>>     scsi: Use 'SCSIRequest' directly
> >>>
> >>> this includes:
> >>>
> >>> *) Fix incorrect errno usage in switch() statement within
> >>>    scsi_command_complete()
> >>>
> >>> *) Remove bogus scsi_command_complete() for residual case
> >>>    within scsi_read_complete()
> >>>
> >>> *) Remove incorrect '-' sign from return in scsi_send_command()
> >>>
> >>> Tested with .37-rc2 TCM_Loop FILEIO backstores on KVM host into
> >>> Debian Lenny v2.6.26 KVM guest with an xfs filesystem mount.
> >>>
> >>> Signed-off-by: Nicholas A. Bellinger address@hidden>
> > 
> > Hannes, can you fold the necessary parts of this into the next version
> > of your series, so that we don't break things first and fix them only later?
> > 
> I'll be folding the first one into my series.
> 
> The second one is actually a genuine error, and a rather old one to
> boot. It went in with this commit:
> 
> commit 89c0f6438d16ebceccdcd096bbc0b5536146a443
> Author: aurel32 <address@hidden>
> Date:   Fri Oct 17 08:08:56 2008 +0000
> Subject: scsi-generic: correct error management
> 
> So I'd rather have it submitted separately.
> And a second opinion is _definitely_ required here.
> nab, can you do the honours?
> 

Yep, so it appears that commit 89c0f6438d16 did introduce the bogus
'double complete' in scsi_read_complete, which I think was intended to
handle residual counts for TYPE_TYPE...

 /* Cancel a pending data transfer.  */
@@ -251,6 +257,8 @@ static void scsi_read_complete(void * opaque, int ret)

     r->len = -1;
     s->completion(s->opaque, SCSI_REASON_DATA, r->tag, len);
+    if (len == 0)
+        scsi_command_complete(r, 0);
 }

I am currently under the assumption for this and bsg_read_complete that
s->completion(..., len) is handling the residual count back to block.

Is this correct..?

--nab