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

[Paul Durrant <xadimgnik@xxxxxxxxx>]

> -----Original Message-----
> From: Julien Grall <julien@xxxxxxx>
> Sent: 03 April 2020 18:24
> To: paul@xxxxxxx; xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: 'Andrew Cooper' <andrew.cooper3@xxxxxxxxxx>; 'George Dunlap' 
> <george.dunlap@xxxxxxxxxx>; 'Ian
> Jackson' <ian.jackson@xxxxxxxxxxxxx>; 'Jan Beulich' <jbeulich@xxxxxxxx>; 
> 'Stefano Stabellini'
> <sstabellini@xxxxxxxxxx>; 'Wei Liu' <wl@xxxxxxx>
> Subject: Re: [PATCH 1/5] xen/common: introduce a new framework for 
> save/restore of 'domain' context
> 
> Hi Paul,
> 
> On 03/04/2020 16:55, Paul Durrant wrote:
> >> -----Original Message-----
> > [snip]
> >>> +
> >>> +#include <xen/save.h>
> >>> +
> >>> +struct domain_context {
> >>> +    bool log;
> >>> +    struct domain_save_descriptor desc;
> >>> +    domain_copy_entry copy;
> >>
> >> As your new framework is technically an extension of existing one, it
> >> would be good to explain why we diverge in the definitions.
> >>
> >
> > I don't follow. What is diverging? I explain in the commit comment that 
> > this is a parallel
> framework. Do I need to justify why it is not a carbon copy of the HVM one?
> 
> Well, they are both restoring/saving guest state. The only difference is
> the existing one is focusing on HVM state.
> 
> So it would make sense long term to have only one hypercall and tell
> what you want to save. In fact, some of the improvement here would
> definitely make the HVM one nicer to use (at least in the context of LU).
> 

I guess we could move the HVM save records over to the new framework, but it 
works for the moment so I don't want to bring it into scope now.

>  From the commit message, it is not clear to me why a new framework and
> why the infrastructure is at the same time different but not.
> 

An alternative would be to move the HVM save code into common code and then try 
to adapt it. I think that would result in more code churn and ultimately be 
harder to review. The extra infrastructure introduced here is fairly minimal 
and, for the moment, only targeting PV state. As I said above there's nothing 
stopping the HVM records being ported over later once any initial issues have 
been shaken out.

  Paul