Re: [V3 PATCH 0/9]: PVH dom0....

[George Dunlap <george.dunlap@eu.citrix.com>]

On 11/27/2013 02:27 AM, Mukesh Rathor wrote:
> Hi,
>
> V3:
>    - New patch #7 for xsm changes to add_to_physmap.
>    - Patches 3, 4 5 6 and 8 are unchanged from V2. (just a new comment in #8).
>
> These patches implement PVH dom0. Please note there is 1 fixme in
> this entire series that is being discussed under a different thread,
> and will be worked on next. PVH dom0 creation is disabled
> until then.

So a couple of thoughts from a release perspective.

Releasing *code* as "experimental" means, "it may work or it may not; 
use at your own risk".  If people use it and it works, then great; they 
can expect that the code will only get better.

However, releasing an *interface* as "experimental" means, "it may work 
for you now, but it may not work later when we change the interface".  
While this is nice in theory, in practice, once something works, people 
may begin to rely on it and we may end up having to support it anyway.  
So the Linux interface cannot really be labelled "experimental"; we have 
to be reasonably certain that we can support it going forward.

Benefits:

We have a fairly solid precedent for releasing features as 
"experimental" or "tech preview".  This allows a much wider testing and 
feedback.  If it turns out to be robust enough, people may even be able 
to use it, gaining the potential performance advantages.

Someone could make an argument the other way: that the best thing to do 
would be to check it in at the beginning of the release cycle, get it 
well tested, and then release it as "ready" for 4.5, without going 
through an "experimental" phase.  Both arguments have their merits; but 
since current way we do things hasn't caused any problems and seems to 
be working OK, it seems best to follow precedent, and assume that a tech 
preview will be beneficial.

Risks, bugs:

All of the actual functional changes in this series are predicated on 
"if(is_pvh_domain())", so in theory they should only have an effect on 
PVH guests.  (There is, of course, a small risk that there will be a 
mistake here.)  It introduces a new p2m type, but since it is the only 
one that uses it, bugs should only affect PVH, and not other functionality.

Risks, interface:

This patch series only adds two things to the interface with Linux: 
XENMAPSPACE_gmfn_foreign and XENMEM_add_to_physmap_range.
These are already used and available in the ARM side.

Normally I'd be afraid of accepting new interfaces at this stage in the 
game, as I'd be afraid that we hadn't had enough time to make sure it's 
something we want to support going forward.  However, since this is just 
duplicating an interface already in use on the ARM side, I think the 
interface *has* been thought of for some time.  This makes is much more 
likely to be worth the risk; if the ARM side has used it for 6 months 
without finding a problem with it, it seems unlikely that the x86 side 
will be particularly different.

So on the whole, there is a benefit (if a bit nebulous) to having it in, 
and a reasonably low risk; and it's not clear that the risk will be 
significantly mitigated by waiting another 6 months.  I'm therefore 
inclined to give it a release ack.

Any thoughts?

  -George