From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MCMed-0005K0-Gp for qemu-devel@nongnu.org; Thu, 04 Jun 2009 19:47:27 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MCMeY-0005Fs-5V for qemu-devel@nongnu.org; Thu, 04 Jun 2009 19:47:26 -0400 Received: from [199.232.76.173] (port=48141 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MCMTS-0008Tu-Ts for qemu-devel@nongnu.org; Thu, 04 Jun 2009 19:35:55 -0400 Received: from mx2.redhat.com ([66.187.237.31]:39447) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MCClO-0000fL-F9 for qemu-devel@nongnu.org; Thu, 04 Jun 2009 09:13:46 -0400 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n54DDjc5027941 for ; Thu, 4 Jun 2009 09:13:45 -0400 Message-ID: <4A27C848.1050405@redhat.com> Date: Thu, 04 Jun 2009 15:12:40 +0200 From: Kevin Wolf MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] Prevent CD-ROM media eject while device is locked References: <1243415171.12756.4.camel@blaa> In-Reply-To: <1243415171.12756.4.camel@blaa> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Mark McLoughlin Cc: "qemu-devel@nongnu.org" Mark McLoughlin schrieb: > Section 10.8.25 ("START/STOP UNIT Command") of SFF-8020i states that > if the device is locked we should refuse to eject if the device is > locked. > > ASC_MEDIA_REMOVAL_PREVENTED is the appropriate return in this case. > > In order to stop itself from ejecting the media it is running from, > Fedora's installer (anaconda) requires the CDROMEJECT ioctl() to fail > if the drive has been previously locked. > > See also https://bugzilla.redhat.com/501412 > > Signed-off-by: Mark McLoughlin > --- > block.c | 9 ++++++++- > block.h | 2 +- > hw/ide.c | 26 ++++++++++++++++++-------- > 3 files changed, 27 insertions(+), 10 deletions(-) > > diff --git a/block.c b/block.c > index 9a2873f..863897a 100644 > --- a/block.c > +++ b/block.c > @@ -1673,11 +1673,15 @@ int bdrv_media_changed(BlockDriverState *bs) > /** > * If eject_flag is TRUE, eject the media. Otherwise, close the tray > */ > -void bdrv_eject(BlockDriverState *bs, int eject_flag) > +int bdrv_eject(BlockDriverState *bs, int eject_flag) > { > BlockDriver *drv = bs->drv; > int ret; > > + if (bs->locked) { > + return -EBUSY; > + } > + > if (!drv || !drv->bdrv_eject) { > ret = -ENOTSUP; > } else { > @@ -1686,7 +1690,10 @@ void bdrv_eject(BlockDriverState *bs, int eject_flag) > if (ret == -ENOTSUP) { > if (eject_flag) > bdrv_close(bs); > + ret = 0; > } > + > + return ret; > } > > int bdrv_is_locked(BlockDriverState *bs) > diff --git a/block.h b/block.h > index 979781a..e1070e9 100644 > --- a/block.h > +++ b/block.h > @@ -132,7 +132,7 @@ int bdrv_is_inserted(BlockDriverState *bs); > int bdrv_media_changed(BlockDriverState *bs); > int bdrv_is_locked(BlockDriverState *bs); > void bdrv_set_locked(BlockDriverState *bs, int locked); > -void bdrv_eject(BlockDriverState *bs, int eject_flag); > +int bdrv_eject(BlockDriverState *bs, int eject_flag); > void bdrv_set_change_cb(BlockDriverState *bs, > void (*change_cb)(void *opaque), void *opaque); > void bdrv_get_format(BlockDriverState *bs, char *buf, int buf_size); > diff --git a/hw/ide.c b/hw/ide.c > index 6ad1d08..9b93e7f 100644 > --- a/hw/ide.c > +++ b/hw/ide.c > @@ -359,6 +359,7 @@ > #define ASC_INCOMPATIBLE_FORMAT 0x30 > #define ASC_MEDIUM_NOT_PRESENT 0x3a > #define ASC_SAVING_PARAMETERS_NOT_SUPPORTED 0x39 > +#define ASC_MEDIA_REMOVAL_PREVENTED 0x53 > > #define CFA_NO_ERROR 0x00 > #define CFA_MISC_ERROR 0x09 > @@ -1818,18 +1819,27 @@ static void ide_atapi_cmd(IDEState *s) > break; > case GPCMD_START_STOP_UNIT: > { > - int start, eject; > + int start, eject, err = 0; > start = packet[4] & 1; > eject = (packet[4] >> 1) & 1; > > - if (eject && !start) { > - /* eject the disk */ > - bdrv_eject(s->bs, 1); > - } else if (eject && start) { > - /* close the tray */ > - bdrv_eject(s->bs, 0); > + if (eject) { > + err = bdrv_eject(s->bs, !start); > + } I would actually prefer to retain the comments on what start means in the eject case. Otherwise looks good to me. hw/scsi-disk.c contains another reference to bdrv_eject. Wouldn't it make sense to check the return value there, too? Kevin