From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:41198)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1Qnrh1-0001bX-Bv
	for qemu-devel@nongnu.org; Mon, 01 Aug 2011 08:34:00 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1Qnrh0-00070k-Iv
	for qemu-devel@nongnu.org; Mon, 01 Aug 2011 08:33:59 -0400
Received: from mx1.redhat.com ([209.132.183.28]:43143)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1Qnrh0-00070H-2U
	for qemu-devel@nongnu.org; Mon, 01 Aug 2011 08:33:58 -0400
From: Markus Armbruster <armbru@redhat.com>
References: <1311179069-27882-1-git-send-email-armbru@redhat.com>
	<1311179069-27882-45-git-send-email-armbru@redhat.com>
	<CAOq732+ry+aHqcP0GgyWkwN_w6c5=49Cp75Y5r37ndKwfWaz_w@mail.gmail.com>
Date: Mon, 01 Aug 2011 14:33:50 +0200
In-Reply-To: <CAOq732+ry+aHqcP0GgyWkwN_w6c5=49Cp75Y5r37ndKwfWaz_w@mail.gmail.com>
	(andrzej zaborowski's message of "Sat, 30 Jul 2011 04:24:44 +0200")
Message-ID: <m339hlb9ap.fsf@blackfin.pond.sub.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [Qemu-devel] [PATCH 44/55] spitz tosa: Simplify "drive is
	suitable for microdrive" test
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: andrzej zaborowski <balrogg@gmail.com>
Cc: kwolf@redhat.com, stefano.stabellini@eu.citrix.com, dbaryshkov@gmail.com, quintela@redhat.com, qemu-devel@nongnu.org, lcapitulino@redhat.com, amit.shah@redhat.com

andrzej zaborowski <balrogg@gmail.com> writes:

> On 20 July 2011 18:24, Markus Armbruster <armbru@redhat.com> wrote:
>> We try the drive defined with -drive if=3Dide,index=3D0 (or equivalent
>> sugar). =C2=A0We use it only if (dinfo && bdrv_is_inserted(dinfo->bdrv) =
&&
>> !bdrv_is_removable(dinfo->bdrv)). =C2=A0This is a convoluted way to test
>> for "drive media can't be removed".
>>
>> The only way to create such a drive with -drive if=3Dide is media=3Dcdro=
m.
>> And that sets dinfo->media_cd, so just test that.
>
> This is a less generic test and more prone to be broken inadvertently,
> so it seems like a step back.  What's the argument against the
> convoluted and explicit test?

My motivation for changing it was to reduce the uses of BlockDriverState
member removable prior to nuking it from orbit [PATCH 45/55].

I consider my change an improvement, because I find "dinfo->media_cd"
clearer than
"bdrv_is_inserted(dinfo->bdrv) && !bdrv_is_removable(dinfo->bdrv)".

Moreover, it's the only place that uses bdrv_is_removable() to test
whether the block driver supports media eject.  All the other places use
dinfo->media_cd: hw/scsi-disk.c hw/xen_devconfig.c hw/ide/core.c
hw/ide/qdev.c.  Can't see why spitz & tosa need to check the same thing
differently, and why it's worth keeping bdrv_is_removable() around just
for that.