From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=56949 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q7k9T-0006e5-GE for qemu-devel@nongnu.org; Thu, 07 Apr 2011 04:01:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q7k9R-0005mK-S2 for qemu-devel@nongnu.org; Thu, 07 Apr 2011 04:01:14 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46925) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q7k9R-0005mA-JW for qemu-devel@nongnu.org; Thu, 07 Apr 2011 04:01:13 -0400 Date: Thu, 7 Apr 2011 13:31:05 +0530 From: Amit Shah Message-ID: <20110407080105.GB1419@amit-x200.redhat.com> References: <4D9D6839.8040809@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4D9D6839.8040809@redhat.com> Subject: [Qemu-devel] Re: [PATCH v2 2/2] cdrom: Make disc change event visible to guests List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Kevin Wolf , Gleb Natapov , Juan Quintela , Stefan Hajnoczi , Markus Armbruster , qemu list 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 > >--- > > 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