From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:39212) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UcorK-0005sB-O7 for qemu-devel@nongnu.org; Wed, 15 May 2013 23:28:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UcorI-0003bJ-90 for qemu-devel@nongnu.org; Wed, 15 May 2013 23:28:02 -0400 Received: from e23smtp06.au.ibm.com ([202.81.31.148]:54327) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UcorH-0003aM-N3 for qemu-devel@nongnu.org; Wed, 15 May 2013 23:28:00 -0400 Received: from /spool/local by e23smtp06.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 16 May 2013 13:21:32 +1000 Received: from d23relay03.au.ibm.com (d23relay03.au.ibm.com [9.190.235.21]) by d23dlp03.au.ibm.com (Postfix) with ESMTP id 64E423578052 for ; Thu, 16 May 2013 13:27:51 +1000 (EST) Received: from d23av01.au.ibm.com (d23av01.au.ibm.com [9.190.234.96]) by d23relay03.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r4G3RfkH18022648 for ; Thu, 16 May 2013 13:27:44 +1000 Received: from d23av01.au.ibm.com (loopback [127.0.0.1]) by d23av01.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r4G3RmC3030091 for ; Thu, 16 May 2013 13:27:48 +1000 Message-ID: <5194522A.3010101@linux.vnet.ibm.com> Date: Thu, 16 May 2013 11:27:38 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1368628476-19622-1-git-send-email-stefanha@redhat.com> <1368628476-19622-3-git-send-email-stefanha@redhat.com> In-Reply-To: <1368628476-19622-3-git-send-email-stefanha@redhat.com> Content-Type: text/plain; charset=GB2312 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 2/8] block: add basic backup support to block driver List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Kevin Wolf , Fam Zheng , qemu-devel@nongnu.org, imain@redhat.com, Paolo Bonzini , dietmar@proxmox.com > From: Dietmar Maurer > > 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 > Signed-off-by: Stefan Hajnoczi > --- > 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 (dietmar@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 > +#include > +#include > + > +#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