Re: [PATCH v10 10/11] tools/libs/guest: add code to restore a v4 libxc stream

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 08/10/2020 19:57, Paul Durrant wrote:
> From: Paul Durrant <pdurrant@xxxxxxxxxx>
>
> This patch adds the necessary code to accept a v4 stream, and to recognise and
> restore a REC_TYPE_DOMAIN_CONTEXT record.
>
> Signed-off-by: Paul Durrant <pdurrant@xxxxxxxxxx>

Somewhere within this needs to be logic to reject the forbidden records
in relevant stream versions.

> diff --git a/tools/libs/guest/xg_sr_restore_x86_hvm.c 
> b/tools/libs/guest/xg_sr_restore_x86_hvm.c
> index d6ea6f3012..6bb164b9f0 100644
> --- a/tools/libs/guest/xg_sr_restore_x86_hvm.c
> +++ b/tools/libs/guest/xg_sr_restore_x86_hvm.c
> @@ -225,6 +225,15 @@ static int x86_hvm_stream_complete(struct xc_sr_context 
> *ctx)
>          return rc;
>      }
>  
> +    rc = xc_domain_set_context(xch, ctx->domid,
> +                               ctx->restore.dom_ctx.ptr,
> +                               ctx->restore.dom_ctx.size);
> +    if ( rc )
> +    {
> +        PERROR("Unable to restore Domain context");
> +        return rc;
> +    }

This doesn't match where you specified the record to live in the stream,
and in particular is reordered WRT HVMCONTEXT restoration.

Also, it appears to be in the middle of a block of code which needs to
become `if ( guest-aware )`.

> +
>      rc = xc_dom_gnttab_seed(xch, ctx->domid, true,
>                              ctx->restore.console_gfn,
>                              ctx->restore.xenstore_gfn,
> diff --git a/tools/libs/guest/xg_sr_restore_x86_pv.c 
> b/tools/libs/guest/xg_sr_restore_x86_pv.c
> index dc50b0f5a8..2dafad7b83 100644
> --- a/tools/libs/guest/xg_sr_restore_x86_pv.c
> +++ b/tools/libs/guest/xg_sr_restore_x86_pv.c
> @@ -1134,6 +1134,15 @@ static int x86_pv_stream_complete(struct xc_sr_context 
> *ctx)
>      if ( rc )
>          return rc;
>  
> +    rc = xc_domain_set_context(xch, ctx->domid,
> +                               ctx->restore.dom_ctx.ptr,
> +                               ctx->restore.dom_ctx.size);
> +    if ( rc )
> +    {
> +        PERROR("Unable to restore Domain context");
> +        return rc;
> +    }

Similar comment as HVM for the reordering.  PV guests in particular tend
to be far more sensitive to the restoration order.

~Andrew