From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55110) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aSw5l-0007Fo-Ju for qemu-devel@nongnu.org; Mon, 08 Feb 2016 19:23:42 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aSw5k-0002lp-NU for qemu-devel@nongnu.org; Mon, 08 Feb 2016 19:23:41 -0500 References: <1453804705-7205-1-git-send-email-famz@redhat.com> <1453804705-7205-4-git-send-email-famz@redhat.com> From: John Snow Message-ID: <56B93185.2040509@redhat.com> Date: Mon, 8 Feb 2016 19:23:33 -0500 MIME-Version: 1.0 In-Reply-To: <1453804705-7205-4-git-send-email-famz@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH 03/16] block: Allow .bdrv_close callback to release dirty bitmaps List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng , qemu-devel@nongnu.org Cc: Kevin Wolf , qemu-block@nongnu.org, Markus Armbruster , mreitz@redhat.com, vsementsov@parallels.com, Stefan Hajnoczi On 01/26/2016 05:38 AM, Fam Zheng wrote: > If the driver owns some dirty bitmaps, this assertion will fail. > > The correct place to release them is in bdrv_close, so move the > assertion one line down. > > Signed-off-by: Fam Zheng > --- > block.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/block.c b/block.c > index afb71c0..fa6ad1d 100644 > --- a/block.c > +++ b/block.c > @@ -2348,10 +2348,11 @@ static void bdrv_delete(BlockDriverState *bs) > assert(!bs->job); > assert(bdrv_op_blocker_is_empty(bs)); > assert(!bs->refcnt); > - assert(QLIST_EMPTY(&bs->dirty_bitmaps)); > > bdrv_close(bs); > > + assert(QLIST_EMPTY(&bs->dirty_bitmaps)); > + > /* remove from list, if necessary */ > bdrv_make_anon(bs); > > I think now is where we need to begin distinguishing internally owned bitmaps from qmp/monitor created ones. I suppose this is just an assert so it isn't changing much, but there are different ideas at play here... - Bitmaps created internally for various reasons (backups, migration, etc) - Bitmaps created explicitly by the user (transient bitmaps) - Bitmaps autoloaded from qcow2, qbm, etc (persistent bitmaps) We should still make sure we don't have any of the first two types when we go to close the bitmap, and making sure we have none of the third is reasonable after the close.