From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=33003 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q76lS-0001G4-3t for qemu-devel@nongnu.org; Tue, 05 Apr 2011 09:57:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q76lN-0000fG-4e for qemu-devel@nongnu.org; Tue, 05 Apr 2011 09:57:50 -0400 Received: from mx1.redhat.com ([209.132.183.28]:10121) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q76lM-0000ex-Ow for qemu-devel@nongnu.org; Tue, 05 Apr 2011 09:57:45 -0400 Message-ID: <4D9B2060.3060000@redhat.com> Date: Tue, 05 Apr 2011 16:00:00 +0200 From: Kevin Wolf MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] scsi-generic: Remove bogus double complete References: <1301980051-20866-1-git-send-email-david@gibson.dropbear.id.au> In-Reply-To: <1301980051-20866-1-git-send-email-david@gibson.dropbear.id.au> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: qemu-devel@nongnu.org Am 05.04.2011 07:07, schrieb David Gibson: > From: Ben Herrenschmidt > > scsi-generic scsi_read_complete() should not -both- call the client > complete callback with SCSI_REASON_DATA -and- call > scsi_command_complete(). The former will cause the client to queue a > new read or write request, while the later will free the request data > structure, thus causing the new read or write request to use a > freed/stale structure when it completes. > > This patch fixes the bug, fixing a crash with scsi-generic & RHEL5.5 > installer. > > Signed-off-by: Benjamin Herrenschmidt > Signed-off-by: David Gibson > --- > hw/scsi-generic.c | 2 -- > 1 files changed, 0 insertions(+), 2 deletions(-) > > diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c > index 9be1cca..c06f5df 100644 > --- a/hw/scsi-generic.c > +++ b/hw/scsi-generic.c > @@ -173,8 +173,6 @@ static void scsi_read_complete(void * opaque, int ret) > > r->len = -1; > r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, r->req.tag, len); > - if (len == 0) > - scsi_command_complete(r, 0); > } > > /* Read more data from scsi device into buffer. */ I have a hard time each time I try to understand this SCSI stuff without reading a lot of code and specs. What I would have expected is this: if (len == 0) { scsi_command_complete(r, 0); } else { r->req.bus->complete(r->req.bus, SCSI_REASON_DATA, r->req.tag, len); } This would fix the problem that both functions are called, but still use SCSI_REASON_DONE if no data is transferred. This is similar to what scsi-disk seems to be doing. However, if you can explain to me why your version is more correct, I'll gladly take it. Kevin