[Qemu-devel] CD-ROM bug round-up

[Qemu-devel] CD-ROM bug round-up

[Stefan Hajnoczi]

Several of us have been investigating CD-ROM bugs.  Let's update each
other, make sure we're not duplicating effort, and see if we can help
each other make progress.

= Stefan =

Guests do not notice media change when using Linux host CD-ROM
pass-through for two reasons:
1. QEMU is caching the device size for removable media and never updating it.
2. On Linux hosts the /dev/cdrom file descriptor needs to be reopened
to refresh the underlying device size.

Patches have been posted to qemu-devel.

Libvirt does not recognize eject failure due to locked tray.  This
causes libvirt to get out-of-sync and believe the medium was
successfully changed.  A libvirt fix is in progress:
https://bugzilla.redhat.com/show_bug.cgi?id=689101

= Anthony =

Using 'change' or scripted commands without a delay between closing
the BlockDriverState and opening the new CD-ROM.  The guest does not
have a chance to poll the drive and determine a medium present ->
medium not present transition followed by a medium not present ->
medium present transition.

A fix for this does not exist yet and might be tricky since we need to
delay or wait for the guest to poll once before opening the new
CD-ROM.

= Amit =

Found that 2.6.38 and later guest kernels fail to report media change.
 The new in-kernel media change polling framework issues ATAPI
commands which are not implemented in hw/ide/core.c.

= Juan =

ATA HSM violation errors after live migration while CD-ROM read
operations.  This is probably a hw/ide/* or DMA migration bug.

= Markus =

?

Stefan

Re: [Qemu-devel] CD-ROM bug round-up

[Gleb Natapov]

On Wed, Mar 30, 2011 at 05:40:40PM +0100, Stefan Hajnoczi wrote:
> Several of us have been investigating CD-ROM bugs.  Let's update each
> other, make sure we're not duplicating effort, and see if we can help
> each other make progress.
> 
> = Stefan =
> 
> Guests do not notice media change when using Linux host CD-ROM
> pass-through for two reasons:
> 1. QEMU is caching the device size for removable media and never updating it.
> 2. On Linux hosts the /dev/cdrom file descriptor needs to be reopened
> to refresh the underlying device size.
> 
> Patches have been posted to qemu-devel.
> 
> Libvirt does not recognize eject failure due to locked tray.  This
> causes libvirt to get out-of-sync and believe the medium was
> successfully changed.  A libvirt fix is in progress:
> https://bugzilla.redhat.com/show_bug.cgi?id=689101
> 
> = Anthony =
> 
> Using 'change' or scripted commands without a delay between closing
> the BlockDriverState and opening the new CD-ROM.  The guest does not
> have a chance to poll the drive and determine a medium present ->
> medium not present transition followed by a medium not present ->
> medium present transition.
> 
> A fix for this does not exist yet and might be tricky since we need to
> delay or wait for the guest to poll once before opening the new
> CD-ROM.
> 
This sounds familiar and should have been fixed by commit
93c8cfd9e67a62711b86f4c93747566885eb7928

> = Amit =
> 
> Found that 2.6.38 and later guest kernels fail to report media change.
>  The new in-kernel media change polling framework issues ATAPI
> commands which are not implemented in hw/ide/core.c.
> 
> = Juan =
> 
> ATA HSM violation errors after live migration while CD-ROM read
> operations.  This is probably a hw/ide/* or DMA migration bug.
> 
> = Markus =
> 
> ?
> 
> Stefan

--
			Gleb.

Re: [Qemu-devel] CD-ROM bug round-up

[Stefan Hajnoczi]

On Wed, Mar 30, 2011 at 5:50 PM, Gleb Natapov <gleb@redhat.com> wrote:
> On Wed, Mar 30, 2011 at 05:40:40PM +0100, Stefan Hajnoczi wrote:
>> = Anthony =
>>
>> Using 'change' or scripted commands without a delay between closing
>> the BlockDriverState and opening the new CD-ROM.  The guest does not
>> have a chance to poll the drive and determine a medium present ->
>> medium not present transition followed by a medium not present ->
>> medium present transition.
>>
>> A fix for this does not exist yet and might be tricky since we need to
>> delay or wait for the guest to poll once before opening the new
>> CD-ROM.
>>
> This sounds familiar and should have been fixed by commit
> 93c8cfd9e67a62711b86f4c93747566885eb7928

Excellent, thanks for pointing this out.  We need to figure out
whether this mechanism is insufficient for some guests.

On a Windows-related note, I've found that Windows 2003 Server sends
START STOP UNIT to eject the medium without unlocking the tray first.
When the CD-ROM is passed through to a Linux host CD-ROM drive the
eject ioctl fails if the tray is locked.  I haven't investigated this
more yet.

Stefan

[Qemu-devel] Re: CD-ROM bug round-up

[Amit Shah]

On (Wed) 30 Mar 2011 [17:40:40], Stefan Hajnoczi wrote:
> Several of us have been investigating CD-ROM bugs.  Let's update each
> other, make sure we're not duplicating effort, and see if we can help
> each other make progress.
> 
> = Stefan =
> 
> Guests do not notice media change when using Linux host CD-ROM
> pass-through for two reasons:
> 1. QEMU is caching the device size for removable media and never updating it.
> 2. On Linux hosts the /dev/cdrom file descriptor needs to be reopened
> to refresh the underlying device size.
> 
> Patches have been posted to qemu-devel.
> 
> Libvirt does not recognize eject failure due to locked tray.  This
> causes libvirt to get out-of-sync and believe the medium was
> successfully changed.  A libvirt fix is in progress:
> https://bugzilla.redhat.com/show_bug.cgi?id=689101
> 
> = Anthony =
> 
> Using 'change' or scripted commands without a delay between closing
> the BlockDriverState and opening the new CD-ROM.  The guest does not
> have a chance to poll the drive and determine a medium present ->
> medium not present transition followed by a medium not present ->
> medium present transition.
> 
> A fix for this does not exist yet and might be tricky since we need to
> delay or wait for the guest to poll once before opening the new
> CD-ROM.

It could be this, it could also be that our media_present/not-present
notifications aren't proper (more info below):

> = Amit =
> 
> Found that 2.6.38 and later guest kernels fail to report media change.

It's not 2.6.38 and later; it's somewhere around the time libata was
introduced and CDROM emulation went from ide to scsi in the kernel.
All current kernels (2.6.32 onwards) have this behaviour.

Also, this looks like it's due to multiple emulation bugs in QEMU:

- Command 46 (GET_CONFIGURATION) handles only specific cases.  The
  ones that aren't handled are the ones invoked by the guest kernel.

- Command 0 (TEST_UNIT_READY) replies with an error message if there's
  no medium instead of sending a reply with the no medium message.
  This causes the guest to do a soft-reset (invoking the HSM violation
  error message).

- Command 4a (GET_EVENT_STATUS_NOTIFICATION) is not implemented by us
  at all.

All these commands are marked as mandatory to be implemented by devices.

The commit Gleb pointed to in another message in this thread may have
something to do with media change not getting propogated as well:


    case GPCMD_TEST_UNIT_READY:
        if (bdrv_is_inserted(s->bs) && !s->cdrom_changed) {
            ide_atapi_cmd_ok(s);
        } else {
            s->cdrom_changed = 0;
            ide_atapi_cmd_error(s, SENSE_NOT_READY,
                                ASC_MEDIUM_NOT_PRESENT);
        }
        break;

Here, if cdrom got changed, we return an error message to the guest
instead of sending a UNIT_READY with UNIT_ATTENTION message.  So the
guest does its soft-reset thing, due to which the guest never then
notices the UNIT_ATTENTION pending message, causing the guest to think
it's still the same media in the device.

>  The new in-kernel media change polling framework issues ATAPI
> commands which are not implemented in hw/ide/core.c.

The in-kernel media change polling has nothing to do with our
emulation or the commands, btw.

> = Juan =
> 
> ATA HSM violation errors after live migration while CD-ROM read
> operations.  This is probably a hw/ide/* or DMA migration bug.

It could be a combination of the above and some migration state.

Overall, looks like we don't implement some mandatory commands from
the spec, causing the guest to try to bail out, and that makes the
guest lose some notifications.  The 'mount cd once, doesn't get
recognised, mount it again, it gets recognised' thing could be related
to this as well -- the bug Anthony talks about earlier.

For now, if we implement the above three commands properly, we would
already be a lot better.  We may then come across other commands that
might be needed to be implemented, but we can worry about that later.

		Amit

Re: [Qemu-devel] CD-ROM bug round-up

[Markus Armbruster]

Stefan Hajnoczi <stefanha@gmail.com> writes:

> Several of us have been investigating CD-ROM bugs.  Let's update each
> other, make sure we're not duplicating effort, and see if we can help
> each other make progress.
>
> = Stefan =
>
> Guests do not notice media change when using Linux host CD-ROM
> pass-through for two reasons:
> 1. QEMU is caching the device size for removable media and never updating it.
> 2. On Linux hosts the /dev/cdrom file descriptor needs to be reopened
> to refresh the underlying device size.
>
> Patches have been posted to qemu-devel.
>
> Libvirt does not recognize eject failure due to locked tray.  This
> causes libvirt to get out-of-sync and believe the medium was
> successfully changed.  A libvirt fix is in progress:
> https://bugzilla.redhat.com/show_bug.cgi?id=689101
>
> = Anthony =
>
> Using 'change' or scripted commands without a delay between closing
> the BlockDriverState and opening the new CD-ROM.  The guest does not
> have a chance to poll the drive and determine a medium present ->
> medium not present transition followed by a medium not present ->
> medium present transition.
>
> A fix for this does not exist yet and might be tricky since we need to
> delay or wait for the guest to poll once before opening the new
> CD-ROM.
>
> = Amit =
>
> Found that 2.6.38 and later guest kernels fail to report media change.
>  The new in-kernel media change polling framework issues ATAPI
> commands which are not implemented in hw/ide/core.c.
>
> = Juan =
>
> ATA HSM violation errors after live migration while CD-ROM read
> operations.  This is probably a hw/ide/* or DMA migration bug.

Note Amit's bug also triggers ATA HSM errors.  Might be related.

> = Markus =
>
> ?

I've wrestled with a few issues:

* udev choked on unimplemented GPCMD_READ_DISC_INFO.  udev maintainer
  fixed the bad error recovery.

* Lika Amit and Juan, I ran into the ATA HSM errors after media change.

* anaconda sometimes fails to recognize an install disk after the media
  check or a media change.  Could be related to media change bugs /
  unimplemented commands.

* Need to wrap this up:

  Subject: [RFC PATCH] block: Fix eject -f for locked devices
  Date: Fri, 18 Feb 2011 16:16:07 +0100
  Message-ID: <m3oc69jruw.fsf@blackfin.pond.sub.org>

A common theme seems to be the incomplete emulation.  I figure we should
make an effort to implement the mandatory parts of the relevant specs.
Guest software can't be expected to handle missing mandatory features
gracefully.

[Qemu-devel] Re: CD-ROM bug round-up

[Stefan Hajnoczi]

On Thu, Mar 31, 2011 at 10:53 AM, Amit Shah <amit.shah@redhat.com> wrote:
>> = Amit =
>>
>> Found that 2.6.38 and later guest kernels fail to report media change.
>
> It's not 2.6.38 and later; it's somewhere around the time libata was
> introduced and CDROM emulation went from ide to scsi in the kernel.
> All current kernels (2.6.32 onwards) have this behaviour.
>
> Also, this looks like it's due to multiple emulation bugs in QEMU:
>
> - Command 46 (GET_CONFIGURATION) handles only specific cases.  The
>  ones that aren't handled are the ones invoked by the guest kernel.
>
> - Command 0 (TEST_UNIT_READY) replies with an error message if there's
>  no medium instead of sending a reply with the no medium message.
>  This causes the guest to do a soft-reset (invoking the HSM violation
>  error message).
>
> - Command 4a (GET_EVENT_STATUS_NOTIFICATION) is not implemented by us
>  at all.
>
> All these commands are marked as mandatory to be implemented by devices.
>
> The commit Gleb pointed to in another message in this thread may have
> something to do with media change not getting propogated as well:
>
>
>    case GPCMD_TEST_UNIT_READY:
>        if (bdrv_is_inserted(s->bs) && !s->cdrom_changed) {
>            ide_atapi_cmd_ok(s);
>        } else {
>            s->cdrom_changed = 0;
>            ide_atapi_cmd_error(s, SENSE_NOT_READY,
>                                ASC_MEDIUM_NOT_PRESENT);
>        }
>        break;
>
> Here, if cdrom got changed, we return an error message to the guest
> instead of sending a UNIT_READY with UNIT_ATTENTION message.  So the
> guest does its soft-reset thing, due to which the guest never then
> notices the UNIT_ATTENTION pending message, causing the guest to think
> it's still the same media in the device.

Linux 2.6.32 sr_media_change() expects the ASC_MEDIUM_NOT_PRESENT error:

        retval = sr_test_unit_ready(cd->device, sshdr);
        if (retval || (scsi_sense_valid(sshdr) &&
                       /* 0x3a is medium not present */
                       sshdr->asc == 0x3a)) {
                /* Media not present or unable to test, unit probably not
                 * ready. This usually means there is no disc in the drive.
                 * Mark as changed, and we will figure it out later once
                 * the drive is available again.
                 */
                cd->device->changed = 1;

Next time sr_media_change() is called and TUR returns success it
notices the medium has been changed.  This code path is invoked from
cdrom_open(), i.e. every time hald/udisks polls the CD-ROM device.

My understanding is that this mechanism works under QEMU today.  Can
you help me understand what I'm missing?

>>  The new in-kernel media change polling framework issues ATAPI
>> commands which are not implemented in hw/ide/core.c.
>
> The in-kernel media change polling has nothing to do with our
> emulation or the commands, btw.

Yes it does, it the in-kernel polling relies on
GET_EVENT_STATUS_NOTIFICATION which was previously not critical (TUR
was used instead as mentioned above).

Stefan

[Qemu-devel] [RFC PATCH 1/3] ide: Move GET_EVENT_STATUS_NOTIFICATION command handling to its own function

[Amit Shah]

This makes the code more readable.

Also, there's a block like:

if () {
  ...
} else {
  ...
}

Split that into

if () {
  ...
  return;
}
...

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

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 007a4ee..e14f2d3 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1084,6 +1084,29 @@ static int ide_dvd_read_structure(IDEState *s, int format,
     }
 }
 
+static void handle_get_event_status_notification(IDEState *s,
+                                                 uint8_t *buf,
+                                                 const uint8_t *packet)
+{
+    unsigned int max_len;
+
+    max_len = ube16_to_cpu(packet + 7);
+
+    if (!(packet[1] & 0x01)) { /* asynchronous mode */
+        /* Only polling is supported, asynchronous mode is not. */
+        ide_atapi_cmd_error(s, SENSE_ILLEGAL_REQUEST,
+                            ASC_INV_FIELD_IN_CMD_PACKET);
+        return;
+    }
+
+    /* polling */
+    /* We don't support any event class (yet). */
+    cpu_to_ube16(buf, 0x00); /* No event descriptor returned */
+    buf[2] = 0x80;           /* No Event Available (NEA) */
+    buf[3] = 0x00;           /* Empty supported event classes */
+    ide_atapi_cmd_reply(s, 4, max_len);
+}
+
 static void ide_atapi_cmd(IDEState *s)
 {
     const uint8_t *packet;
@@ -1522,19 +1545,7 @@ static void ide_atapi_cmd(IDEState *s)
             break;
         }
     case GPCMD_GET_EVENT_STATUS_NOTIFICATION:
-        max_len = ube16_to_cpu(packet + 7);
-
-        if (packet[1] & 0x01) { /* polling */
-            /* We don't support any event class (yet). */
-            cpu_to_ube16(buf, 0x00); /* No event descriptor returned */
-            buf[2] = 0x80;           /* No Event Available (NEA) */
-            buf[3] = 0x00;           /* Empty supported event classes */
-            ide_atapi_cmd_reply(s, 4, max_len);
-        } else { /* asynchronous mode */
-            /* Only polling is supported, asynchronous mode is not. */
-            ide_atapi_cmd_error(s, SENSE_ILLEGAL_REQUEST,
-                                ASC_INV_FIELD_IN_CMD_PACKET);
-        }
+        handle_get_event_status_notification(s, buf, packet);
         break;
     default:
         ide_atapi_cmd_error(s, SENSE_ILLEGAL_REQUEST,
-- 
1.7.4

[Qemu-devel] [RFC PATCH 2/3] atapi: implement GET_EVENT_STATUS_NOTIFICATION command

[Amit Shah]

This commit could be split further; but the main thing it does is
implement this command.  Linux kernels (at least upto 2.6.38.2) need a
fix to the media change handling to recognise size changes.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/ide/core.c     |  138 ++++++++++++++++++++++++++++++++++++++++++++++++----
 hw/ide/internal.h |    6 ++
 2 files changed, 133 insertions(+), 11 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index e14f2d3..89e212b 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1084,27 +1084,142 @@ static int ide_dvd_read_structure(IDEState *s, int format,
     }
 }
 
+static unsigned int event_status_media(IDEState *s,
+                                       uint8_t *buf,
+                                       unsigned int max_len)
+{
+    enum media_event_code {
+        no_change = 0,       /* Status unchanged */
+        eject_requested,     /* received a request from user to eject */
+        new_media,           /* new media inserted and ready for access */
+        media_removal,       /* only for media changers */
+        media_changed,       /* only for media changers */
+        bg_format_completed, /* MRW or DVD+RW b/g format completed */
+        bg_format_restarted, /* MRW or DVD+RW b/g format restarted */
+    };
+    uint8_t event_code, media_status;
+
+    /* TODO: ensure max_len is >= 6 */
+
+    /* Event notification descriptor */
+    event_code = no_change;
+    if (s->events.new_media) {
+        event_code = new_media;
+        s->events.new_media = false;
+    }
+    /*
+     * We never report a 'door or tray opened' state; it's assumed
+     * always closed.
+     */
+    media_status = bdrv_is_inserted(s->bs) ? 2 : 0;
+
+    buf[4] = event_code;
+    buf[5] = media_status;
+
+    return 6; /* We wrote to just 2 extra bytes from the header */
+}
+
+static unsigned int event_status_nea(uint8_t *buf, unsigned int max_len)
+{
+    unsigned int used_len;
+
+    /*
+     * Ensure we don't write on memory we don't have.
+     * max_len of 0 should not produce an error as well.
+     */
+    used_len = 0;
+
+    /* No event descriptor returned */
+    if (max_len > 0) {
+        buf[0] = 0;
+        used_len++;
+    }
+    if (max_len > 1) {
+        buf[1] = 0;
+        used_len++;
+    }
+
+    if (max_len > 3) {
+        buf[2] = 0x80;           /* No Event Available (NEA) */
+        used_len++;
+    }
+    if (max_len > 3) {
+        buf[3] = 0x00;           /* Empty supported event classes */
+        used_len++;
+    }
+    return used_len;
+}
+
 static void handle_get_event_status_notification(IDEState *s,
                                                  uint8_t *buf,
                                                  const uint8_t *packet)
 {
-    unsigned int max_len;
-
-    max_len = ube16_to_cpu(packet + 7);
-
-    if (!(packet[1] & 0x01)) { /* asynchronous mode */
+    enum cdb {
+        polled = 1,
+        request = 4,
+        allocation_length_msb = 7,
+        allocation_length_lsb = 8,
+        control = 9,
+    };
+    enum notification_class_request_type {
+        reserved1 = 1 << 0,
+        operational_change = 1 << 1,
+        power_management = 1 << 2,
+        external_request = 1 << 3,
+        media = 1 << 4,
+        multi_host = 1 << 5,
+        device_busy = 1 << 6,
+        reserved2 = 1 << 7,
+    };
+    enum event_notification_class_field {
+        enc_no_events = 0,
+        enc_operational_change,
+        enc_power_management,
+        enc_external_request,
+        enc_media,
+        enc_multiple_hosts,
+        enc_device_busy,
+        enc_reserved,
+    };
+    unsigned int max_len, used_len;
+    unsigned int supported_events;
+
+    max_len = ube16_to_cpu(packet + allocation_length_msb);
+
+    /* It is fine by the MMC spec to not support async mode operations */
+    if (!(packet[polled] & 0x01)) { /* asynchronous mode */
         /* Only polling is supported, asynchronous mode is not. */
         ide_atapi_cmd_error(s, SENSE_ILLEGAL_REQUEST,
                             ASC_INV_FIELD_IN_CMD_PACKET);
         return;
     }
 
-    /* polling */
-    /* We don't support any event class (yet). */
-    cpu_to_ube16(buf, 0x00); /* No event descriptor returned */
-    buf[2] = 0x80;           /* No Event Available (NEA) */
-    buf[3] = 0x00;           /* Empty supported event classes */
-    ide_atapi_cmd_reply(s, 4, max_len);
+    /* polling mode operation */
+
+    /*
+     * These are the supported events.
+     *
+     * We currently only support requests of the 'media' type.
+     */
+    supported_events = media;
+
+    /*
+     * Responses to requests are to be based on request priority.  The
+     * notification_class_request_type enum above specifies the
+     * priority: upper elements are higher prio than lower ones.
+     */
+    if (packet[request] & media) {
+
+        /* Event notification header */
+        cpu_to_ube16(buf, max_len);
+        buf[2] = enc_media;
+        buf[3] = supported_events;
+
+        used_len = event_status_media(s, buf, max_len);
+    } else {
+        used_len = event_status_nea(buf, max_len);
+    }
+    ide_atapi_cmd_reply(s, used_len, max_len);
 }
 
 static void ide_atapi_cmd(IDEState *s)
@@ -1623,6 +1738,7 @@ static void cdrom_change_cb(void *opaque, int reason)
     s->sense_key = SENSE_UNIT_ATTENTION;
     s->asc = ASC_MEDIUM_MAY_HAVE_CHANGED;
     s->cdrom_changed = 1;
+    s->events.new_media = true;
     ide_set_irq(s->bus);
 }
 
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index d533fb6..ba7e9a8 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -373,6 +373,11 @@ typedef int DMAFunc(IDEDMA *);
 typedef int DMAIntFunc(IDEDMA *, int);
 typedef void DMARestartFunc(void *, int, int);
 
+struct unreported_events {
+    bool eject_request;
+    bool new_media;
+};
+
 /* NOTE: IDEState represents in fact one drive */
 struct IDEState {
     IDEBus *bus;
@@ -408,6 +413,7 @@ struct IDEState {
     BlockDriverState *bs;
     char version[9];
     /* ATAPI specific */
+    struct unreported_events events;
     uint8_t sense_key;
     uint8_t asc;
     uint8_t cdrom_changed;
-- 
1.7.4

[Qemu-devel] [RFC PATCH 3/3] atapi: Allow GET_EVENT_STATUS_NOTIFICATION and TEST_UNIT_READY after media change

[Amit Shah]

After a media change, the only commands allowed from the guest were
REQUEST_SENSE and INQUIRY.  The guest may also issue
GET_EVENT_STATUS_NOTIFICATION and TEST_UNIT_READY commands to get media
changed notification.  Enable those commands.

After this, the HSM violation messages from Linux guests aren't seen.

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

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 89e212b..362a258 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -625,6 +625,14 @@ void ide_atapi_cmd_ok(IDEState *s)
     ide_set_irq(s->bus);
 }
 
+static void ide_atapi_cmd_ua(IDEState *s)
+{
+    s->sense_key = SENSE_UNIT_ATTENTION;
+    s->asc = ASC_MEDIUM_MAY_HAVE_CHANGED;
+    ide_atapi_cmd_ok(s);
+    s->sense_key = SENSE_NONE;
+}
+
 void ide_atapi_cmd_error(IDEState *s, int sense_key, int asc)
 {
 #ifdef DEBUG_IDE_ATAPI
@@ -1243,20 +1251,26 @@ static void ide_atapi_cmd(IDEState *s)
     /* If there's a UNIT_ATTENTION condition pending, only
        REQUEST_SENSE and INQUIRY commands are allowed to complete. */
     if (s->sense_key == SENSE_UNIT_ATTENTION &&
-	s->io_buffer[0] != GPCMD_REQUEST_SENSE &&
-	s->io_buffer[0] != GPCMD_INQUIRY) {
-	ide_atapi_cmd_check_status(s);
-	return;
+        s->io_buffer[0] != GPCMD_REQUEST_SENSE &&
+        s->io_buffer[0] != GPCMD_INQUIRY &&
+        s->io_buffer[0] != GPCMD_GET_EVENT_STATUS_NOTIFICATION &&
+        s->io_buffer[0] != GPCMD_TEST_UNIT_READY) {
+        ide_atapi_cmd_check_status(s);
+        return;
     }
     switch(s->io_buffer[0]) {
     case GPCMD_TEST_UNIT_READY:
-        if (bdrv_is_inserted(s->bs) && !s->cdrom_changed) {
-            ide_atapi_cmd_ok(s);
+        if (bdrv_is_inserted(s->bs)) {
+            if (!s->cdrom_changed) {
+                ide_atapi_cmd_ok(s);
+            } else {
+                ide_atapi_cmd_ua(s);
+            }
         } else {
-            s->cdrom_changed = 0;
             ide_atapi_cmd_error(s, SENSE_NOT_READY,
                                 ASC_MEDIUM_NOT_PRESENT);
         }
+        s->cdrom_changed = 0;
         break;
     case GPCMD_MODE_SENSE_6:
     case GPCMD_MODE_SENSE_10:
-- 
1.7.4