Re: [PATCH 09/29] [HACK] tools/libxc: save/restore v2 framework

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Tue, Sep 16, 2014 at 12:44:45PM +0100, Andrew Cooper wrote:
> 
> On 15/09/2014 19:58, Konrad Rzeszutek Wilk wrote:
> >On Mon, Sep 15, 2014 at 04:09:51PM +0100, Andrew Cooper wrote:
> >>On 14/09/2014 11:23, Shriram Rajagopalan wrote:
> >>>On Sep 11, 2014 4:08 AM, "Andrew Cooper" <andrew.cooper3@citrix.com
> >>><mailto:andrew.cooper3@citrix.com>> wrote:
> >>>>On 11/09/14 12:01, Ian Campbell wrote:
> >>>>>On Thu, 2014-09-11 at 11:37 +0100, Andrew Cooper wrote:
> >>>>>>On 11/09/14 11:34, Ian Campbell wrote:
> >>>>>>>On Wed, 2014-09-10 at 18:10 +0100, Andrew Cooper wrote:
> >>>>>>>>For testing purposes, the environmental variable
> >>>"XG_MIGRATION_V2" allows the
> >>>>>>>>two save/restore codepaths to coexist, and have a runtime switch.
> >>>>>>>>
> >>>>>>>>It is indended that once this series is less RFC, the v2
> >>>framework will
> >>>>>>>>completely replace v1.
> >>>>>>>I think we are now at the point where this hack needs to be
> >>>dropped from
> >>>>>>>the series.
> >>>>>>One problem is remus.  My plan when dropping this patch was to
> >The other is 'tmem'. But 'tmem' has not yet been declared 'baked' so
> >not making it work from a release perspective is OK.
> >
> >With the 'tmem' maintainer hat on, however I would like to it work without
> >having to do anything :-) Which reminds me I need to follow up
> >on double-checking the migation hasn't bitrotten!
> 
> While reverse engineering the existing protocol is not too difficult, I
> think the TMEM migration needs redesigning.  From memory, there is a huge
> quantity of metadata which is sent redundantly (tmem pool uuid with every
> frame).  It would also benefit massively from some batching to help reduce
> the quantity of hypercalls made (5 per frame iirc).

Ugh.
.. snip..
> >>>before a feature freeze, while the rest of the series has still not
> >>>been reviewed at all for the past 3 months.
> >What is the dependency on "full remus" support? Is there a list of
> >all the different patchset that need to be reviewed?
> 
> As with TMEM, remus support needs redesigning, as it needs coordinated
> additions to both the libxc and libxl stream formats to support checkpoints
> without the current layer violations.
> 
> >
> >>I can appreciate your frustration on this point, and do not envy your
> >>position.
> >>
> >>The concern I have is that XenServer 6.5 is shipping with migrationv2 as
> >>we absolutely need it, given the 32->64bit upgrade.  We were hoping to
> >>get the new format committed in 4.5 to guarantee stability, but that is
> >>looking increasingly unlikely to happen.  As a result, it will probably
> >>have to go in early in 4.6, with extra care taken to ensure that no
> >>incompatible changes are made as a result of further review.
> >Could you tell me what are the benefits of having a v1 to v2 runtime
> >switch for developers/users besides the obvious (faster migration,
> >easier to understand code)?
> 
> Users should not notice a difference, other than it being faster.
> 
> From a developer point of view,
> 
> * It actually has some header information now
> * It is independent of the bitness of the toolstack (which is the key reason
> we needed to do it for XenServers switch from 32 to 64bit dom0)
> * The old format (little that it was) was basically inextensible for PV
> guests (See the PV MSRs thread)
> * It has allowed for dropping 2-level PV guest support, as well as other
> 32bit Xen bits.

The disadvantages are that:
 - Breaks tmem migration.
 - Breaks outside users of libxc (granted we don't specify an API for
   that, but I am not a huge fan of putting barriers).

> 
> >
> >For me it sounded that this would allow the community to also
> >test it and report bugs - which would be invaluable. And better
> >yet there is a env flag to swap between a baseline and new
> >code to ease the testing.
> 
> That was only supposed to be development, and removed when committed
> upstream.
> 
> ~Andrew
> 
> >
> >The risks seem quite contained - if something goes awry, folks can
> >use the v1 version - which should have the same amount of bugs
> >that it had in previous releases. And since it is on by default - so
> >only dedicated users would turn v2 on.
> >
> > From an maintaince perspective, it does add more code but then once
> >feature freeze hits we do not pay attention to features anymore,
> >but rather to bug-fixes.
> >
> >Hm, Ian's - what are you folks take on it?
>