[Qemu-devel][PATCH] Send unit_attention on cd-rom not_ready to ready transition

[Qemu-devel][PATCH] Send unit_attention on cd-rom not_ready to ready transition

[Pat Campbell]

[-- Attachment #1: Type: text/plain, Size: 433 bytes --]

Patch sends a UNIT_ATTENTION(6), MEDIUM_MAY_HAVE_CHANGED(0x28) sense
when cdrom transitions from not ready to ready. 

I used the following documentation:
 ATA Packet interface for CD-ROMS, SFF8020i.pdf. See state diagram Figure
 12, page 82 and Table 44 -recommended Sense Key, ASC

With patch in place HVM win2008 server guest sees the CD/DVD contents
have changed when the media is switched.

Patch applies to SVN revision 4456.


[-- Attachment #2: qemu-unit-attention.patch --]
[-- Type: text/plain, Size: 936 bytes --]

Index: hw/ide.c
===================================================================
--- hw/ide.c	(revision 4456)
+++ hw/ide.c	(working copy)
@@ -351,6 +351,7 @@
 #define ASC_ILLEGAL_OPCODE                   0x20
 #define ASC_LOGICAL_BLOCK_OOR                0x21
 #define ASC_INV_FIELD_IN_CMD_PACKET          0x24
+#define ASC_MEDIUM_MAY_HAVE_CHANGED          0x28
 #define ASC_MEDIUM_NOT_PRESENT               0x3a
 #define ASC_SAVING_PARAMETERS_NOT_SUPPORTED  0x39
 
@@ -1385,6 +1386,11 @@
     switch(s->io_buffer[0]) {
     case GPCMD_TEST_UNIT_READY:
         if (bdrv_is_inserted(s->bs)) {
+            if (s->is_cdrom && s->sense_key == SENSE_NOT_READY) {
+                ide_atapi_cmd_error(s, SENSE_UNIT_ATTENTION,
+                                    ASC_MEDIUM_MAY_HAVE_CHANGED);
+                break;
+            }
             ide_atapi_cmd_ok(s);
         } else {
             ide_atapi_cmd_error(s, SENSE_NOT_READY,

Re: [Qemu-devel][PATCH] Send unit_attention on cd-rom not_ready to ready transition

[Ian Jackson]

Pat Campbell writes ("[Qemu-devel][PATCH] Send unit_attention on cd-rom not_ready to ready transition"):
> Patch sends a UNIT_ATTENTION(6), MEDIUM_MAY_HAVE_CHANGED(0x28) sense
> when cdrom transitions from not ready to ready. 

Was there something wrong with Pat's patch, or has it just been
overlooked ?

Ian.

Re: [Qemu-devel][PATCH] Send unit_attention on cd-rom not_ready to ready transition

[Ian Jackson]

Pat Campbell writes ("[Qemu-devel][PATCH] Send unit_attention on cd-rom not_ready to ready transition"):
> Patch sends a UNIT_ATTENTION(6), MEDIUM_MAY_HAVE_CHANGED(0x28) sense
> when cdrom transitions from not ready to ready. 
...
> Patch applies to SVN revision 4456.

I have checked that this code compiles, and it seems to make sense
(although I haven't double-checked the SCSI specification to check
whether this is the right place to send that UNIT ATTENTION message).

I've also confirmed with Pat by email that it should be considered
signed off, so:

Acked-By: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Pat Campbell <plc@novell.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

Full disclosure: I have an interest in this in as much as I encouraged
Pat Campbell to submit this patch here as well as to xen-unstable.

> Index: hw/ide.c
> ===================================================================
> --- hw/ide.c	(revision 4456)
> +++ hw/ide.c	(working copy)
> @@ -351,6 +351,7 @@
>  #define ASC_ILLEGAL_OPCODE                   0x20
>  #define ASC_LOGICAL_BLOCK_OOR                0x21
>  #define ASC_INV_FIELD_IN_CMD_PACKET          0x24
> +#define ASC_MEDIUM_MAY_HAVE_CHANGED          0x28
>  #define ASC_MEDIUM_NOT_PRESENT               0x3a
>  #define ASC_SAVING_PARAMETERS_NOT_SUPPORTED  0x39
>  
> @@ -1385,6 +1386,11 @@
>      switch(s->io_buffer[0]) {
>      case GPCMD_TEST_UNIT_READY:
>          if (bdrv_is_inserted(s->bs)) {
> +            if (s->is_cdrom && s->sense_key == SENSE_NOT_READY) {
> +                ide_atapi_cmd_error(s, SENSE_UNIT_ATTENTION,
> +                                    ASC_MEDIUM_MAY_HAVE_CHANGED);
> +                break;
> +            }
>              ide_atapi_cmd_ok(s);
>          } else {
>              ide_atapi_cmd_error(s, SENSE_NOT_READY,

Re: [Qemu-devel][PATCH] Send unit_attention on cd-rom not_ready to ready transition

[Jamie Lokier]

> > Patch sends a UNIT_ATTENTION(6), MEDIUM_MAY_HAVE_CHANGED(0x28) sense
> > when cdrom transitions from not ready to ready. 
> ...
> > Patch applies to SVN revision 4456.
> 
> I have checked that this code compiles, and it seems to make sense
> (although I haven't double-checked the SCSI specification to check
> whether this is the right place to send that UNIT ATTENTION message).

Is this expected to fix the following?

Running Windows XP guest, in the QEMU monitor I do "change ide1-cd0
none", then a little while later, "change ide1-cd0 newfile.iso".

Interestingly, Windows XP _does_ show the CD (drive D:) change from
having contents to being not present (after the first change command).

But after the second change command, Windows XP shows it changes to
having contents again - but the _old_ contents of the drive!

Using the "eject" QEMU monitor command doesn't help.

However, using the eject menu option from within Windows XP itself,
then changing the CD image QEMU's monitor - that does work and it
shows the new contents.

I'm thinking perhaps XP caches the disk contents, and even though it
sees the CD transition from present to not-present and back, it does
not flush it's cache for some (bizarre) reason.

Is this patch expected to fix this, so issuing the "change" command in
QEMU's monitor will cause XP to show the new contents?  Does that
imply _all_ CD drives issue the error shown in this patch?

Thanks,
-- Jamie