From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60659) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZEfMI-0004HB-7W for qemu-devel@nongnu.org; Mon, 13 Jul 2015 11:09:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZEfMH-0004jd-Ex for qemu-devel@nongnu.org; Mon, 13 Jul 2015 11:09:30 -0400 Message-ID: <55A3D4A2.3000006@redhat.com> Date: Mon, 13 Jul 2015 11:09:22 -0400 From: John Snow MIME-Version: 1.0 References: <1436576710-10414-1-git-send-email-jsnow@redhat.com> <1436576710-10414-2-git-send-email-jsnow@redhat.com> <20150713101859.GB5893@noname.redhat.com> In-Reply-To: <20150713101859.GB5893@noname.redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/2] atapi: abort transfers with 0 byte limits List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: pbonzini@redhat.com, stefanha@redhat.com, hare@suse.de, qemu-block@nongnu.org, qemu-devel@nongnu.org On 07/13/2015 06:18 AM, Kevin Wolf wrote: > Am 11.07.2015 um 03:05 hat John Snow geschrieben: >> We're supposed to abort on transfers like this, unless we fill >> Word 125 of our IDENTIFY data with a default transfer size, which >> we don't currently do. >> >> This is an ATA error, not a SCSI/ATAPI one. >> See ATA8-ACS3 sections 7.17.6.49 or 7.21.5. >> >> If we don't do this, QEMU will loop forever trying to transfer >> zero bytes, which isn't particularly useful. >> >> Signed-off-by: John Snow > > Of course, one could argue that for a clean separation between the ATA > core and SCSI/ATAPI, this check should really be done in cmd_packet and > ide_abort_command() could stay static. And in fact, I do think it's > worthwhile to try getting a better separation in the mid term. > > However, for a 2.4 fix, this patch might be the right thing to do. > > The interesting part is that the SCSI command is already partially > handled before the error is returned. The required assumption here is > that the guest doesn't actually see any intermediate state between > issuing the command and getting the abort, so it doesn't notice that we > already started the command successfully. I _think_ that assumption > holds true. > > Reviewed-by: Kevin Wolf > Yes, in theory it should be handled in cmd_packet, but it needs to know in advance all of the commands that obey the byte count limit and that information didn't seem easy to extract from the specification. This is indeed a hack, but it's effective for now. --js