| Message ID | 1441182199-8328-9-git-send-email-zhang.zhanghailiang@huawei.com |
|---|---|
| State | New |
| Headers | show |
* 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 <zhang.zhanghailiang@huawei.com> > Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> > Cc: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > 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' > + 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'. > + 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. > 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. Dave > /* 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
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 <zhang.zhanghailiang@huawei.com> >> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> >> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com> >> --- >> 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 > > . >
* 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 <zhang.zhanghailiang@huawei.com> > >>Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> > >>Cc: Dr. David Alan Gilbert <dgilbert@redhat.com> > >>--- > >> 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. 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 came from; especially if the person who sent you the log file didn't tell you much :-) Dave > > > > >> 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 > > > >. > > > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 2015/10/21 3:32, Dr. David Alan Gilbert wrote: > * 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 <zhang.zhanghailiang@huawei.com> >>>> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> >>>> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com> >>>> --- >>>> 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. > > 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 came > from; especially if the person who sent you the log file didn't tell you much :-) > OK, that's a good idea, will fix in next version. Thanks. >> >>> >>>> 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 >>> >>> . >>> >> >> > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK > > . >
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"); + if (!s->from_dst_file) { + ret = -EINVAL; + error_report("Open QEMUFile failed!"); + 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)); + } 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; + } /* 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;