Hi Paul,
On 06/04/2020 09:27, Paul Durrant wrote:
-----Original Message-----
From: Julien Grall <jul...@xen.org>
Sent: 03 April 2020 18:24
To: p...@xen.org; xen-devel@lists.xenproject.org
Cc: 'Andrew Cooper' <andrew.coop...@citrix.com>; 'George Dunlap'
<george.dun...@citrix.com>; 'Ian
Jackson' <ian.jack...@eu.citrix.com>; 'Jan Beulich' <jbeul...@suse.com>;
'Stefano Stabellini'
<sstabell...@kernel.org>; 'Wei Liu' <w...@xen.org>
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.
And I agree, hence why I say "long term" :).
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.
Code churn is always going to necessary one day or another if we want to
consolidate the two.
Having two frameworks is not without risks. There are a few unknown to
be answered:
* Is there any dependency between the two? If yes, what is the ordering?
* The name of the hypercall does not say anything about "PV". So a
contributor could think it would be fine to save/restore new HVM state
in the new generic hypercall. Is it going to be an issue? If so, how do
we prevent it?
* Are we going to deprecate the existing framework?
I am not suggesting we should not go with two frameworks, but the
reasons and implications are not clear to me. Hence, why I think the
commit message should be expanded with some rationale.
Cheers,
--
Julien Grall
