> From: Dietmar Maurer <diet...@proxmox.com>
>
> backup_start() creates a block job that copies a point-in-time snapshot
> of a block device to a target block device.
>
> We call backup_do_cow() for each write during backup. That function
> reads the original data from the block device before it gets
> overwritten. The data is then written to the target device.
>
> Currently backup cluster size is hardcoded to 65536 bytes.
>
> [I made a number of changes to Dietmar's original patch and folded them
> in to make code review easy. Here is the full list:
>
> * Drop BackupDumpFunc interface in favor of a target block device
> * Detect zero clusters with buffer_is_zero()
> * Don't write zero clusters to the target
> * Use 0 delay instead of 1us, like other block jobs
> * Unify creation/start functions into backup_start()
> * Simplify cleanup, free bitmap in backup_run() instead of cb
> * function
> * Use HBitmap to avoid duplicating bitmap code
> * Use bdrv_getlength() instead of accessing ->total_sectors
> * directly
> * Delete the backup.h header file, it is no longer necessary
> * Move ./backup.c to block/backup.c
> * Remove #ifdefed out code
> * Coding style and whitespace cleanups
> * Use bdrv_add_before_write_cb() instead of blockjob-specific hooks
> * Keep our own in-flight CowRequest list instead of using block.c
> tracked requests
>
> -- stefanha]
>
> Signed-off-by: Dietmar Maurer <diet...@proxmox.com>
> Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com>
> ---
> block/Makefile.objs | 1 +
> block/backup.c | 282
> ++++++++++++++++++++++++++++++++++++++++++++++
> include/block/block_int.h | 16 +++
> 3 files changed, 299 insertions(+)
> create mode 100644 block/backup.c
>
> diff --git a/block/Makefile.objs b/block/Makefile.objs
> index 5f0358a..88bd101 100644
> --- a/block/Makefile.objs
> +++ b/block/Makefile.objs
> @@ -20,5 +20,6 @@ endif
> common-obj-y += stream.o
> common-obj-y += commit.o
> common-obj-y += mirror.o
> +common-obj-y += backup.o
>
> $(obj)/curl.o: QEMU_CFLAGS+=$(CURL_CFLAGS)
> diff --git a/block/backup.c b/block/backup.c
> new file mode 100644
> index 0000000..01d2fd3
> --- /dev/null
> +++ b/block/backup.c
> @@ -0,0 +1,282 @@
> +/*
> + * QEMU backup
> + *
> + * Copyright (C) 2013 Proxmox Server Solutions
> + *
> + * Authors:
> + * Dietmar Maurer (diet...@proxmox.com)
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include <stdio.h>
> +#include <errno.h>
> +#include <unistd.h>
> +
> +#include "block/block.h"
> +#include "block/block_int.h"
> +#include "block/blockjob.h"
> +#include "qemu/ratelimit.h"
> +
> +#define DEBUG_BACKUP 0
> +
> +#define DPRINTF(fmt, ...) \
> + do { \
> + if (DEBUG_BACKUP) { \
> + fprintf(stderr, "backup: " fmt, ## __VA_ARGS__); \
> + } \
> + } while (0)
> +
> +#define BACKUP_CLUSTER_BITS 16
> +#define BACKUP_CLUSTER_SIZE (1 << BACKUP_CLUSTER_BITS)
> +#define BACKUP_BLOCKS_PER_CLUSTER (BACKUP_CLUSTER_SIZE / BDRV_SECTOR_SIZE)
> +
> +#define SLICE_TIME 100000000ULL /* ns */
> +
> +typedef struct CowRequest {
> + int64_t start;
> + int64_t end;
> + QLIST_ENTRY(CowRequest) list;
> + CoQueue wait_queue; /* coroutines blocked on this request */
> +} CowRequest;
> +
> +typedef struct BackupBlockJob {
> + BlockJob common;
> + BlockDriverState *target;
> + RateLimit limit;
> + CoRwlock flush_rwlock;
> + uint64_t sectors_read;
> + HBitmap *bitmap;
> + QLIST_HEAD(, CowRequest) inflight_reqs;
> +} BackupBlockJob;
> +
> +/* See if in-flight requests overlap and wait for them to complete */
> +static void coroutine_fn wait_for_overlapping_requests(BackupBlockJob *job,
> + int64_t start,
> + int64_t end)
> +{
> + CowRequest *req;
> + bool retry;
> +
> + do {
> + retry = false;
> + QLIST_FOREACH(req, &job->inflight_reqs, list) {
> + if (end > req->start && start < req->end) {
> + qemu_co_queue_wait(&req->wait_queue);
> + retry = true;
> + break;
> + }
> + }
> + } while (retry);
> +}
> +
In my understanding, there will be possible two program routines entering
here at same time since it holds read lock instead of write lock, and
they may also modify job->inflight_reqs, is it possible a race
condition here? I am not sure whether back-ground job will becomes a
thread.
--
Best Regards
Wenchao Xia
