Re: [PATCH RFC XEN v1 11/14] tools: migrate: refactor selection of save/restore ops to be arch specific

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 09/12/15 14:32, Ian Campbell wrote:
> I wasn't sure of the best way to achieve this, but a pair of per-arch
> hooks seemed to be preferable to ifdeffery.

I agree.  The patch looks good.

>
> I also wasn't sure about the change to guest_type for save. The
> restore half of the ctxt already has such a field but since the save
> side treats it as an input to the process as opposed to the restore
> side which determines it from the stream it seemed like keeping them
> separate was best.

I would recommend introducing a const uint32_t field in xc_sr_save_ops,
to make it harder to accidentally get the guest type out of sync with
the ops in use.

The save/restore difference is because of the asymetry in their use. 
The save side needs to select the correct one to used, based on the
domid, while the restore side gets one in the migration stream and has
to verify it against the domain it is restoring into.

>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: andyhhp

I don't think that is quite enough routing information to find me ;)

> ---
>  tools/libxc/xc_sr_common.h     | 15 +++++++++++++++
>  tools/libxc/xc_sr_common_x86.c | 22 ++++++++++++++++++++++
>  tools/libxc/xc_sr_restore.c    | 15 +++------------
>  tools/libxc/xc_sr_save.c       | 22 +++++++---------------
>  4 files changed, 47 insertions(+), 27 deletions(-)
>
> diff --git a/tools/libxc/xc_sr_common.h b/tools/libxc/xc_sr_common.h
> index 64f6082..0d36c8d 100644
> --- a/tools/libxc/xc_sr_common.h
> +++ b/tools/libxc/xc_sr_common.h
> @@ -174,6 +174,9 @@ struct xc_sr_context
>              struct xc_sr_save_ops ops;
>              struct save_callbacks *callbacks;
>  
> +            /* For Domain Header */
> +            uint32_t guest_type;
> +
>              /* Live migrate vs non live suspend. */
>              bool live;
>  
> @@ -317,9 +320,21 @@ struct xc_sr_context
>  
>  extern struct xc_sr_save_ops save_ops_x86_pv;
>  extern struct xc_sr_save_ops save_ops_x86_hvm;
> +extern struct xc_sr_save_ops save_ops_arm;
>  
>  extern struct xc_sr_restore_ops restore_ops_x86_pv;
>  extern struct xc_sr_restore_ops restore_ops_x86_hvm;
> +extern struct xc_sr_restore_ops restore_ops_arm;

These two externs look like they want to be in patch 13.

~Andrew