From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:40481)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1XW8lu-00057M-Dl
	for qemu-devel@nongnu.org; Mon, 22 Sep 2014 14:55:53 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1XW8lK-0007I3-IT
	for qemu-devel@nongnu.org; Mon, 22 Sep 2014 14:55:37 -0400
Received: from mx1.redhat.com ([209.132.183.28]:7801)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1XW8lJ-0007A9-ML
	for qemu-devel@nongnu.org; Mon, 22 Sep 2014 14:55:02 -0400
From: Markus Armbruster <armbru@redhat.com>
References: <1410891148-28849-1-git-send-email-armbru@redhat.com>
	<1410891148-28849-24-git-send-email-armbru@redhat.com>
	<54202CEB.9070606@redhat.com>
Date: Mon, 22 Sep 2014 17:08:49 +0200
In-Reply-To: <54202CEB.9070606@redhat.com> (Max Reitz's message of "Mon, 22
	Sep 2014 16:06:35 +0200")
Message-ID: <87d2an7kvy.fsf@blackfin.pond.sub.org>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [Qemu-devel] [PATCH v3 23/23] block: Make device model's
	references to BlockBackend strong
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: Max Reitz <mreitz@redhat.com>
Cc: kwolf@redhat.com, stefanha@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, benoit.canet@nodalink.com

Max Reitz <mreitz@redhat.com> writes:

> On 16.09.2014 20:12, Markus Armbruster wrote:
>> Doesn't make a difference just yet, but it's the right thing to do.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   block/block-backend.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/block/block-backend.c b/block/block-backend.c
>> index d49c988..5646628 100644
>> --- a/block/block-backend.c
>> +++ b/block/block-backend.c
>> @@ -253,6 +253,7 @@ int blk_attach_dev(BlockBackend *blk, void *dev)
>>       if (blk->dev) {
>>           return -EBUSY;
>>       }
>> +    blk_ref(blk);
>>       blk->dev = dev;
>>       bdrv_iostatus_reset(blk->bs);
>>   @@ -281,9 +282,10 @@ void blk_detach_dev(BlockBackend *blk, void
>> *dev)
>>   /* TODO change to DeviceState *dev when all users are qdevified */
>>   {
>>       assert(blk->dev == dev);
>> -    blk->dev = NULL;
>>       blk->dev_ops = NULL;
>>       blk->dev_opaque = NULL;
>> +    blk->dev = NULL;
>> +    blk_unref(blk);
>>       bdrv_set_guest_block_size(blk->bs, 512);
>>       qemu_coroutine_adjust_pool_size(-COROUTINE_POOL_RESERVATION);
>>   }
>
> I'd put the blk_unref() call at the very end of this function. It
> probably won't happen but theoretically blk_unref() can free the
> BlockBackend object and I don't like the risk of use-after-free in
> blk->bs.

Even if it can't happen, putting it at the end is more obviously
correct.  I'll do it.