Re: [Qemu-devel] [PATCH][RFC] Fix bugs in the ATAPI cdrom driver

2007-08-19 Thread Thiemo Seufer
Brandon Philips wrote:
> On 17:42 Fri 17 Aug 2007, Matthew Kent wrote:
> > On Fri, 2007-17-08 at 16:43 -0700, Brandon Philips wrote:
> > > The new libata-eh in the Linux kernel is throwing a fit over the QEMU
> > > cdrom device for two reasons:
> > 
> > Nice thanks, was suffering from this issue as well.
> > 
> > Tested with some linux 2.6.23-rc3 guests, looks great now.
> 
> Great.  I would like to see someone from the QEMU project sign off on
> the correctness of the code too before we carry the patch in our
> package.

Committed.


Thiemo




Re: [Qemu-devel] [PATCH][RFC] Fix bugs in the ATAPI cdrom driver

2007-08-18 Thread Brandon Philips
On 17:42 Fri 17 Aug 2007, Matthew Kent wrote:
> On Fri, 2007-17-08 at 16:43 -0700, Brandon Philips wrote:
> > The new libata-eh in the Linux kernel is throwing a fit over the QEMU
> > cdrom device for two reasons:
> 
> Nice thanks, was suffering from this issue as well.
> 
> Tested with some linux 2.6.23-rc3 guests, looks great now.

Great.  I would like to see someone from the QEMU project sign off on
the correctness of the code too before we carry the patch in our
package.

Thanks for testing!

Brandon




Re: [Qemu-devel] [PATCH][RFC] Fix bugs in the ATAPI cdrom driver

2007-08-17 Thread Matthew Kent
On Fri, 2007-17-08 at 16:43 -0700, Brandon Philips wrote:
> The new libata-eh in the Linux kernel is throwing a fit over the QEMU
> cdrom device for two reasons:

Nice thanks, was suffering from this issue as well.

Tested with some linux 2.6.23-rc3 guests, looks great now.
-- 
Matthew Kent <[EMAIL PROTECTED]>
http://magoazul.com





[Qemu-devel] [PATCH][RFC] Fix bugs in the ATAPI cdrom driver

2007-08-17 Thread Brandon Philips
The new libata-eh in the Linux kernel is throwing a fit over the QEMU
cdrom device for two reasons:

1) DRQ can be set with ERR_STAT set.  This is a violation of the ATAPI
state machine.

2) After a TEST_UNIT_READY ATAPI command is sent ERR_STAT is getting set
which is correct.  But, when the OS issues another ATAPI command
ERR_STAT is still set.  Which is bad since the next expected command
from the OS is REQUEST_SENSE to find out why ERR_STAT is set.

bug this fixes: https://bugzilla.novell.com/show_bug.cgi?id=291775

Signed-off-by: Brandon Philips <[EMAIL PROTECTED]>

---
 hw/ide.c |7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

Index: qemu-0.9.0/hw/ide.c
===
--- qemu-0.9.0.orig/hw/ide.c
+++ qemu-0.9.0/hw/ide.c
@@ -586,7 +586,9 @@ static void ide_transfer_start(IDEState 
 s->end_transfer_func = end_transfer_func;
 s->data_ptr = buf;
 s->data_end = buf + size;
-s->status |= DRQ_STAT;
+/* don't violate the HSM */
+if (!(s->status & ERR_STAT))
+s->status |= DRQ_STAT;
 }
 
 static void ide_transfer_stop(IDEState *s)
@@ -1805,6 +1807,7 @@ static void ide_ioport_write(void *opaqu
 /* overlapping commands not supported */
 if (s->feature & 0x02)
 goto abort_cmd;
+s->status = READY_STAT;
 s->atapi_dma = s->feature & 1;
 s->nsector = 1;
 ide_transfer_start(s, s->io_buffer, ATAPI_PACKET_SIZE,