[Qemu-devel] Re: [PATCH v2 1/2] cdrom: Allow the TEST_UNIT_READY command after a cdrom change

[Amit Shah]

On (Thu) 07 Apr 2011 [12:20:16], Kevin Wolf wrote:
> Am 07.04.2011 11:11, schrieb Amit Shah:
> > On (Thu) 07 Apr 2011 [10:59:20], Kevin Wolf wrote:
> >> Am 07.04.2011 07:05, schrieb Amit Shah:
> >>> 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 <address@hidden>
> >>
> >> Hm... MMC-5, section 4.1.6.1 seems to conflict with this:
> >>
> >> "If a Host issues a command other than GET CONFIGURATION, GET EVENT
> >> STATUS NOTIFICATION, INQUIRY or REQUEST SENSE while a unit attention
> >> condition exists for that Host, the Drive shall not perform the command
> >> and shall report CHECK CONDITION status unless a higher priority status
> >> as defined by the Drive is also pending."
> >>
> >> So while you're right that our list is incomplete, TEST UNIT READY
> >> doesn't seem to be among the missing commands.
> > 
> > Hm - older Linux guests (pre 2.6.38) and Windows guests, as Gleb's
> > commit mentioned, rely on this command to get CD change notifications:
> > 
> > /* identical to scsi_test_unit_ready except that it doesn't
> >  * eat the NOT_READY returns for removable media */
> > int sr_test_unit_ready(struct scsi_device *sdev, struct scsi_sense_hdr 
> > *sshdr)
> > {
> >         int retries = MAX_RETRIES;
> >         int the_result;
> >         u8 cmd[] = {TEST_UNIT_READY, 0, 0, 0, 0, 0 };
> > 
> >         /* issue TEST_UNIT_READY until the initial startup UNIT_ATTENTION
> >          * conditions are gone, or a timeout happens
> >          */
> >         do {
> >                 the_result = scsi_execute_req(sdev, cmd, DMA_NONE, NULL,
> >                                               0, sshdr, SR_TIMEOUT,
> >                                               retries--, NULL);
> >                 if (scsi_sense_valid(sshdr) &&
> >                     sshdr->sense_key == UNIT_ATTENTION)
> >                         sdev->changed = 1;
> > 
> >         } while (retries > 0 &&
> >                  (!scsi_status_is_good(the_result) ||
> >                   (scsi_sense_valid(sshdr) &&
> >                    sshdr->sense_key == UNIT_ATTENTION)));
> >         return the_result;
> > }
> 
> I think the scsi_execute_req() call might issue a REQUEST SENSE
> internally and therefore clear the unit attention condition. Tried to
> check that in the source, but I'm hopelessly lost in the kernel...

No, it doesn't.

It's clear the 2nd patch is needed; the 1st one must be papering over
something else.  Right now I'm inclined to think that it's because we
don't report tray open and no media as separate events to the guest,
as Markus has found out.

                Amit