Re: [PATCH v5 42/50] multi-process/mig: Send VMSD of remote to the Proxy object

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

* Jagannathan Raman (jag.raman@oracle.com) wrote:
> The remote process sends the VMSD to the Proxy object, on the source
> side
> 
> Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>
> Signed-off-by: John G Johnson <john.g.johnson@oracle.com>
> Signed-off-by: Jagannathan Raman <jag.raman@oracle.com>
> ---
>  migration/savevm.c   | 27 +++++++++++++++++++++++++++
>  migration/savevm.h   |  2 ++
>  remote/remote-main.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 72 insertions(+)
> 
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 1d4220e..09af14d 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2942,3 +2942,30 @@ bool vmstate_check_only_migratable(const VMStateDescription *vmsd)
>  
>      return !(vmsd && vmsd->unmigratable);
>  }
> +

Can we add something here commenting, e.g.
/* Called by the remote process to serialise migration back to the qemu
 * */
> +int qemu_remote_savevm(QEMUFile *f)
> +{
> +    SaveStateEntry *se;
> +    int ret;
> +
> +    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
> +        if (!se->vmsd || !vmstate_save_needed(se->vmsd, se->opaque)) {
> +            continue;
> +        }
> +
> +        save_section_header(f, se, QEMU_VM_SECTION_FULL);
> +
> +        ret = vmstate_save(f, se, NULL);
> +        if (ret) {
> +            qemu_file_set_error(f, ret);
> +            return ret;
> +        }
> +
> +        save_section_footer(f, se);
> +    }
> +
> +    qemu_put_byte(f, QEMU_VM_EOF);
> +    qemu_fflush(f);

You have a qemu_fflush in process_start_mig_out  just after you call it
- so you don't need both; I suggest you remove this one.

> +    return 0;

And make this return qemu_file_get_error(f);  just like
qemu_save_device_state and then makybe some day we can merge them.

> +}


> diff --git a/migration/savevm.h b/migration/savevm.h
> index ba64a7e..0491d3a 100644
> --- a/migration/savevm.h
> +++ b/migration/savevm.h
> @@ -65,4 +65,6 @@ void qemu_loadvm_state_cleanup(void);
>  int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis);
>  int qemu_load_device_state(QEMUFile *f);
>  
> +int qemu_remote_savevm(QEMUFile *f);
> +
>  #endif
> diff --git a/remote/remote-main.c b/remote/remote-main.c
> index 58d9905..e97eb76 100644
> --- a/remote/remote-main.c
> +++ b/remote/remote-main.c
> @@ -53,6 +53,16 @@
>  #include "qemu/log.h"
>  #include "qemu/cutils.h"
>  #include "remote-opts.h"
> +#include "qapi/error.h"
> +#include "io/channel-util.h"
> +
> +#include "io/channel.h"
> +#include "io/channel-socket.h"
> +#include "migration/qemu-file-types.h"
> +#include "migration/savevm.h"
> +#include "migration/qemu-file-channel.h"
> +#include "migration/qemu-file.h"
> +
>  #include "monitor/monitor.h"
>  #include "chardev/char.h"
>  #include "sysemu/reset.h"
> @@ -322,6 +332,36 @@ static int setup_device(MPQemuMsg *msg, Error **errp)
>  
>  }
>  
> +static void process_start_mig_out(MPQemuMsg *msg)
> +{
> +    int wait = msg->fds[1];
> +    Error *err = NULL;
> +    QIOChannel *ioc;
> +    QEMUFile *f;
> +
> +    ioc = qio_channel_new_fd(msg->fds[0], &err);
> +    if (err) {
> +        error_report_err(err);
> +        return;
> +    }
> +
> +    qio_channel_set_name(QIO_CHANNEL(ioc), "remote-migration-channel");
> +
> +    f = qemu_fopen_channel_output(ioc);
> +
> +    bdrv_drain_all();
> +    (void)bdrv_flush_all();

Do remote process always have block code? I mean can't we have a remote
process that's just say a NIC ?

> +    (void)qemu_remote_savevm(f);

It's probably bad to ignore errors here; what you could do is if there's
an error, you shoul dprint something and then send a poison value back
to the QEMU to let it know that you've failed.

> +    qemu_fflush(f);
> +
> +    notify_proxy(wait, (uint64_t)qemu_ftell(f));
> +    PUT_REMOTE_WAIT(wait);
> +
> +    qemu_fclose(f);
> +}
> +
>  static void process_msg(GIOCondition cond, MPQemuChannel *chan)
>  {
>      MPQemuMsg *msg = NULL;
> @@ -411,6 +451,9 @@ static void process_msg(GIOCondition cond, MPQemuChannel *chan)
>              notify_proxy(msg->fds[0], 0);
>          }
>          break;
> +    case START_MIG_OUT:
> +        process_start_mig_out(msg);
> +        break;
>      default:
>          error_setg(&err, "Unknown command");
>          goto finalize_loop;
> -- 
> 1.8.3.1

--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK