qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] BdrvDirtyBitmap and bdrv_snapshot_goto


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [Qemu-devel] BdrvDirtyBitmap and bdrv_snapshot_goto
Date: Tue, 8 Nov 2016 19:48:27 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0

08.11.2016 15:18, Kevin Wolf wrote:
Am 08.11.2016 um 12:08 hat Vladimir Sementsov-Ogievskiy geschrieben:
08.11.2016 14:05, Kevin Wolf wrote:
Am 07.11.2016 um 17:10 hat Max Reitz geschrieben:
On 07.11.2016 16:24, Vladimir Sementsov-Ogievskiy wrote:
Hi all!

As I can see, in bdrv_snapshot_goto, existing dirty bitmaps are not
handled.. Is it ok? Should not they be filled with ones or something
like this?
Filling them with ones makes sense to me. I guess nobody noticed because
nobody was crazy enough to use block jobs alongside loadvm...
What's the use case in which ones make sense?

It rather seems to me that an active dirty bitmap and snapshot switching
should exclude each other because the bitmap becomes meaningless by the
switch. And chances are that after switching a snapshot you don't want
to "incrementally" backup everything, but that you should access a
different backup.
In other words, dirty bitmaps should be deleted on snapshot switch?
All? Or only named?
As Max said, we should probably integrate bitmaps with snapshots. After
reloading the old state, the bitmap becomes valid again, so throwing it
away in the active state seems only right if we included it in the
snapshot and can bring it back.

If we choose this way, it should be firstly done for BdrvDirtyBitmap's without any persistance. And it is not as simple as just drop dirty bitmaps or fill them with ones. Current behavior is definitely wrong: if user create incremental backup after snapshot switch this incremental backup will be incorrect. I think it should be fixed now simpler way (actually this fix means "for now incremental backup is incompatible with snapshot switch"), and in future, if we really need this, make them work together.

Also, I think that filling with ones is safer and more native. It really describes, what happens (with some overhead of dirty bits). Simple improvement: instead of filling with ones, new_dirty_bitmap_state = old_dirty_bitmap_state | old_allocated_mask | new_allocated_mask, where allocated mask is bitmap with same granularity, showing which ranges are allocated in the image.


Kevin


--
Best regards,
Vladimir




reply via email to

[Prev in Thread] Current Thread [Next in Thread]