Re: [PATCH v10 11/11] tools/libs/guest: add code to save 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 save a REC_TYPE_DOMAIN_CONTEXT record,
> and stop saving the now obsolete REC_TYPE_SHARED_INFO and REC_TYPE_TSC_INFO
> records for PV guests.
>
> Signed-off-by: Paul Durrant <pdurrant@xxxxxxxxxx>

Looks broadly ok.

> diff --git a/tools/libs/guest/xg_sr_common_x86.h 
> b/tools/libs/guest/xg_sr_common_x86.h
> index b55758c96d..e504169705 100644
> --- a/tools/libs/guest/xg_sr_common_x86.h
> +++ b/tools/libs/guest/xg_sr_common_x86.h
> @@ -44,6 +44,52 @@ static int write_headers(struct xc_sr_context *ctx, 
> uint16_t guest_type)
>      return 0;
>  }
>  
> +/*
> + * Writes a DOMAIN_CONTEXT record into the stream.
> + */
> +static int write_domain_context_record(struct xc_sr_context *ctx)
> +{
> +    xc_interface *xch = ctx->xch;
> +    struct xc_sr_record rec = {
> +        .type = REC_TYPE_DOMAIN_CONTEXT,
> +    };
> +    size_t len = 0;
> +    int rc;
> +
> +    rc = xc_domain_get_context(xch, ctx->domid, NULL, &len);
> +    if ( rc < 0 )
> +    {
> +        PERROR("can't get record length for dom %u\n", ctx->domid);
> +        goto out;
> +    }
> +
> +    rec.data = malloc(len);
> +
> +    rc = -1;
> +    if ( !rec.data )
> +    {
> +        PERROR("can't allocate %lu bytes\n", len);

%zu, because not all versions of C have size_t the same as unsigned long.

> +        goto out;
> +    }
> +
> +    rc = xc_domain_get_context(xch, ctx->domid, rec.data, &len);
> +    if ( rc < 0 )
> +    {
> +        PERROR("can't get domain record for dom %u\n", ctx->domid);

"domain context", and above.

> diff --git a/tools/libs/guest/xg_sr_save_x86_pv.c 
> b/tools/libs/guest/xg_sr_save_x86_pv.c
> index 4964f1f7b8..3de7b19f54 100644
> --- a/tools/libs/guest/xg_sr_save_x86_pv.c
> +++ b/tools/libs/guest/xg_sr_save_x86_pv.c
> @@ -849,20 +849,6 @@ static int write_x86_pv_p2m_frames(struct xc_sr_context 
> *ctx)
>      return rc;
>  }
>  
> -/*
> - * Writes an SHARED_INFO record into the stream.
> - */
> -static int write_shared_info(struct xc_sr_context *ctx)
> -{
> -    struct xc_sr_record rec = {
> -        .type = REC_TYPE_SHARED_INFO,
> -        .length = PAGE_SIZE,
> -        .data = ctx->x86.pv.shinfo,
> -    };
> -
> -    return write_record(ctx, &rec);
> -}

This change also wants to strip out ctx->x86.pv.shinfo, and the mapping
logic.

~Andrew