From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MWD6j-0005N0-Sx for qemu-devel@nongnu.org; Wed, 29 Jul 2009 13:38:29 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MWD6f-0005MF-0b for qemu-devel@nongnu.org; Wed, 29 Jul 2009 13:38:29 -0400 Received: from [199.232.76.173] (port=49592 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MWD6e-0005MC-Tn for qemu-devel@nongnu.org; Wed, 29 Jul 2009 13:38:24 -0400 Received: from mx2.redhat.com ([66.187.237.31]:52649) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MWD6e-0004gC-Cc for qemu-devel@nongnu.org; Wed, 29 Jul 2009 13:38:24 -0400 Date: Wed, 29 Jul 2009 20:38:20 +0300 From: Gleb Natapov Subject: Re: [Qemu-devel] [PATCH v3] make windows notice media change Message-ID: <20090729173820.GA7382@redhat.com> References: <20090729160902.GG30449@redhat.com> <5b31733c0907291010k6ce95d54g6cbaaa954f22409b@mail.gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=koi8-r Content-Disposition: inline Content-Transfer-Encoding: quoted-printable In-Reply-To: <5b31733c0907291010k6ce95d54g6cbaaa954f22409b@mail.gmail.com> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Filip Navara Cc: qemu-devel@nongnu.org On Wed, Jul 29, 2009 at 07:10:29PM +0200, Filip Navara wrote: > On Wed, Jul 29, 2009 at 6:09 PM, Gleb Natapov wrote: > > @@ -3250,6 +3253,8 @@ static int pci_ide_load(QEMUFile* f, void *opaque= , int version_id) > > =9A =9A /* per IDE drive data */ > > =9A =9A for(i =3D 0; i < 4; i++) { > > =9A =9A =9A =9A ide_load(f, &d->ide_if[i]); > > + =9A =9A =9A =9Aif (version_id =3D=3D 3) > > + =9A =9A =9A =9A =9A =9Aqemu_get_8s(f, &d->ide_if[i].cdrom_changed); > > =9A =9A } > > =9A =9A return 0; > > =9A} >=20 > I'd prefer passing the version to ide_load and doing the actual load ther= e... >=20 Then you'll break ide_load for md ad pmac. > ... but the patch is all wrong and based on wrong assumptions, which is > much more fundamental problem. Windows cdrom driver is not that stupid > about the change as you think. Have you seen the code? How do you know? >=20 > The cdrom driver really has a timer and polls the IDE controller, but it > doesn't require the intermediate ASC_MEDIUM_NOT_PRESENT state > you introduced. It's perfectly ok to return SENSE_UNIT_ATTENTION / I have=20 > ASC_MEDIUM_MAY_HAVE_CHANGED from GPCMD_TEST_UNIT_READY > and Windows will recognize it as medium change. >=20 > Something like this should work: > if (bdrv_is_inserted(s->bs)) { > if (s->cdrom_changed) { > ide_atapi_cmd_error(s, SENSE_UNIT_ATTENTION, > ASC_MEDIUM_MAY_HAVE_CHANGED); > s->cdrom_changed =3D 0; > } else { > ide_atapi_cmd_ok(s); > } > } else { > ide_atapi_cmd_error(s, SENSE_NOT_READY, > ASC_MEDIUM_NOT_PRESENT); > } >=20 > The benefit is that it will not break guests which issue the request only > once. >=20 10.8.26 TEST UNIT READY Command The TEST UNIT READY command provides a means to check if the Device is ready. This is not a request for a self-test. If the Device would accept an appropriate medium-access command without returning CHECK CONDITION status, this command shall return a GOOD status. If the Device cannot become operational or is in a state such that an Host Computer action (e.g. START/STOP UNIT command with LoEj =3D 0 & Start =3D 1) is required to make the unit ready, the ATAPI CD-ROM Drive shall return CHECK CONDITION status with a sense key of NOT READY. No mentioning of returning MEDIUM MAY HAVE CHANGED from UNIT READY command, so you code it already incorrect. So what should be done in this case. Here is what spec says and code actually implements: 10.6 Unit Attention Condition If an Host Computer issues a command other than INQUIRY or REQUEST SENSE while a unit attention condition exists for that Host, the ATAPI CD-ROM Drive shall not perform the command and shall report CHECK CONDITION status unless a higher priority status as defined by the ATAPI CD-ROM Drive is also pending (e.g. BUSY). Cool. So Windows calls REQUEST SENSE after seeing CHECK CONDITION. Gets MEDIUM MAY HAVE CHANGED calls TEST UNIT once again see that media is present and thinks that CDROM gone crazy. If you claim that my fix is incorrect (it may very well be) please provide working tested solution compliant to spec. -- Gleb.