Re: [Qemu-devel] [PATCH v12 07/17] qmp: Add support of "dirty-bitmap" sync mode for drive-backup

[Max Reitz <mreitz@redhat.com>]

On 2015-02-11 at 13:31, John Snow wrote:
>
>
> On 02/11/2015 01:18 PM, Max Reitz wrote:
>> On 2015-02-11 at 12:54, John Snow wrote:
>>>
>>> On 02/11/2015 12:47 PM, Max Reitz wrote:
>>>> Looks good to me in general, now I need to find out what the successor
>>>> bitmap is used for; but I guess I'll find that out by reviewing the 
>>>> rest
>>>> of this series.
>>>>
>>>> Max
>>>
>>> They don't really come up again, actually.
>>>
>>> The basic idea is this: While the backup is going on, reads and writes
>>> may occur (albeit delayed) and we want to track those writes in a
>>> separate bitmap for the duration of the backup operation.
>>
>> Yes, I thought as much; but where are writes to the named bitmap being
>> redirected to its successor? bdrv_set_dirty() doesn't do that, as far as
>> I can see.
>>
>
> bdrv_dirty_bitmap_create_successor calls bdrv_create_dirty_bitmap, 
> which installs it in the bitmap chain attached to a BDS:
>
>     QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
>
> which is read by bdrv_set_dirty.

Oooh, clever. Right.

>
> bdrv_set_dirty operates on all bitmaps attached to a BDS, while 
> bdrv_set_dirty_bitmap operates on a single specific instance.
>
>>> If the backup operation fails, we use the dirty sector tracking info
>>> in the successor to know what has changed since we started the backup,
>>> and we merge this bitmap back into the originating bitmap; then if an
>>> incremental backup is tried again, it includes all of the original
>>> data plus any data changed while we failed to do a backup.
>>>
>>> If the backup operation succeeds, the originating bitmap is deleted
>>> and the successor is installed in its place.
>>>
>>> It's a namespace trick: by having an anonymous bitmap as a child of
>>> the "real" bitmap, the real bitmap can be frozen and prohibited from
>>> being moved, renamed, deleted, etc. This prevents the user from adding
>>> a new bitmap with the same name or similar while the backup is in
>>> progress.
>>
>> Hm, if it's just for that, wouldn't disabling the bitmap suffice?
>>
>> Max
>>
>
> Kind of? We still want to track writes while it's disabled.

Right, I was assuming here that writes are not tracked in the successor.

Thanks for pointing out that they are!

Max

>
> If we try to use a single bitmap, we have no real way to know which 
> bits to clear after the operation succeeds. I think two bitmaps is a 
> requirement to accommodate both failure and success cases.
>
> A distinction is made between a disabled bitmap (which is just 
> read-only: it can be deleted) and a frozen bitmap (which is in-use by 
> an operation, implicitly disabled, and cannot be enabled, disabled, 
> deleted, cleared, set or reset.)
>
>>> A previous approach was to immediately take the bitmap off of the BDS,
>>> but in the error case here, the logic becomes more complicated when we
>>> need to re-install the bitmap but the user has already installed a new
>>> bitmap with the same name, etc.
>>>
>>> So the general lifetime is this:
>>>
>>> (1) A backup is started. the block/backup routine calls 
>>> create_successor.
>>> (2) If the backup fails to start, the block/backup routine will call
>>> the "reclaim" method, which will merge the (empty) successor back into
>>> the original bitmap, unfreezing it.
>>> (3) If the backup starts, and then fails, the bitmap is "reclaim"ed
>>> (merged back into one bitmap.)
>>> (4) If the backup succeeds, the bitmap "abdicates" to the successor.
>>> (The parent bitmap is erased and the successor is installed in its
>>> place.)
>>
>> Yes, see the graph at the whiteboard behind me. :-)
>>
>> Max