[Qemu-devel] [PATCH] atapi: Add 'medium ready' to 'medium not ready' transition on cd change

[Qemu-devel] [PATCH] atapi: Add 'medium ready' to 'medium not ready' transition on cd change

[Amit Shah]

MMC-5 Table F.1 lists errors that can be thrown for the TEST_UNIT_READY
command.  Going from medium not ready to medium ready states is
communicated by throwing an error.

This adds the missing 'tray opened' event that we fail to report to
guests.  After doing this, older Linux guests properly revalidate a disc
on the change command.  HSM violation errors, which caused Linux guests
to do a soft-reset of the link, also go away:

ata2.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6
sr 1:0:0:0: CDB: Test Unit Ready: 00 00 00 00 00 00
ata2.00: cmd a0/00:00:00:00:00/00:00:00:00:00/a0 tag 0
         res 01/60:00:00:00:00/00:00:00:00:00/a0 Emask 0x3 (HSM violation)
ata2.00: status: { ERR }
ata2: soft resetting link
ata2.00: configured for MWDMA2
ata2: EH complete

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/ide/core.c |   20 ++++++++++++++++----
 1 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index f028ddb..d8c613a 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1248,12 +1248,19 @@ static void ide_atapi_cmd(IDEState *s)
         ide_atapi_cmd_check_status(s);
         return;
     }
+    if (bdrv_is_inserted(s->bs) && s->cdrom_changed) {
+        ide_atapi_cmd_error(s, SENSE_NOT_READY, ASC_MEDIUM_NOT_PRESENT);
+
+        s->cdrom_changed = 0;
+        s->sense_key = SENSE_UNIT_ATTENTION;
+        s->asc = ASC_MEDIUM_MAY_HAVE_CHANGED;
+        return;
+    }
     switch(s->io_buffer[0]) {
     case GPCMD_TEST_UNIT_READY:
-        if (bdrv_is_inserted(s->bs) && !s->cdrom_changed) {
+        if (bdrv_is_inserted(s->bs)) {
             ide_atapi_cmd_ok(s);
         } else {
-            s->cdrom_changed = 0;
             ide_atapi_cmd_error(s, SENSE_NOT_READY,
                                 ASC_MEDIUM_NOT_PRESENT);
         }
@@ -1734,8 +1741,13 @@ static void cdrom_change_cb(void *opaque, int reason)
     bdrv_get_geometry(s->bs, &nb_sectors);
     s->nb_sectors = nb_sectors;
 
-    s->sense_key = SENSE_UNIT_ATTENTION;
-    s->asc = ASC_MEDIUM_MAY_HAVE_CHANGED;
+    /*
+     * First indicate to the guest that a CD has been removed.  That's
+     * done on the next command the guest sends us.
+     *
+     * Then we set SENSE_UNIT_ATTENTION, by which the guest will
+     * detect a new CD in the drive.  See ide_atapi_cmd() for details.
+     */
     s->cdrom_changed = 1;
     s->events.new_media = true;
     ide_set_irq(s->bus);
-- 
1.7.4.4

Re: [Qemu-devel] [PATCH] atapi: Add 'medium ready' to 'medium not ready' transition on cd change

[Jes Sorensen]

On 04/18/11 13:45, Amit Shah wrote:
> MMC-5 Table F.1 lists errors that can be thrown for the TEST_UNIT_READY
> command.  Going from medium not ready to medium ready states is
> communicated by throwing an error.
> 
> This adds the missing 'tray opened' event that we fail to report to
> guests.  After doing this, older Linux guests properly revalidate a disc
> on the change command.  HSM violation errors, which caused Linux guests
> to do a soft-reset of the link, also go away:
> 
> ata2.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6
> sr 1:0:0:0: CDB: Test Unit Ready: 00 00 00 00 00 00
> ata2.00: cmd a0/00:00:00:00:00/00:00:00:00:00/a0 tag 0
>          res 01/60:00:00:00:00/00:00:00:00:00/a0 Emask 0x3 (HSM violation)
> ata2.00: status: { ERR }
> ata2: soft resetting link
> ata2.00: configured for MWDMA2
> ata2: EH complete
> 
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
>  hw/ide/core.c |   20 ++++++++++++++++----
>  1 files changed, 16 insertions(+), 4 deletions(-)

Looks good to me.

Acked-by: Jes Sorensen <Jes.Sorensen@redhat.com>

Re: [Qemu-devel] [PATCH] atapi: Add 'medium ready' to 'medium not ready' transition on cd change

[Kevin Wolf]

Am 18.04.2011 13:45, schrieb Amit Shah:
> MMC-5 Table F.1 lists errors that can be thrown for the TEST_UNIT_READY
> command.  Going from medium not ready to medium ready states is
> communicated by throwing an error.
> 
> This adds the missing 'tray opened' event that we fail to report to
> guests.  After doing this, older Linux guests properly revalidate a disc
> on the change command.  HSM violation errors, which caused Linux guests
> to do a soft-reset of the link, also go away:
> 
> ata2.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6
> sr 1:0:0:0: CDB: Test Unit Ready: 00 00 00 00 00 00
> ata2.00: cmd a0/00:00:00:00:00/00:00:00:00:00/a0 tag 0
>          res 01/60:00:00:00:00/00:00:00:00:00/a0 Emask 0x3 (HSM violation)
> ata2.00: status: { ERR }
> ata2: soft resetting link
> ata2.00: configured for MWDMA2
> ata2: EH complete
> 
> Signed-off-by: Amit Shah <amit.shah@redhat.com>

Thanks, applied to the block branch.

Besides the tests that you did, Win 7 seems to work well with it, too,
and I almost like the patch, so it's probably the best that we can get
for now.

Kevin

Re: [Qemu-devel] [PATCH] atapi: Add 'medium ready' to 'medium not ready' transition on cd change

[Markus Armbruster]

Amit Shah <amit.shah@redhat.com> writes:

> MMC-5 Table F.1 lists errors that can be thrown for the TEST_UNIT_READY
> command.  Going from medium not ready to medium ready states is
> communicated by throwing an error.
>
> This adds the missing 'tray opened' event that we fail to report to
> guests.  After doing this, older Linux guests properly revalidate a disc
> on the change command.  HSM violation errors, which caused Linux guests
> to do a soft-reset of the link, also go away:
>
> ata2.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6
> sr 1:0:0:0: CDB: Test Unit Ready: 00 00 00 00 00 00
> ata2.00: cmd a0/00:00:00:00:00/00:00:00:00:00/a0 tag 0
>          res 01/60:00:00:00:00/00:00:00:00:00/a0 Emask 0x3 (HSM violation)
> ata2.00: status: { ERR }
> ata2: soft resetting link
> ata2.00: configured for MWDMA2
> ata2: EH complete
>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>

Linux is happy with it in my tests.

Tested-by: Markus Armbruster <armbru@redhat.com>