From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53772) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b7gng-000888-OZ for qemu-devel@nongnu.org; Tue, 31 May 2016 06:21:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b7gnf-00022x-4p for qemu-devel@nongnu.org; Tue, 31 May 2016 06:21:28 -0400 Message-ID: <574D668C.4050102@cn.fujitsu.com> Date: Tue, 31 May 2016 18:25:16 +0800 From: Changlong Xie MIME-Version: 1.0 References: <1463729780-31982-1-git-send-email-xiecl.fnst@cn.fujitsu.com> <20160530182016.GC1366@stefanha-x1.localdomain> In-Reply-To: <20160530182016.GC1366@stefanha-x1.localdomain> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Qemu-block] [PATCH v19 00/10] Block replication for continuous checkpoints List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: qemu devel , Stefan Hajnoczi , Fam Zheng , Max Reitz , Kevin Wolf , Jeff Cody , Wen Congyang , zhanghailiang , qemu block , Jiang Yunhong , Dong Eddie , "Dr. David Alan Gilbert" , Markus Armbruster , Gonglei , Paolo Bonzini On 05/31/2016 02:20 AM, Stefan Hajnoczi wrote: > On Fri, May 20, 2016 at 03:36:10PM +0800, Changlong Xie wrote: >> Block replication is a very important feature which is used for >> continuous checkpoints(for example: COLO). >> >> You can get the detailed information about block replication from here: >> http://wiki.qemu.org/Features/BlockReplication >> >> Usage: >> Please refer to docs/block-replication.txt >> >> You can get the patch here: >> https://github.com/Pating/qemu/tree/changlox/block-replication-v19 >> >> You can get the patch with framework here: >> https://github.com/Pating/qemu/tree/changlox/colo_framework_v18 >> >> TODO: >> 1. Continuous block replication. It will be started after basic functions >> are accepted. >> >> Changs Log: >> V19: >> 1. Rebase to v2.6.0 >> 2. Address comments from stefan >> p3: a new patch that export interfaces for extra serialization >> p8: >> 1. call replication_stop() before freeing s->top_id >> 2. check top_bs >> 3. reopen file readonly in error return paths >> 4. enable extra serialization between read and COW >> p9: try to hanlde SIGABRT >> V18: >> p6: add local_err in all replication callbacks to prevent "errp == NULL" >> p7: add missing qemu_iovec_destroy(xxx) >> V17: >> 1. Rebase to the lastest codes >> p2: refactor backup_do_checkpoint addressed comments from Jeff Cody >> p4: fix bugs in "drive_add buddy xxx" hmp commands >> p6: add "since: 2.7" >> p7: fix bug in replication_close(), add missing "qapi/error.h", add test-replication >> p8: add "since: 2.7" >> V16: >> 1. Rebase to the newest codes >> 2. Address comments from Stefan & hailiang >> p3: we don't need this patch now >> p4: add "top-id" parameters for secondary >> p6: fix NULL pointer in replication callbacks, remove unnecessary typedefs, >> add doc comments that explain the semantics of Replication >> p7: Refactor AioContext for thread-safe, remove unnecessary get_top_bs() >> *Note*: I'm working on replication testcase now, will send out in V17 >> V15: >> 1. Rebase to the newest codes >> 2. Fix typos and coding style addresed Eric's comments >> 3. Address Stefan's comments >> 1) Make backup_do_checkpoint public, drop the changes on BlockJobDriver >> 2) Update the message and description for [PATCH 4/9] >> 3) Make replication_(start/stop/do_checkpoint)_all as global interfaces >> 4) Introduce AioContext lock to protect start/stop/do_checkpoint callbacks >> 5) Use BdrvChild instead of holding on to BlockDriverState * pointers >> 4. Clear BDRV_O_INACTIVE for hidden disk's open_flags since commit 09e0c771 >> 5. Introduce replication_get_error_all to check replication status >> 6. Remove useless discard interface >> V14: >> 1. Implement auto complete active commit >> 2. Implement active commit block job for replication.c >> 3. Address the comments from Stefan, add replication-specific API and data >> structure, also remove old block layer APIs >> V13: >> 1. Rebase to the newest codes >> 2. Remove redundant marcos and semicolon in replication.c >> 3. Fix typos in block-replication.txt >> V12: >> 1. Rebase to the newest codes >> 2. Use backing reference to replcace 'allow-write-backing-file' >> V11: >> 1. Reopen the backing file when starting blcok replication if it is not >> opened in R/W mode >> 2. Unblock BLOCK_OP_TYPE_BACKUP_SOURCE and BLOCK_OP_TYPE_BACKUP_TARGET >> when opening backing file >> 3. Block the top BDS so there is only one block job for the top BDS and >> its backing chain. >> V10: >> 1. Use blockdev-remove-medium and blockdev-insert-medium to replace backing >> reference. >> 2. Address the comments from Eric Blake >> V9: >> 1. Update the error messages >> 2. Rebase to the newest qemu >> 3. Split child add/delete support. These patches are sent in another patchset. >> V8: >> 1. Address Alberto Garcia's comments >> V7: >> 1. Implement adding/removing quorum child. Remove the option non-connect. >> 2. Simplify the backing refrence option according to Stefan Hajnoczi's suggestion >> V6: >> 1. Rebase to the newest qemu. >> V5: >> 1. Address the comments from Gong Lei >> 2. Speed the failover up. The secondary vm can take over very quickly even >> if there are too many I/O requests. >> V4: >> 1. Introduce a new driver replication to avoid touch nbd and qcow2. >> V3: >> 1: use error_setg() instead of error_set() >> 2. Add a new block job API >> 3. Active disk, hidden disk and nbd target uses the same AioContext >> 4. Add a testcase to test new hbitmap API >> V2: >> 1. Redesign the secondary qemu(use image-fleecing) >> 2. Use Error objects to return error message >> 3. Address the comments from Max Reitz and Eric Blake >> >> Changlong Xie (3): >> Backup: export interfaces for extra serialization >> Introduce new APIs to do replication operation >> tests: add unit test case for replication >> >> Wen Congyang (7): >> unblock backup operations in backing file >> Backup: clear all bitmap when doing block checkpoint >> Link backup into block core >> docs: block replication's description >> auto complete active commit >> Implement new driver for block replication >> support replication driver in blockdev-add >> >> Makefile.objs | 1 + >> block.c | 17 ++ >> block/Makefile.objs | 3 +- >> block/backup.c | 59 +++- >> block/mirror.c | 13 +- >> block/replication.c | 666 +++++++++++++++++++++++++++++++++++++++++++ >> blockdev.c | 2 +- >> docs/block-replication.txt | 239 ++++++++++++++++ >> include/block/block_backup.h | 17 ++ >> include/block/block_int.h | 3 +- >> qapi/block-core.json | 33 ++- >> qemu-img.c | 2 +- >> replication.c | 105 +++++++ >> replication.h | 176 ++++++++++++ >> tests/.gitignore | 1 + >> tests/Makefile | 4 + >> tests/test-replication.c | 523 +++++++++++++++++++++++++++++++++ >> 17 files changed, 1847 insertions(+), 17 deletions(-) >> create mode 100644 block/replication.c >> create mode 100644 docs/block-replication.txt >> create mode 100644 include/block/block_backup.h >> create mode 100644 replication.c >> create mode 100644 replication.h >> create mode 100644 tests/test-replication.c > > I have reviewed many revisions of this series. The main mechanism in > this series makes sense to me. > Thanks. > I'm still concerned that checkpointing (vm_stop(), not in this series > but COLO in general) depends on bdrv_drain(), which can block forever if > I/O is hung. That doesn't seem like a reasonable limitation for a high > availability feature since it may lead to the VM becoming unavailable. IIRC, this issue is mentioned in my older patchset. > > I'd like Jeff and/or Kevin to review this series and merge it once they > are happy. > @jeff, kevin and all block maintainters, would you like to review this patchset? Thanks -Xie > Stefan >