[Qemu-devel] Re: [PATCH v2 2/2] cdrom: Make disc change event visible to guests

[Paolo Bonzini <pbonzini@redhat.com>]

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.shah@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.

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.

Paolo