[Xen-devel] [PATCH 08/12] libxc/restore: Support v3 streams, and cope with v2 compatibilty

[Ian Jackson <ian.jackson@xxxxxxxxxx>]

Andrew Cooper writes ("[PATCH 08/12] libxc/restore: Support v3 streams, and 
cope with v2 compatibilty"):
> Introduce a static_data_complete() hook which is called when a
> STATIC_DATA_END record is found (v3) or inferred (v2).
> 
> Modify handle_page_data() and handle_x86_pv_p2m_frames() to infer the
> position in v2 streams.
> 
> The implementation of x86_static_data_complete() needs to wait until
> more plumbing is in place, to make a combined libxl/libxc change to
> maintain (functional) bisectability.
> 
> No practical change to behaviour - this is all plumbing work.

These parts are confusing to me:

> +    /*
> +     * This is a bit of a bodge, but it is less bad than duplicating
> +     * handle_page_data() between different architectures.
> +     */
> +#if defined(__i386__) || defined(__x86_64__)
> +    /* v2 compat.  Infer the position of STATIC_DATA_END. */
> +    if ( ctx->restore.format_version < 3 && 
> !ctx->restore.seen_static_data_end )
> +    {
> +        rc = handle_static_data_end(ctx);
> +        if ( rc )
> +        {
> +            ERROR("Inferred STATIC_DATA_END record failed");
> +            goto err;
> +        }
> +        rc = -1;
> +    }
> +
> +    if ( !ctx->restore.seen_static_data_end )
> +    {
> +        ERROR("No STATIC_DATA_END seen");
> +        goto err;
> +    }
> +#endif

...

> +    /* v2 compat.  Infer the position of STATIC_DATA_END. */
> +    if ( ctx->restore.format_version < 3 && 
> !ctx->restore.seen_static_data_end )
> +    {
> +        rc = handle_static_data_end(ctx);
> +        if ( rc )
> +        {
> +            ERROR("Inferred STATIC_DATA_END record failed");
> +            return rc;
> +        }
> +    }
> +
> +    if ( !ctx->restore.seen_static_data_end )
> +    {
> +        ERROR("No STATIC_DATA_END seen");
> +        return -1;
> +    }

Firstly, this code is remarkably similar.  Surely it should be
factored out into something like
  possible_implicit_static_data_end()
?

And secondly, I don't understand why the first part in
handle_page_data is arch-qualified.  Maybe this would make more sense
if I looked at the code in context rather than just the diff ?

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel