Am 15.10.2018 um 18:06 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Drop write notifiers and use filter node instead. Changes:
>
> 1. copy-before-writes now handled by filter node, so, drop all
> is_write_notifier arguments.
>
> 2. we don't have intersecting requests, so their handling is dropped.
> Instead, synchronization works as follows:
> when backup or backup-top starts copying of some area it firstly
> clears copy-bitmap bits, and nobody touches areas, not marked with
> dirty bits in copy-bitmap, so there is no intersection. Also, backup
> job copy operations are surrounded by bdrv region lock, which is
> actually serializing request, to not interfer with guest writes and
> not read changed data from source (before reading we clear
> corresponding bit in copy-bitmap, so, this area is not more handled by
> backup-top).
>
> 3. To sync with in-flight requests we now just drain hook node, we
> don't need rw-lock.
>
> 4. After the whole backup loop (top, full, incremental modes), we need
> to check for not copied clusters, which were under backup-top operation
> and we skipped them, but backup-top operation failed, error returned to
> the guest and dirty bits set back.
>
> 5. Don't create additional blk, use backup-top children for copy
> operations.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
Haven't reviewed it yet, but gcc complains correctly here:
block/backup.c: In function 'backup_job_create':
block/backup.c:717:9: error: 'backup_top' may be used uninitialized in this
function [-Werror=maybe-uninitialized]
bdrv_backup_top_drop(backup_top);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
> @@ -653,24 +648,29 @@ BlockJob *backup_job_create(const char *job_id,
> BlockDriverState *bs,
>
> copy_bitmap = hbitmap_alloc(len, ctz32(cluster_size));
>
> + /* bdrv_get_device_name will not help to find device name starting from
> + * @bs after backup-top append, so let's calculate job_id before. Do
> + * it in the same way like block_job_create
> + */
> + if (job_id == NULL && !(creation_flags & JOB_INTERNAL)) {
> + job_id = bdrv_get_device_name(bs);
> + }
> +
> + backup_top = bdrv_backup_top_append(bs, target, copy_bitmap, errp);
> + if (!backup_top) {
> + return NULL;
This needs to be goto error, just like in the bdrv_getlength() failure a
few lines above (which is where the uninitialised backup_top warning
comes from).
Kevin
