Re: [Qemu-block] [PATCH v22 00/10] Block replication for continuous checkpoints
On 07/25/2016 10:34 PM, Stefan Hajnoczi wrote: On Mon, Jul 25, 2016 at 11:44:34AM +0800, Changlong Xie wrote: COLO block is the necessary prerequisite of COLO framework and COLO network, what are blocked by these patchsets now. Since v19, Stefan said he had reviewed most part of this patchsets. So, this series *REALLY* need more comments from all of you. I've ping so many times, but no response until now. All i have to do is rebase them to the lastest code. Do you have time to review this? Any response would be appreciate. There has been no review activity for a long time. You have been patient and addressed issues that I raised back when I reviewed the series. Both you and I have pinged maintainers numerous times. It's not fair to keep this out of tree at this stage. Since no one else seems to care I suggest rebasing/testing a final time with the following changes: 1. Update the MAINTAINERS for new files you are adding. 2. Make the feature optional in ./configure so distros that don't feel comfortable shipping it yet can easily disable it: $ ./configure --disable-replication && make -j4 Will follow you advise in next version. Then I will merge it if there are no comments. Stefan
Re: [Qemu-block] [PATCH v22 00/10] Block replication for continuous checkpoints
On 25.07.2016 16:34, Stefan Hajnoczi wrote: > On Mon, Jul 25, 2016 at 11:44:34AM +0800, Changlong Xie wrote: >> COLO block is the necessary prerequisite of COLO framework and COLO network, >> what are blocked by these patchsets now. >> >> Since v19, Stefan said he had reviewed most part of this patchsets. So, this >> series *REALLY* need more comments from all of you. >> >> I've ping so many times, but no response until now. All i have to do is >> rebase them to the lastest code. >> >> Do you have time to review this? Any response would be appreciate. > > There has been no review activity for a long time. You have been > patient and addressed issues that I raised back when I reviewed the > series. Both you and I have pinged maintainers numerous times. It's > not fair to keep this out of tree at this stage. > > Since no one else seems to care I suggest rebasing/testing a final time > with the following changes: > > 1. Update the MAINTAINERS for new files you are adding. > 2. Make the feature optional in ./configure so distros that don't feel >comfortable shipping it yet can easily disable it: > > $ ./configure --disable-replication && make -j4 > > Then I will merge it if there are no comments. I'd be completely happy with this. I only had minor comments, nothing critical, but mind you, I have not looked at the series in depth. I know it's my own fault for not doing so, but I did actually review the series back in v2, but I remember that it was rather difficult to delve into it (because this series is just part of a big picture) and I couldn't keep up with the following revisions, so I fell out of the loop and felt that you and Eric took over reviewing. I find the documentation rather hard to understand (but maybe that's just me) and the fact that it's in the middle of this series instead of at the start doesn't really help. Because of this, I don't feel quite comfortable taking this through my tree. That would be a different story if every patch had an R-b from you or Eric, but they don't. But that said, my not-so-deep review did not result in me finding any way how these patches would do harm to qemu. Therefore, if we do have a maintainer for them, I'd be completely fine with them getting merged. Max signature.asc Description: OpenPGP digital signature
Re: [Qemu-block] [PATCH v22 00/10] Block replication for continuous checkpoints
On Mon, Jul 25, 2016 at 11:44:34AM +0800, Changlong Xie wrote: > COLO block is the necessary prerequisite of COLO framework and COLO network, > what are blocked by these patchsets now. > > Since v19, Stefan said he had reviewed most part of this patchsets. So, this > series *REALLY* need more comments from all of you. > > I've ping so many times, but no response until now. All i have to do is > rebase them to the lastest code. > > Do you have time to review this? Any response would be appreciate. There has been no review activity for a long time. You have been patient and addressed issues that I raised back when I reviewed the series. Both you and I have pinged maintainers numerous times. It's not fair to keep this out of tree at this stage. Since no one else seems to care I suggest rebasing/testing a final time with the following changes: 1. Update the MAINTAINERS for new files you are adding. 2. Make the feature optional in ./configure so distros that don't feel comfortable shipping it yet can easily disable it: $ ./configure --disable-replication && make -j4 Then I will merge it if there are no comments. Stefan signature.asc Description: PGP signature
Re: [Qemu-block] [PATCH v22 00/10] Block replication for continuous checkpoints
Hi all block maintainers Sorry to bother. COLO block is the necessary prerequisite of COLO framework and COLO network, what are blocked by these patchsets now. Since v19, Stefan said he had reviewed most part of this patchsets. So, this series *REALLY* need more comments from all of you. I've ping so many times, but no response until now. All i have to do is rebase them to the lastest code. Do you have time to review this? Any response would be appreciate. Thanks -Xie On 07/22/2016 06:15 PM, Wang WeiWei 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/wangww-fnst/qemu/tree/block-replication-v22 You can get the patch with framework here: https://github.com/wangww-fnst/qemu/tree/colo_framework_v21 TODO: 1. Continuous block replication. It will be started after basic functions are accepted. Changs Log: V22: 1. Rebase to the lastest code 2. modify code adapt to the modification of backup_start & commit_active_start 3. rewrite io_read & io_write for interface changes V21: 1. Rebase to the lastest code 2. use bdrv_pwrite_zeroes() and BDRV_SECTOR_BITS for p9 V20 Resend: 1. Resend to avoid bothering qemu-trivial maintainers 2. Address comments from Eric, fix header file issue and add a brief commit message for p7 V20: 1. Rebase to the lastest code 2. Address comments from stefan p8: 1. error_setg() with an error message when check_top_bs() fails. 2. remove bdrv_ref(s->hidden_disk->bs) since commit 5c438bc6 3. use bloc_job_cancel_sync() before active commit p9: 1. fix uninitialized 'pattern_buf' 2. introduce mkstemp(3) to fix unique filenames 3. use qemu_vfree() for qemu_blockalign() memory 4. add missing replication_start_all() 5. remove useless pattern for io_write() 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
