From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58520) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zocei-0006wM-Lc for qemu-devel@nongnu.org; Tue, 20 Oct 2015 15:33:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zocee-0000Un-DV for qemu-devel@nongnu.org; Tue, 20 Oct 2015 15:33:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:43931) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zocee-0000Ui-5g for qemu-devel@nongnu.org; Tue, 20 Oct 2015 15:33:04 -0400 Date: Tue, 20 Oct 2015 20:32:57 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20151020193256.GD2520@work-vm> 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> <5625FBC3.9040505@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable In-Reply-To: <5625FBC3.9040505@huawei.com> 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: zhanghailiang 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 * zhanghailiang (zhang.zhanghailiang@huawei.com) wrote: > 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/migratio= n.h > >>index 6488e03..0c94103 100644 > >>--- a/include/migration/migration.h > >>+++ b/include/migration/migration.h > >>@@ -50,7 +50,7 @@ typedef QLIST_HEAD(, LoadStateEntry) LoadStateEntry_H= ead; > >> /* 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 =3D opaque; > >>+ int fd, ret =3D 0; > >>+ > >>+ /* Dup the fd of to_dst_file */ > >>+ fd =3D dup(qemu_get_fd(s->to_dst_file)); > >>+ if (fd =3D=3D -1) { > >>+ ret =3D -errno; > >>+ goto out; > >>+ } > >>+ s->from_dst_file =3D 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' > > >=20 > I have looked at it. That's a good solution, we use the same fd for retur= n path, and > i don't have to call qemu_file_shutdown two times in failover process. >=20 > >>+ if (!s->from_dst_file) { > >>+ ret =3D -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'. > > >=20 > OK. I will fix it in next version. >=20 > >>+ 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. > > >=20 > Hmm, it is a little difficult to say where exactly this error happened he= re, > what i can do is to error out in the place where the error happened. > Here is only a summary for the error. OK, just add the function name then, e.g. error_report("%s: %s", __func__, strerror(-ret)); it just helps when you're looking at a log file, to understand where it c= ame =66rom; especially if the person who sent you the log file didn't tell you = much :-) Dave >=20 > > > >> 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 =3D opaque; > >>+ int fd, ret =3D 0; > >> > >> migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE, > >> MIGRATION_STATUS_COLO); > >> > >>+ fd =3D dup(qemu_get_fd(mis->from_src_file)); > >>+ if (fd < 0) { > >>+ ret =3D -errno; > >>+ goto out; > >>+ } > >>+ mis->to_src_file =3D qemu_fopen_socket(fd, "wb"); > >>+ if (!mis->to_src_file) { > >>+ ret =3D -EINVAL; > >>+ error_report("Can't open incoming channel!"); > >>+ goto out; > >>+ } > > > >Same as above. >=20 > OK. Will fix it. >=20 > 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 > > > >. > > >=20 >=20 -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK