On (Thu) 07 Apr 2011 [09:31:05], Paolo Bonzini wrote:
> On 04/07/2011 07:05 AM, Amit Shah wrote:
> >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<amit.s...@redhat.com>
> >---
> > hw/ide/core.c | 42 +++++++++++++++++++++++++++++++++++++++---
> > hw/ide/internal.h | 1 +
> > 2 files changed, 40 insertions(+), 3 deletions(-)
> >
> >diff --git a/hw/ide/core.c b/hw/ide/core.c
> >index d55d804..abb577c 100644
> >--- a/hw/ide/core.c
> >+++ b/hw/ide/core.c
> >@@ -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;
> >@@ -2713,6 +2747,8 @@ static int ide_drive_post_load(void *opaque, int
> >version_id)
> > if (s->sense_key == SENSE_UNIT_ATTENTION&&
> > s->asc == ASC_MEDIUM_MAY_HAVE_CHANGED) {
> > s->cdrom_changed = 1;
> >+ } else {
>
> This is within a version_id < 3 condition, so media_change_notified
> is never set to 1 when loading from a current QEMU.
Right.
> Integrating the new flag into cdrom_changed (0 = ok, 1 = cdrom
> changed and NOT_PRESENT sent, 2 = cdrom changed and NOT_PRESENT not
> sent) should also work for older guests. They just treat 2 the same
> as 1.
Yes, thanks -- I was thinking the same.
v3 coming up.
Amit
