[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • From: Ian Jackson <ian.jackson@xxxxxxxxxx>
  • Date: Tue, 14 Jan 2020 17:02:20 +0000
  • Authentication-results: esa5.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none; spf=None smtp.pra=ian.jackson@xxxxxxxxxx; spf=Pass smtp.mailfrom=Ian.Jackson@xxxxxxxxxx; spf=None smtp.helo=postmaster@xxxxxxxxxxxxxxx
  • Cc: Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Ian Jackson <Ian.Jackson@xxxxxxxxxx>
  • Delivery-date: Tue, 14 Jan 2020 17:02:33 +0000
  • Ironport-sdr: 8zUJUmAZIONNLLP4QjqfuOZjyiL/WRe4NIHg6/edmL4yIjVMOHkfqpcnX/LH+jDyuF7+SP1Z9H /ZOXU8W2OuOLjPM/dGoNFH2QsfHp4yB6RLATlTn2QpYO+762TYSFrkAKW2MUhJXLlucc6BOUHh BiLPOoO6KOzYEyJJDmKimtvzdSkcpnK5/S4Djd2UqZASw7A09kTIbPaBLyMsgNt9ccug51G8Ta RBmhAXggwl2tLpMnaeK18NPFuKe7ZynGSOolVd5TClNWJTHr93RwRn4i6lr4RJsuexxSUiH0GC m7s=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.