From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41438) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XZdph-0005ll-JF for qemu-devel@nongnu.org; Thu, 02 Oct 2014 06:42:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XZdpb-0002JL-Ep for qemu-devel@nongnu.org; Thu, 02 Oct 2014 06:42:01 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59160) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XZdpb-0002Hr-7Z for qemu-devel@nongnu.org; Thu, 02 Oct 2014 06:41:55 -0400 Date: Thu, 2 Oct 2014 12:41:47 +0200 From: Kevin Wolf Message-ID: <20141002104147.GA4888@noname.redhat.com> References: <1412240698-21695-1-git-send-email-armbru@redhat.com> <1412240698-21695-4-git-send-email-armbru@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1412240698-21695-4-git-send-email-armbru@redhat.com> Subject: Re: [Qemu-devel] [PATCH v5 03/23] block: Connect BlockBackend to BlockDriverState List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: mreitz@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, benoit.canet@nodalink.com Am 02.10.2014 um 11:04 hat Markus Armbruster geschrieben: > Convenience function blk_new_with_bs() creates a BlockBackend with its > BlockDriverState. Callers have to unref both. The commit after next > will relieve them of the need to unref the BlockDriverState. > > Complication: due to the silly way drive_del works, we need a way to > hide a BlockBackend, just like bdrv_make_anon(). To emphasize its > "special" status, give the function a suitably off-putting name: > blk_hide_on_behalf_of_do_drive_del(). Unfortunately, hiding turns the > BlockBackend's name into the empty string. Can't avoid that without > breaking the blk->bs->device_name equals blk->name invariant. > > The patch adds a memory leak: drive_del while a device model is > connected leaks the BlockBackend. Avoiding the leak here is rather > hairy, but it'll become straightforward shortly, so I mark it FIXME in > the code now, and plug it when it's easy. Does this leak actually still exist now that you have a blk_unref() in drive_del() (which is called during autodel) rather than do_drive_del()? > Signed-off-by: Markus Armbruster > Reviewed-by: Max Reitz Kevin