From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53366) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZoSKR-0002it-OS for qemu-devel@nongnu.org; Tue, 20 Oct 2015 04:31:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZoSKN-000537-Hi for qemu-devel@nongnu.org; Tue, 20 Oct 2015 04:31:31 -0400 Received: from szxga01-in.huawei.com ([58.251.152.64]:37392) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZoSKM-00052O-G1 for qemu-devel@nongnu.org; Tue, 20 Oct 2015 04:31:27 -0400 References: <1441182199-8328-1-git-send-email-zhang.zhanghailiang@huawei.com> <1441182199-8328-9-git-send-email-zhang.zhanghailiang@huawei.com> <20151019095422.GB2462@work-vm> From: zhanghailiang Message-ID: <5625FBC3.9040505@huawei.com> Date: Tue, 20 Oct 2015 16:30:59 +0800 MIME-Version: 1.0 In-Reply-To: <20151019095422.GB2462@work-vm> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH COLO-Frame v9 08/32] COLO/migration: establish a new communication path from destination to source List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert" Cc: lizhijian@cn.fujitsu.com, quintela@redhat.com, yunhong.jiang@intel.com, eddie.dong@intel.com, peter.huangpeng@huawei.com, qemu-devel@nongnu.org, arei.gonglei@huawei.com, stefanha@redhat.com, amit.shah@redhat.com, yanghy@cn.fujitsu.com On 2015/10/19 17:54, Dr. David Alan Gilbert wrote: > * zhanghailiang (zhang.zhanghailiang@huawei.com) wrote: >> Add a new member 'to_src_file' to MigrationIncomingState and a >> new member 'from_dst_file' to MigrationState. >> They will be used for returning messages from destination to source. >> It will also be used by post-copy migration. >> >> Signed-off-by: zhanghailiang >> Signed-off-by: Li Zhijian >> Cc: Dr. David Alan Gilbert >> --- >> include/migration/migration.h | 3 ++- >> migration/colo.c | 43 +++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 45 insertions(+), 1 deletion(-) >> >> diff --git a/include/migration/migration.h b/include/migration/migration.h >> index 6488e03..0c94103 100644 >> --- a/include/migration/migration.h >> +++ b/include/migration/migration.h >> @@ -50,7 +50,7 @@ typedef QLIST_HEAD(, LoadStateEntry) LoadStateEntry_Head; >> /* State for the incoming migration */ >> struct MigrationIncomingState { >> QEMUFile *from_src_file; >> - >> + QEMUFile *to_src_file; >> int state; >> >> bool have_colo_incoming_thread; >> @@ -74,6 +74,7 @@ struct MigrationState >> QemuThread thread; >> QEMUBH *cleanup_bh; >> QEMUFile *to_dst_file; >> + QEMUFile *from_dst_file; >> int parameters[MIGRATION_PARAMETER_MAX]; >> >> int state; >> diff --git a/migration/colo.c b/migration/colo.c >> index a341eee..5f4fb20 100644 >> --- a/migration/colo.c >> +++ b/migration/colo.c >> @@ -39,6 +39,20 @@ bool migration_incoming_in_colo_state(void) >> static void *colo_thread(void *opaque) >> { >> MigrationState *s = opaque; >> + int fd, ret = 0; >> + >> + /* Dup the fd of to_dst_file */ >> + fd = dup(qemu_get_fd(s->to_dst_file)); >> + if (fd == -1) { >> + ret = -errno; >> + goto out; >> + } >> + s->from_dst_file = qemu_fopen_socket(fd, "rb"); > > In my postcopy code I add the return-path opening as a new > method on QEMUFile, that way if we get a return path working > on another transport (RDMA which I hope to do) then it > works; have a look at 'Return path: Open a return path on QEMUFile for sockets' > I have looked at it. That's a good solution, we use the same fd for return path, and i don't have to call qemu_file_shutdown two times in failover process. >> + if (!s->from_dst_file) { >> + ret = -EINVAL; >> + error_report("Open QEMUFile failed!"); > > In errors, try to give detail of where a problem was; > e.g. 'colo_thread: Open QEMUFile from_dst failed'. > OK. I will fix it in next version. >> + goto out; >> + } >> >> qemu_mutex_lock_iothread(); >> vm_start(); >> @@ -47,9 +61,17 @@ static void *colo_thread(void *opaque) >> >> /*TODO: COLO checkpoint savevm loop*/ >> >> +out: >> + if (ret < 0) { >> + error_report("Detect some error: %s", strerror(-ret)); >> + } > > Again, best to say where the error happened. > Hmm, it is a little difficult to say where exactly this error happened here, what i can do is to error out in the place where the error happened. Here is only a summary for the error. > >> migrate_set_state(&s->state, MIGRATION_STATUS_COLO, >> MIGRATION_STATUS_COMPLETED); >> >> + if (s->from_dst_file) { >> + qemu_fclose(s->from_dst_file); >> + } >> + >> qemu_mutex_lock_iothread(); >> qemu_bh_schedule(s->cleanup_bh); >> qemu_mutex_unlock_iothread(); >> @@ -86,12 +108,33 @@ void colo_init_checkpointer(MigrationState *s) >> void *colo_process_incoming_thread(void *opaque) >> { >> MigrationIncomingState *mis = opaque; >> + int fd, ret = 0; >> >> migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE, >> MIGRATION_STATUS_COLO); >> >> + fd = dup(qemu_get_fd(mis->from_src_file)); >> + if (fd < 0) { >> + ret = -errno; >> + goto out; >> + } >> + mis->to_src_file = qemu_fopen_socket(fd, "wb"); >> + if (!mis->to_src_file) { >> + ret = -EINVAL; >> + error_report("Can't open incoming channel!"); >> + goto out; >> + } > > Same as above. OK. Will fix it. Thanks, zhanghailiang > >> /* TODO: COLO checkpoint restore loop */ >> >> +out: >> + if (ret < 0) { > >> + error_report("colo incoming thread will exit, detect error: %s", >> + strerror(-ret)); >> + } >> + >> + if (mis->to_src_file) { >> + qemu_fclose(mis->to_src_file); >> + } >> migration_incoming_exit_colo(); >> >> return NULL; >> -- >> 1.8.3.1 >> >> > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK > > . >