Re: [PATCH 19/27] tools/libxc+libxl+xl: Restore v2 streams

[Ian Campbell <ian.campbell@citrix.com>]

On Mon, 2015-06-15 at 14:44 +0100, Andrew Cooper wrote:
> @@ -377,6 +384,28 @@ static void record_body_done(libxl__egc *egc,
>      stream_failed(egc, stream, ret);
>  }
>  
> +void libxl__xc_domain_restore_done(libxl__egc *egc, void *dcs_void,
> +                                   int ret, int retval, int errnoval)
> +{
> +    libxl__domain_create_state *dcs = dcs_void;
> +    STATE_AO_GC(dcs->ao);
> +
> +    if (ret)
> +        goto err;
> +
> +    if (retval) {
> +        LOGEV(ERROR, errnoval, "restoring domain");
> +        ret = ERROR_FAIL;
> +        goto err;
> +    }
> +
> +    libxl__stream_read_continue(egc, &dcs->srs);

continue? Is this something to do with checkpointing?

> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 23f27d4..7418d92 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -346,6 +346,8 @@ libxl_domain_create_info = Struct("domain_create_info",[
>  
>  libxl_domain_restore_params = Struct("domain_restore_params", [

At some point we will need a LIBXL_HAVE #define.

>      ("checkpointed_stream", integer),
> +    ("stream_version", uint32, {'init_val': '1'}),

If we aren't going to go for an IDL enum rather than a uint32 you
probably just want the bare integer 1.

But, I suspect we would prefer an enum, i.e an explicit list of known
versions, rather than an integer?

I wonder when, if ever, we will be able to flip this to 2? I suppose
whenever the legacy conversion stuff gets pulled out.

> +    ("legacy_width", uint32),

>From what I've seen so far this is never user provided but is internal
to libxl and detected[0] at runtime. As such it belongs somewhere else
other than in the public API.

[0] FVO "detected" == "hardcoded depending on the build arch"

>      ])
>  
>  libxl_domain_sched_params = Struct("domain_sched_params",[
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index ddb293c..14d96c9 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -110,7 +110,9 @@
>  
>  #define XL_MANDATORY_FLAG_JSON (1U << 0) /* config data is in JSON format */
>  #define XL_MANDATORY_FLAG_STREAMv2 (1U << 1) /* stream is v2 */
> -#define XL_MANDATORY_FLAG_ALL  (XL_MANDATORY_FLAG_JSON)
> +#define XL_MANDATORY_FLAG_ALL  (XL_MANDATORY_FLAG_JSON |        \
> +                                XL_MANDATORY_FLAG_STREAMv2)
> +
>  struct save_file_header {
>      char magic[32]; /* savefileheader_magic */
>      /* All uint32_ts are in domain's byte order. */
> @@ -2724,6 +2726,9 @@ static uint32_t create_domain(struct domain_create *dom_info)
>          libxl_domain_restore_params_init(&params);
>  
>          params.checkpointed_stream = dom_info->checkpointed_stream;
> +        params.stream_version =
> +            (hdr.mandatory_flags & XL_MANDATORY_FLAG_STREAMv2) ? 2 : 1;
> +
>          ret = libxl_domain_create_restore(ctx, &d_config,
>                                            &domid, restore_fd,
>                                            &params,