From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44630) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1buHrx-0006nR-6C for qemu-devel@nongnu.org; Wed, 12 Oct 2016 07:38:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1buHrv-0003jG-74 for qemu-devel@nongnu.org; Wed, 12 Oct 2016 07:38:44 -0400 Received: from mailhub.sw.ru ([195.214.232.25]:9263 helo=relay.sw.ru) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1buHru-0003j0-Rm for qemu-devel@nongnu.org; Wed, 12 Oct 2016 07:38:43 -0400 References: <1475232808-4852-1-git-send-email-vsementsov@virtuozzo.com> <1475232808-4852-10-git-send-email-vsementsov@virtuozzo.com> <48e6c042-67ba-6c39-7887-64f94d262362@redhat.com> From: Vladimir Sementsov-Ogievskiy Message-ID: <57FE20B6.7000806@virtuozzo.com> Date: Wed, 12 Oct 2016 14:38:30 +0300 MIME-Version: 1.0 In-Reply-To: <48e6c042-67ba-6c39-7887-64f94d262362@redhat.com> Content-Type: text/plain; charset=iso-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 09/22] block: introduce persistent dirty bitmaps List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz , qemu-block@nongnu.org, qemu-devel@nongnu.org Cc: kwolf@redhat.com, armbru@redhat.com, eblake@redhat.com, jsnow@redhat.com, famz@redhat.com, den@openvz.org, stefanha@redhat.com, pbonzini@redhat.com On 07.10.2016 22:28, Max Reitz wrote: > On 30.09.2016 12:53, Vladimir Sementsov-Ogievskiy wrote: >> New field BdrvDirtyBitmap.persistent means, that bitmap should be saved >> on bdrv_close, using format driver. Format driver should maintain bitmap >> storing. >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy >> --- >> block.c | 30 ++++++++++++++++++++++++++++++ >> block/dirty-bitmap.c | 27 +++++++++++++++++++++++++++ >> block/qcow2-bitmap.c | 1 + >> include/block/block.h | 2 ++ >> include/block/block_int.h | 2 ++ >> include/block/dirty-bitmap.h | 6 ++++++ >> 6 files changed, 68 insertions(+) > [...] > >> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c >> index 623e1d1..0314581 100644 >> --- a/block/dirty-bitmap.c >> +++ b/block/dirty-bitmap.c > [...] > >> @@ -555,3 +559,26 @@ bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap) >> { >> return bitmap->autoload; >> } >> + >> +void bdrv_dirty_bitmap_set_persistance(BdrvDirtyBitmap *bitmap, >> + bool persistent) >> +{ >> + bitmap->persistent = persistent; > After some thinking, I think this function should be more complex: It > should check whether the node the bitmap is attached to actually can > handle persistent bitmaps and whether it would actually support storing > *this* bitmap. > > For instance, a qcow2 node would not support writing overly large > bitmaps (limited by BME_MAX_TABLE_SIZE and BME_MAX_PHYS_SIZE) or bitmaps > with overly large granularities (BME_MAX_GRANULARITY_BITS) or bitmaps > whose name is already occupied by some bitmap that is already stored in > the file but has not been loaded. > > Checking this here will trivially prevent users from creating such > bitmaps and will also preempt detection of such failures during > bdrv_close() when they cannot be handled gracefully. > > Max Good point, but I can't do it exactly as you say, because I call this function from qcow2_read_bitmaps, for just created bitmap and it should not be checked and of course it's name is occupied.. > >> +} -- Best regards, Vladimir