From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35135) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1buIgS-000278-Gq for qemu-devel@nongnu.org; Wed, 12 Oct 2016 08:30:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1buIgO-0003R9-BH for qemu-devel@nongnu.org; Wed, 12 Oct 2016 08:30:55 -0400 Received: from mailhub.sw.ru ([195.214.232.25]:47376 helo=relay.sw.ru) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1buIgN-0003Qs-UR for qemu-devel@nongnu.org; Wed, 12 Oct 2016 08:30:52 -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> <57FE20B6.7000806@virtuozzo.com> From: Vladimir Sementsov-Ogievskiy Message-ID: <57FE2CDE.4080404@virtuozzo.com> Date: Wed, 12 Oct 2016 15:30:22 +0300 MIME-Version: 1.0 In-Reply-To: <57FE20B6.7000806@virtuozzo.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 12.10.2016 14:38, Vladimir Sementsov-Ogievskiy wrote: > 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.. So, I'll add an additional checking function, to call it from qmp_block_dirty_bitmap_add, if persistent parameter is set to true. > >> >>> +} > > -- Best regards, Vladimir