Re: [Qemu-devel] [PATCH v5 18/45] MIG_CMD_PACKAGED: Send a packaged chunk of migration stream

[David Gibson]

On Wed, Feb 25, 2015 at 04:51:41PM +0000, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <address@hidden>
> 
> MIG_CMD_PACKAGED is a migration command that allows a chunk
> of migration stream to be sent in one go, and be received by
> a separate instance of the loadvm loop while not interacting
> with the migration stream.

Hrm.  I'd be more comfortable if the semantics of CMD_PACKAGED were
defined in terms of visible effects on the other end, rather than in
terms of how it's implemented internally.

> This is used by postcopy to load device state (from the package)
> while loading memory pages from the main stream.

Which makes the above paragraph a bit misleading - the whole point
here is that loading the package data *does* interact with the
migration stream - just that it's the migration stream after the end
of the package.

> Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> ---
>  include/sysemu/sysemu.h |  4 +++
>  savevm.c                | 82 
> +++++++++++++++++++++++++++++++++++++++++++++++++
>  trace-events            |  4 +++
>  3 files changed, 90 insertions(+)
> 
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index d6a6d51..e83bf80 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -87,6 +87,7 @@ enum qemu_vm_cmd {
>      MIG_CMD_INVALID = 0,       /* Must be 0 */
>      MIG_CMD_OPEN_RETURN_PATH,  /* Tell the dest to open the Return path */
>      MIG_CMD_PING,              /* Request a PONG on the RP */
> +    MIG_CMD_PACKAGED,          /* Send a wrapped stream within this stream */
>  
>      MIG_CMD_POSTCOPY_ADVISE = 20,  /* Prior to any page transfers, just
>                                        warn we might want to do PC */
> @@ -101,6 +102,8 @@ enum qemu_vm_cmd {
>  
>  };
>  
> +#define MAX_VM_CMD_PACKAGED_SIZE (1ul << 24)
> +
>  bool qemu_savevm_state_blocked(Error **errp);
>  void qemu_savevm_state_begin(QEMUFile *f,
>                               const MigrationParams *params);
> @@ -113,6 +116,7 @@ void qemu_savevm_command_send(QEMUFile *f, enum 
> qemu_vm_cmd command,
>                                uint16_t len, uint8_t *data);
>  void qemu_savevm_send_ping(QEMUFile *f, uint32_t value);
>  void qemu_savevm_send_open_return_path(QEMUFile *f);
> +void qemu_savevm_send_packaged(QEMUFile *f, const QEMUSizedBuffer *qsb);
>  void qemu_savevm_send_postcopy_advise(QEMUFile *f);
>  void qemu_savevm_send_postcopy_listen(QEMUFile *f);
>  void qemu_savevm_send_postcopy_run(QEMUFile *f);
> diff --git a/savevm.c b/savevm.c
> index e31ccb0..f65bff3 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -636,6 +636,38 @@ void qemu_savevm_send_open_return_path(QEMUFile *f)
>      qemu_savevm_command_send(f, MIG_CMD_OPEN_RETURN_PATH, 0, NULL);
>  }
>  
> +/* We have a buffer of data to send; we don't want that all to be loaded
> + * by the command itself, so the command contains just the length of the
> + * extra buffer that we then send straight after it.
> + * TODO: Must be a better way to organise that
> + */
> +void qemu_savevm_send_packaged(QEMUFile *f, const QEMUSizedBuffer *qsb)
> +{
> +    size_t cur_iov;
> +    size_t len = qsb_get_length(qsb);
> +    uint32_t tmp;
> +
> +    tmp = cpu_to_be32(len);
> +
> +    trace_qemu_savevm_send_packaged();
> +    qemu_savevm_command_send(f, MIG_CMD_PACKAGED, 4, (uint8_t *)&tmp);
> +
> +    /* all the data follows (concatinating the iov's) */
> +    for (cur_iov = 0; cur_iov < qsb->n_iov; cur_iov++) {
> +        /* The iov entries are partially filled */
> +        size_t towrite = (qsb->iov[cur_iov].iov_len > len) ?
> +                              len :
> +                              qsb->iov[cur_iov].iov_len;
> +        len -= towrite;
> +
> +        if (!towrite) {
> +            break;
> +        }
> +
> +        qemu_put_buffer(f, qsb->iov[cur_iov].iov_base, towrite);
> +    }
> +}
> +
>  /* Send prior to any postcopy transfer */
>  void qemu_savevm_send_postcopy_advise(QEMUFile *f)
>  {
> @@ -1265,6 +1297,48 @@ static int 
> loadvm_process_command_simple_lencheck(const char *name,
>      return 0;
>  }
>  
> +/* Immediately following this command is a blob of data containing an 
> embedded
> + * chunk of migration stream; read it and load it.
> + */
> +static int loadvm_handle_cmd_packaged(MigrationIncomingState *mis,
> +                                      uint32_t length)
> +{
> +    int ret;
> +    uint8_t *buffer;
> +    QEMUSizedBuffer *qsb;
> +
> +    trace_loadvm_handle_cmd_packaged(length);
> +
> +    if (length > MAX_VM_CMD_PACKAGED_SIZE) {
> +        error_report("Unreasonably large packaged state: %u", length);
> +        return -1;

It would be a good idea to check this on the send side as well as
receive, wouldn't it?

> +    }
> +    buffer = g_malloc0(length);
> +    ret = qemu_get_buffer(mis->file, buffer, (int)length);
> +    if (ret != length) {
> +        g_free(buffer);
> +        error_report("CMD_PACKAGED: Buffer receive fail ret=%d length=%d\n",
> +                ret, length);
> +        return (re/t < 0) ? ret : -EAGAIN;
> +    }
> +    trace_loadvm_handle_cmd_packaged_received(ret);
> +
> +    /* Setup a dummy QEMUFile that actually reads from the buffer */
> +    qsb = qsb_create(buffer, length);
> +    g_free(buffer); /* Because qsb_create copies */
> +    if (!qsb) {
> +        error_report("Unable to create qsb");
> +    }
> +    QEMUFile *packf = qemu_bufopen("r", qsb);
> +
> +    ret = qemu_loadvm_state_main(packf, mis);
> +    trace_loadvm_handle_cmd_packaged_main(ret);
> +    qemu_fclose(packf);
> +    qsb_free(qsb);
> +
> +    return ret;
> +}
> +
>  /*
>   * Process an incoming 'QEMU_VM_COMMAND'
>   * negative return on error (will issue error message)
> @@ -1315,6 +1389,14 @@ static int loadvm_process_command(QEMUFile *f)
>          migrate_send_rp_pong(mis, tmp32);
>          break;
>  
> +    case MIG_CMD_PACKAGED:
> +        if (loadvm_process_command_simple_lencheck("CMD_POSTCOPY_PACKAGED",
> +            len, 4)) {
> +            return -1;
> +         }
> +        tmp32 = qemu_get_be32(f);
> +        return loadvm_handle_cmd_packaged(mis, tmp32);
> +
>      case MIG_CMD_POSTCOPY_ADVISE:
>          if (loadvm_process_command_simple_lencheck("CMD_POSTCOPY_ADVISE",
>                                                     len, 16)) {
> diff --git a/trace-events b/trace-events
> index 050f553..cbf995c 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -1171,6 +1171,10 @@ qemu_loadvm_state_main(void) ""
>  qemu_loadvm_state_main_quit_parent(void) ""
>  qemu_loadvm_state_post_main(int ret) "%d"
>  qemu_loadvm_state_section_startfull(uint32_t section_id, const char *idstr, 
> uint32_t instance_id, uint32_t version_id) "%u(%s) %u %u"
> +qemu_savevm_send_packaged(void) ""
> +loadvm_handle_cmd_packaged(unsigned int length) "%u"
> +loadvm_handle_cmd_packaged_main(int ret) "%d"
> +loadvm_handle_cmd_packaged_received(int ret) "%d"
>  loadvm_postcopy_handle_advise(void) ""
>  loadvm_postcopy_handle_end(void) ""
>  loadvm_postcopy_handle_listen(void) ""

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

pgp2QBZpBZghc.pgp
Description: PGP signature