From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:52607)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1XZemG-0004ly-0B
	for qemu-devel@nongnu.org; Thu, 02 Oct 2014 07:42:38 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1XZem9-0001ue-Qu
	for qemu-devel@nongnu.org; Thu, 02 Oct 2014 07:42:31 -0400
Received: from mx1.redhat.com ([209.132.183.28]:43026)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1XZem9-0001tV-I0
	for qemu-devel@nongnu.org; Thu, 02 Oct 2014 07:42:25 -0400
From: Markus Armbruster <armbru@redhat.com>
References: <1412240698-21695-1-git-send-email-armbru@redhat.com>
	<1412240698-21695-5-git-send-email-armbru@redhat.com>
	<20141002105534.GB4888@noname.redhat.com>
Date: Thu, 02 Oct 2014 13:42:20 +0200
In-Reply-To: <20141002105534.GB4888@noname.redhat.com> (Kevin Wolf's message
	of "Thu, 2 Oct 2014 12:55:34 +0200")
Message-ID: <87y4syem03.fsf@blackfin.pond.sub.org>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [Qemu-devel] [PATCH v5 04/23] block: Connect BlockBackend and
	DriveInfo
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: Kevin Wolf <kwolf@redhat.com>
Cc: benoit.canet@nodalink.com, qemu-devel@nongnu.org, stefanha@redhat.com, mreitz@redhat.com

Kevin Wolf <kwolf@redhat.com> writes:

> Am 02.10.2014 um 11:04 hat Markus Armbruster geschrieben:
>> Make the BlockBackend own the DriveInfo.  Change blockdev_init() to
>> return the BlockBackend instead of the DriveInfo.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> Reviewed-by: Max Reitz <mreitz@redhat.com>
>
>> @@ -200,11 +202,11 @@ DriveInfo *drive_get_next(BlockInterfaceType type)
>>  
>>  DriveInfo *drive_get_by_blockdev(BlockDriverState *bs)
>>  {
>> -    DriveInfo *dinfo;
>> +    BlockBackend *blk;
>>  
>> -    QTAILQ_FOREACH(dinfo, &drives, next) {
>> -        if (dinfo->bdrv == bs) {
>> -            return dinfo;
>> +    for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {
>> +        if (blk == bs->blk) {
>> +            return blk_legacy_dinfo(blk);
>>          }
>>      }
>>      return NULL;
>
> In v3, I asked why you don't use bs->blk here. Apparently you understood
> this as a suggestion to change the if condition from:
>
>     if (blk_bs(blk) == bs)
>
> to:
>
>     if (blk == bs->blk)
>
> Which isn't a wrong change, it just doesn't change a lot. What I really
> meant is something like this, removing the loop:
>
>     DriveInfo *drive_get_by_blockdev(BlockDriverState *bs)
>     {
>         return bs->blk ? blk_legacy_dinfo(bs->blk) : NULL;
>     }
>
> This would only behave differently if there were BlockBackends that can
> be assigned to bs->blk, but aren't iterated by blk_next(). But such
> BlockBackends don't exist, blk_next() includes all of them.

Actually, there are: blk_hide_on_behalf_of_do_drive_del() removes from
blk_backends without also zapping bs->blk.

However, your version is *less* of a change, because it also doesn't
remove from drives.

In short: sold, thanks!