Skip to contentSkip to conversation

Correct transitions for cd change state

PATCH Superseded Amit Shah 1 participant 3 messages Apr 6, 2011

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Amit Shah

author
#0 lore raw

cover Correct transitions for cd change state

Hello,

These two patches fix the cd media size change bugs.

The test scenario is:

1. create an iso image from a file
2. create a second iso image from a bigger file
3. mount 1st cd in guest
4. unmount it
5. change cd via qemu monitor
6. mount 2nd cd
7. copy file on cd to local disk -- shows errors.

I've not handled save/load state in these patches. There's no harm in always letting one extra 'cd not present' event being sent to the guest, so things should continue working fine across migrations.

Please apply.

Amit Shah (2):
cdrom: Allow the TEST_UNIT_READY command after a cdrom change
cdrom: Make disc change event visible to guests

 hw/ide/core.c     |   49 ++++++++++++++++++++++++++++++++++++++++++-------
 hw/ide/internal.h |    1 +
 2 files changed, 43 insertions(+), 7 deletions(-)
Signature · 1 line
1.7.4
author
#1 lore raw

PATCH 1/2 cdrom: Allow the TEST_UNIT_READY command after a cdrom change

We restrict the commands that a guest can send us after a cdrom change event. The current list includes REQUEST_SENSE and INQUIRY commands. Guests can also issue TEST_UNIT_READY to inquire for the status, so allow this command as well.

Signed-off-by Amit Shah <[email protected]>
---
 hw/ide/core.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)
hw/ide/core.c +5 -4
index 007a4ee..d55d804 100644
@@ -1105,10 +1105,11 @@ 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_TEST_UNIT_READY) {
+        ide_atapi_cmd_check_status(s);
+        return;
     }
     switch(s->io_buffer[0]) {
     case GPCMD_TEST_UNIT_READY:
Signature · 1 line
1.7.4
author
#2 lore raw

PATCH 2/2 cdrom: Make disc change event visible to guests

In reply to Amit Shah

Commit 93c8cfd9e67a62711b86f4c93747566885eb7928 tried to send a 'no disc' event after a cdrom change so that guests notice a cd change event between two 'cd present' states. However, we don't go from

'cd present' -> 'no cd' -> 'cd present'

as the SENSE_UNIT_ATTENTION sense_key is written over by the ide_atapi_cmd_error() function.

So for the disc change event, let us ensure the error() function doesn't trample over that value so we do get to report it the next time around. Also, ensure we go from 'no cd' to 'cd present' state.

With this, older Linux guests (< 2.6.38) notice cd changes just fine. For newer Linux guests (2.6.38+) cd change events break again and that will be fixed by implementing the GET_EVENT_STATUS_NOTIFICATION command.

Signed-off-by Amit Shah <[email protected]>
---
 hw/ide/core.c     |   40 +++++++++++++++++++++++++++++++++++++---
 hw/ide/internal.h |    1 +
 2 files changed, 38 insertions(+), 3 deletions(-)
hw/ide/core.c +37 -3
index d55d804..fd82439 100644
@@ -1113,10 +1113,42 @@ static void ide_atapi_cmd(IDEState *s)
     }
     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)) {
+            int sense, asc;
+
+            sense = s->sense_key;
+            asc = s->asc;
+
+	    /*
+	     * First, check if there's any pending media change
+	     * notification to be given.
+	     *
+	     * We want the guest to notice an empty tray between a cd
+	     * change, so send one MEDIUM_NOT_PRESENT message after a
+	     * cd change.
+	     *
+	     * After we've sent that message, the guest will poke at
+	     * us again and send the UNIT_ATTENTION message then.
+	     * Once this is done, reset the UNIT_ATTENTION message to
+	     * ensure we don't keep repeating it.
+	     */
+            if (!s->media_change_notified) {
+                ide_atapi_cmd_error(s, SENSE_NOT_READY,
+                                    ASC_MEDIUM_NOT_PRESENT);
+                s->media_change_notified = 1;
+            } else if (s->cdrom_changed) {
+                s->sense_key = SENSE_UNIT_ATTENTION;
+                s->asc = ASC_MEDIUM_MAY_HAVE_CHANGED;
+                ide_atapi_cmd_ok(s);
+
+                s->cdrom_changed = 0;
+		sense = SENSE_NONE;
+            } else {
+                ide_atapi_cmd_ok(s);
+            }
+            s->sense_key = sense;
+            s->asc = asc;
         } else {
-            s->cdrom_changed = 0;
             ide_atapi_cmd_error(s, SENSE_NOT_READY,
                                 ASC_MEDIUM_NOT_PRESENT);
         }

@@ -1613,6 +1645,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->media_change_notified = 0;
     ide_set_irq(s->bus);
 }
 

@@ -2474,6 +2507,7 @@ static void ide_reset(IDEState *s)
     s->sense_key = 0;
     s->asc = 0;
     s->cdrom_changed = 0;
+    s->media_change_notified = 0;
     s->packet_transfer_size = 0;
     s->elementary_transfer_size = 0;
     s->io_buffer_index = 0;
hw/ide/internal.h +1 -0
index d533fb6..5d58be4 100644
@@ -411,6 +411,7 @@ struct IDEState {
     uint8_t sense_key;
     uint8_t asc;
     uint8_t cdrom_changed;
+    uint8_t media_change_notified;
     int packet_transfer_size;
     int elementary_transfer_size;
     int io_buffer_index;
Signature · 1 line
1.7.4