On 7/4/19 2:05 PM, Max Reitz wrote:
> On 04.07.19 19:43, Max Reitz wrote:
>> On 03.07.19 23:55, John Snow wrote:
>>> This adds an "always" policy for bitmap synchronization. Regardless of if
>>> the job succeeds or fails, the bitmap is *always* synchronized. This means
>>> that for backups that fail part-way through, the bitmap retains a record of
>>> which sectors need to be copied out to accomplish a new backup using the
>>> old, partial result.
>>>
>>> In effect, this allows us to "resume" a failed backup; however the new
>>> backup
>>> will be from the new point in time, so it isn't a "resume" as much as it is
>>> an "incremental retry." This can be useful in the case of extremely large
>>> backups that fail considerably through the operation and we'd like to not
>>> waste
>>> the work that was already performed.
>>>
>>> Signed-off-by: John Snow <js...@redhat.com>
>>> ---
>>> block/backup.c | 25 +++++++++++++++++--------
>>> qapi/block-core.json | 5 ++++-
>>> 2 files changed, 21 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/block/backup.c b/block/backup.c
>>> index 9cc5a7f6ca..495d8f71aa 100644
>>> --- a/block/backup.c
>>> +++ b/block/backup.c
>>> @@ -266,16 +266,25 @@ static void backup_cleanup_sync_bitmap(BackupBlockJob
>>> *job, int ret)
>
> [...]
>
>>> + /* If we failed and synced, merge in the bits we didn't copy: */
>>> + bdrv_dirty_bitmap_merge_internal(bm, job->copy_bitmap,
>>> + NULL, true);
>>
>> I presume this is for sync=full?
>
Well, we don't allow bitmaps for sync=full at this point in the series.
> Ah, no. This is necessary because the original sync bitmap was
> discarded by bdrv_dirty_bitmap_abdicate(). So yep, these bits need to
> go back into the sync bitmap then.
>
> Max
>
Yuh -- we actually want to clear the original bitmap for the 'always'
case, which the "abdicate" handles for us. This ought to trigger only
for the always case, so I think the conditional here is correct and as
simple as it can be.
Does your R-B stand?
--js