On 24/09/2020 14:10, Paul Durrant wrote:
> From: Paul Durrant <pdurr...@amazon.com>
>
> ... and bump the version.
>
> This patch implements version 4 of the migration stream by adding the code
> necessary to save and restore DOMAIN_CONTEXT records, and removing the code
> to save the SHARED_INFO and TSC_INFO records (as these are deprecated in
> version 4).
>
> Signed-off-by: Paul Durrant <pdurr...@amazon.com>
This really needs to be at least 3 patches.
First to adjust tools/python/scripts/verify-stream-v2 to understand the
new changes in the stream.
My testing tends to include running the script over the result of `xl
save`, and using a script in place of libxl-save-helper which tee's the
stream through the script, which lets you test in-line migrate. (I
wonder if it would be better to add a pass-through mode to the script
and give libxl a way of running it in the same way as it currently runs
covert-legacy-stream.)
Next, a patch updating the receive side only to understand the new
changes in the stream. In particular, this makes it far easier to
confirm that backwards compatibility is maintained.
Finally, a patch updating the sending side, if applicable. (I've got an
alternative suggestion to avoid burning a load of major version numbers,
but will follow up on a different patch with that).
> ---
> Cc: Ian Jackson <ian.jack...@eu.citrix.com>
> Cc: Wei Liu <w...@xen.org>
>
> v7:
> - New in v7
> ---
> tools/libs/guest/xg_sr_common.h | 3 ++
> tools/libs/guest/xg_sr_common_x86.c | 20 -----------
> tools/libs/guest/xg_sr_common_x86.h | 6 ----
> tools/libs/guest/xg_sr_restore.c | 45 +++++++++++++++++++++--
> tools/libs/guest/xg_sr_save.c | 52 ++++++++++++++++++++++++++-
> tools/libs/guest/xg_sr_save_x86_hvm.c | 5 ---
> tools/libs/guest/xg_sr_save_x86_pv.c | 22 ------------
> 7 files changed, 97 insertions(+), 56 deletions(-)
>
> diff --git a/tools/libs/guest/xg_sr_common.h b/tools/libs/guest/xg_sr_common.h
> index 13fcc47420..d440281cc1 100644
> --- a/tools/libs/guest/xg_sr_common.h
> +++ b/tools/libs/guest/xg_sr_common.h
> @@ -298,6 +298,9 @@ struct xc_sr_context
>
> /* Sender has invoked verify mode on the stream. */
> bool verify;
> +
> + /* Domain context blob. */
> + struct xc_sr_blob context;
We already have
ctx->x86.hvm.restore.context
and are now gaining
ctx->restore.context
This is concerning close to being ambiguous. How about dom_context ?
Also, you leak the memory allocation. Free it in xg_sr_restore.c:cleanup().
> diff --git a/tools/libs/guest/xg_sr_restore.c
> b/tools/libs/guest/xg_sr_restore.c
> index b57a787519..453a383ba4 100644
> --- a/tools/libs/guest/xg_sr_restore.c
> +++ b/tools/libs/guest/xg_sr_restore.c
> @@ -529,6 +529,20 @@ static int send_checkpoint_dirty_pfn_list(struct
> xc_sr_context *ctx)
> return rc;
> }
>
> +static int stream_complete(struct xc_sr_context *ctx)
> +{
> + xc_interface *xch = ctx->xch;
> + int rc;
> +
> + rc = xc_domain_setcontext(xch, ctx->domid,
> + ctx->restore.context.ptr,
> + ctx->restore.context.size);
> + if ( rc < 0 )
> + PERROR("Unable to restore domain context");
> +
> + return rc;
> +}
Please put this in the PV and HVM stream_complete() hooks.
This is somewhat of a layering violation and enforcing an order which
might not be appropriate in all cases.
~Andrew
