Re: [Qemu-devel] [PATCH COLO-Frame (Base) v21 05/17] COLO: Establish a new communicating path for COLO
On 2016/10/27 11:57, Amit Shah wrote: On (Wed) 26 Oct 2016 [22:05:22], Hailiang Zhang wrote: On 2016/10/26 13:06, Amit Shah wrote: On (Tue) 18 Oct 2016 [20:10:01], zhanghailiang wrote: This new communication path will be used for returning messages >from Secondary side to Primary side. Signed-off-by: zhanghailiangSigned-off-by: Li Zhijian Reviewed-by: Dr. David Alan Gilbert Reviewed-by: Amit Shah @@ -63,8 +75,24 @@ void *colo_process_incoming_thread(void *opaque) migrate_set_state(>state, MIGRATION_STATUS_ACTIVE, MIGRATION_STATUS_COLO); +mis->to_src_file = qemu_file_get_return_path(mis->from_src_file); +if (!mis->to_src_file) { +error_report("COLO incoming thread: Open QEMUFile to_src_file failed"); +goto out; +} +/* + * Note: We set the fd to unblocked in migration incoming coroutine, + * But here we are in the COLO incoming thread, so it is ok to set the + * fd back to blocked. + */ +qemu_file_set_blocking(mis->from_src_file, true); Why does it need to be blocking? Because, the communication/action between Primary side and Secondary side should be sequential. Just as postcopy does. :) Yea - I mean please include that in the comment too so it's obvious why it's done. OK, i will add this in next version, thanks. Amit .
Re: [Qemu-devel] [PATCH COLO-Frame (Base) v21 05/17] COLO: Establish a new communicating path for COLO
On (Wed) 26 Oct 2016 [22:05:22], Hailiang Zhang wrote: > On 2016/10/26 13:06, Amit Shah wrote: > >On (Tue) 18 Oct 2016 [20:10:01], zhanghailiang wrote: > >>This new communication path will be used for returning messages > >>from Secondary side to Primary side. > >> > >>Signed-off-by: zhanghailiang> >>Signed-off-by: Li Zhijian > >>Reviewed-by: Dr. David Alan Gilbert > > > >Reviewed-by: Amit Shah > > > >>@@ -63,8 +75,24 @@ void *colo_process_incoming_thread(void *opaque) > >> migrate_set_state(>state, MIGRATION_STATUS_ACTIVE, > >>MIGRATION_STATUS_COLO); > >> > >>+mis->to_src_file = qemu_file_get_return_path(mis->from_src_file); > >>+if (!mis->to_src_file) { > >>+error_report("COLO incoming thread: Open QEMUFile to_src_file > >>failed"); > >>+goto out; > >>+} > >>+/* > >>+ * Note: We set the fd to unblocked in migration incoming coroutine, > >>+ * But here we are in the COLO incoming thread, so it is ok to set the > >>+ * fd back to blocked. > >>+ */ > >>+qemu_file_set_blocking(mis->from_src_file, true); > > > >Why does it need to be blocking? > > > > Because, the communication/action between Primary side and Secondary side > should be > sequential. Just as postcopy does. :) Yea - I mean please include that in the comment too so it's obvious why it's done. Amit
Re: [Qemu-devel] [PATCH COLO-Frame (Base) v21 05/17] COLO: Establish a new communicating path for COLO
On 2016/10/26 13:06, Amit Shah wrote: On (Tue) 18 Oct 2016 [20:10:01], zhanghailiang wrote: This new communication path will be used for returning messages from Secondary side to Primary side. Signed-off-by: zhanghailiangSigned-off-by: Li Zhijian Reviewed-by: Dr. David Alan Gilbert Reviewed-by: Amit Shah @@ -63,8 +75,24 @@ void *colo_process_incoming_thread(void *opaque) migrate_set_state(>state, MIGRATION_STATUS_ACTIVE, MIGRATION_STATUS_COLO); +mis->to_src_file = qemu_file_get_return_path(mis->from_src_file); +if (!mis->to_src_file) { +error_report("COLO incoming thread: Open QEMUFile to_src_file failed"); +goto out; +} +/* + * Note: We set the fd to unblocked in migration incoming coroutine, + * But here we are in the COLO incoming thread, so it is ok to set the + * fd back to blocked. + */ +qemu_file_set_blocking(mis->from_src_file, true); Why does it need to be blocking? Because, the communication/action between Primary side and Secondary side should be sequential. Just as postcopy does. :) Thanks, hailiang Amit .
Re: [Qemu-devel] [PATCH COLO-Frame (Base) v21 05/17] COLO: Establish a new communicating path for COLO
On (Tue) 18 Oct 2016 [20:10:01], zhanghailiang wrote: > This new communication path will be used for returning messages > from Secondary side to Primary side. > > Signed-off-by: zhanghailiang> Signed-off-by: Li Zhijian > Reviewed-by: Dr. David Alan Gilbert Reviewed-by: Amit Shah > @@ -63,8 +75,24 @@ void *colo_process_incoming_thread(void *opaque) > migrate_set_state(>state, MIGRATION_STATUS_ACTIVE, >MIGRATION_STATUS_COLO); > > +mis->to_src_file = qemu_file_get_return_path(mis->from_src_file); > +if (!mis->to_src_file) { > +error_report("COLO incoming thread: Open QEMUFile to_src_file > failed"); > +goto out; > +} > +/* > + * Note: We set the fd to unblocked in migration incoming coroutine, > + * But here we are in the COLO incoming thread, so it is ok to set the > + * fd back to blocked. > + */ > +qemu_file_set_blocking(mis->from_src_file, true); Why does it need to be blocking? Amit
[Qemu-devel] [PATCH COLO-Frame (Base) v21 05/17] COLO: Establish a new communicating path for COLO
This new communication path will be used for returning messages from Secondary side to Primary side. Signed-off-by: zhanghailiangSigned-off-by: Li Zhijian Reviewed-by: Dr. David Alan Gilbert --- v13: - Remove useless error report v12: - Add Reviewed-by tag v11: - Rebase master to use qemu_file_get_return_path() for opening return path v10: - fix the the error log (Dave's suggestion). --- migration/colo.c | 28 1 file changed, 28 insertions(+) diff --git a/migration/colo.c b/migration/colo.c index 968cd51..7c5769b 100644 --- a/migration/colo.c +++ b/migration/colo.c @@ -14,6 +14,7 @@ #include "sysemu/sysemu.h" #include "migration/colo.h" #include "trace.h" +#include "qemu/error-report.h" bool colo_supported(void) { @@ -36,6 +37,12 @@ bool migration_incoming_in_colo_state(void) static void colo_process_checkpoint(MigrationState *s) { +s->rp_state.from_dst_file = qemu_file_get_return_path(s->to_dst_file); +if (!s->rp_state.from_dst_file) { +error_report("Open QEMUFile from_dst_file failed"); +goto out; +} + qemu_mutex_lock_iothread(); vm_start(); qemu_mutex_unlock_iothread(); @@ -43,8 +50,13 @@ static void colo_process_checkpoint(MigrationState *s) /* TODO: COLO checkpoint savevm loop */ +out: migrate_set_state(>state, MIGRATION_STATUS_COLO, MIGRATION_STATUS_COMPLETED); + +if (s->rp_state.from_dst_file) { +qemu_fclose(s->rp_state.from_dst_file); +} } void migrate_start_colo_process(MigrationState *s) @@ -63,8 +75,24 @@ void *colo_process_incoming_thread(void *opaque) migrate_set_state(>state, MIGRATION_STATUS_ACTIVE, MIGRATION_STATUS_COLO); +mis->to_src_file = qemu_file_get_return_path(mis->from_src_file); +if (!mis->to_src_file) { +error_report("COLO incoming thread: Open QEMUFile to_src_file failed"); +goto out; +} +/* + * Note: We set the fd to unblocked in migration incoming coroutine, + * But here we are in the COLO incoming thread, so it is ok to set the + * fd back to blocked. + */ +qemu_file_set_blocking(mis->from_src_file, true); + /* TODO: COLO checkpoint restore loop */ +out: +if (mis->to_src_file) { +qemu_fclose(mis->to_src_file); +} migration_incoming_exit_colo(); return NULL; -- 1.8.3.1