From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49913) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ay7E6-0003Hl-5N for qemu-devel@nongnu.org; Wed, 04 May 2016 20:33:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ay7Du-0006ZO-Hq for qemu-devel@nongnu.org; Wed, 04 May 2016 20:33:04 -0400 Date: Thu, 5 May 2016 08:32:24 +0800 From: Fam Zheng Message-ID: <20160505003224.GA13202@ad.usersys.redhat.com> References: <1461030177-29144-1-git-send-email-famz@redhat.com> <1461030177-29144-3-git-send-email-famz@redhat.com> <20160504101242.GC14972@noname.str.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160504101242.GC14972@noname.str.redhat.com> Subject: Re: [Qemu-devel] [PATCH 2/2] block: Inactivate all children List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-devel@nongnu.org, Max Reitz , qemu-block@nongnu.org On Wed, 05/04 12:12, Kevin Wolf wrote: > Am 19.04.2016 um 03:42 hat Fam Zheng geschrieben: > > Currently we only inactivate the top BDS. Actually bdrv_inactivate > > should be the opposite of bdrv_invalidate_cache. > > > > Recurse into the whole subtree instead. > > > > Signed-off-by: Fam Zheng > > Did you actually test this? > > I would expect that bs->drv->bdrv_inactivate() fails now (as in > assertion failure) if it has anything to flush to the image because > bs->file has already be inactivated before. I think children need to be > inactived after their parents. OK, my test apparently failed to trigger that bdrv_pwritv() path. Good catch! > > Nodes with multiple parents could actually become even more > interesting... I'll make it two passes recursion: one for calling drv->bdrv_inactivate and the other for setting BDRV_O_INACTIVATE. Fam