Re: [Qemu-devel] [PATCH COLO-Frame v9 08/32] COLO/migration: establish a new communication path from destination to source

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* 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