01.08.2019 18:12, Max Reitz wrote:
> The backup job must only copy areas that the copy_bitmap reports as
> dirty. This is always the case when using traditional non-offloading
> backup, because it copies each cluster separately. When offloading the
> copy operation, we sometimes copy more than one cluster at a time, but
> we only check whether the first one is dirty.
>
> Therefore, whenever copy offloading is possible, the backup job
> currently produces wrong output when the guest writes to an area of
> which an inner part has already been backed up, because that inner part
> will be re-copied.
>
> Fixes: 9ded4a0114968e98b41494fc035ba14f84cdf700
> Signed-off-by: Max Reitz <mre...@redhat.com>
> ---
> block/backup.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/block/backup.c b/block/backup.c
> index 715e1d3be8..1ee271f9f1 100644
> --- a/block/backup.c
> +++ b/block/backup.c
> @@ -202,22 +202,31 @@ static int coroutine_fn backup_do_cow(BackupBlockJob
> *job,
> cow_request_begin(&cow_request, job, start, end);
>
> while (start < end) {
> + int64_t dirty_end;
> +
> if (!hbitmap_get(job->copy_bitmap, start)) {
> trace_backup_do_cow_skip(job, start);
> start += job->cluster_size;
> continue; /* already copied */
> }
>
> + dirty_end = hbitmap_next_zero(job->copy_bitmap, start, (end -
> start));
> + if (dirty_end < 0) {
> + dirty_end = end;
> + }
1. hbitmap_next_dirty_area is better I think
2. we can do it only in use_copy_range case, as backup_cow_with_bounce_buffer
copies
only one cluster anyway
But this all should be refactored anyway, so:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
> +
> trace_backup_do_cow_process(job, start);
>
> if (job->use_copy_range) {
> - ret = backup_cow_with_offload(job, start, end,
> is_write_notifier);
> + ret = backup_cow_with_offload(job, start, dirty_end,
> + is_write_notifier);
> if (ret < 0) {
> job->use_copy_range = false;
> }
> }
> if (!job->use_copy_range) {
> - ret = backup_cow_with_bounce_buffer(job, start, end,
> is_write_notifier,
> + ret = backup_cow_with_bounce_buffer(job, start, dirty_end,
> + is_write_notifier,
> error_is_read,
> &bounce_buffer);
> }
> if (ret < 0) {
>
--
Best regards,
Vladimir
