qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: John Snow <jsnow@redhat.com>
To: Kevin Wolf <kwolf@redhat.com>
Cc: pbonzini@redhat.com, stefanha@redhat.com, hare@suse.de,
	qemu-block@nongnu.org, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 1/2] atapi: abort transfers with 0 byte limits
Date: Mon, 13 Jul 2015 11:09:22 -0400	[thread overview]
Message-ID: <55A3D4A2.3000006@redhat.com> (raw)
In-Reply-To: <20150713101859.GB5893@noname.redhat.com>



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 <jsnow@redhat.com>
> 
> 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 <kwolf@redhat.com>
> 

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

  reply	other threads:[~2015-07-13 15:09 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-11  1:05 [Qemu-devel] [PATCH 0/2] ahci: fix atapi PIO (for 2.4?) John Snow
2015-07-11  1:05 ` [Qemu-devel] [PATCH 1/2] atapi: abort transfers with 0 byte limits John Snow
2015-07-13 10:18   ` Kevin Wolf
2015-07-13 15:09     ` John Snow [this message]
2015-08-25  0:25     ` John Snow
2015-09-01 12:06       ` Kevin Wolf
2015-07-11  1:05 ` [Qemu-devel] [PATCH 2/2] ide: unify io_buffer_offset increments John Snow

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55A3D4A2.3000006@redhat.com \
    --to=jsnow@redhat.com \
    --cc=hare@suse.de \
    --cc=kwolf@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).