Re: [PATCH v5 1/5] xen/common: introduce a new framework for save/restore of 'domain' context

[Jan Beulich <jbeulich@suse.com>]

On 21.05.2020 18:19, Paul Durrant wrote:
> To allow enlightened HVM guests (i.e. those that have PV drivers) to be
> migrated without their co-operation it will be necessary to transfer 'PV'
> state such as event channel state, grant entry state, etc.
> 
> Currently there is a framework (entered via the hvm_save/load() functions)
> that allows a domain's 'HVM' (architectural) state to be transferred but
> 'PV' state is also common with pure PV guests and so this framework is not
> really suitable.
> 
> This patch adds the new public header and low level implementation of a new
> common framework, entered via the domain_save/load() functions. Subsequent
> patches will introduce other parts of the framework, and code that will
> make use of it within the current version of the libxc migration stream.
> 
> This patch also marks the HVM-only framework as deprecated in favour of the
> new framework.
> 
> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
> Acked-by: Julien Grall <julien@xen.org>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
with one remark:

> +int domain_load_end(struct domain_context *c)
> +{
> +    struct domain *d = c->domain;
> +    size_t len = c->desc.length - c->len;
> +
> +    while ( c->len != c->desc.length ) /* unconsumed data or pad */
> +    {
> +        uint8_t pad;
> +        int rc = domain_load_data(c, &pad, sizeof(pad));
> +
> +        if ( rc )
> +            return rc;
> +
> +        if ( pad )
> +            return -EINVAL;
> +    }
> +
> +    gdprintk(XENLOG_INFO, "%pd load: %s[%u] +%zu (-%zu)\n", d, c->name,
> +             c->desc.instance, c->len, len);

Unlike on the save side you assume c->name to be non-NULL here.
We're not going to crash because of this, but it feels a little
odd anyway, specifically with the function being non-static
(albeit on the positive side we have the type being private to
this file).

Jan