Re: [Xen-devel] [PATCH 0/6] [VERY RFC] Migration Stream v2

[Frediano Ziglio <frediano.ziglio@xxxxxxxxxx>]

On Thu, 2014-04-10 at 12:21 +0100, Andrew Cooper wrote:
> On 10/04/14 11:42, Ian Campbell wrote:
> > On Wed, 2014-04-09 at 19:28 +0100, Andrew Cooper wrote:
....
> 
> >
> >> The error handling is known to only semi-consistent.  Functions return 0 
> >> for
> >> success and non-zero for failure.  This is typically -1, although errno is 
> >> not
> >> always relevant.  However, the logging messages should all be relevant and
> >> correct.  Making this properly consistent will involve wider effort across 
> >> all
> >> of libxc.
> > It would be useful if the new code was correct at least so far as its
> > own behaviour went (meaning no need to fix functions it calls as part of
> > this).
> 
> libxc is too broken for that to be possible, (including such gems as the
> save_callbacks functions which is not specified as to how to indicate
> success or error, and have developed at least 3 different flavours)
> 
> Currently, the state of play is "if you get non0, something went wrong. 
> Please read the log for relevant information"  Once we get a libxc_err_t
> (or so, given a discussion down the pub) capable of expressing more
> meaningful error problems, most codepaths (including these new ones)
> will need updating, although starting from a fairly-consistent position
> will be much easier than not.
> 

I agree with Ian, we should have a first patch that just replace
xc_domain_save/xc_domain_restore. We can fix functions return and error
later in another set of patches.

> >
> >> An area needing discussing is how to do v1 -> v2 transformations for a 
> >> one-time
> >> upgrade.  There is a (very basic currently) python script which can pick a 
> >> v1
> >> stream, and a separate python library to write v2 streams.
> >>
> >> One option would be to combine these two into a program which takes two 
> >> fds,
> >> which libxc can exec() out to.  There is deliberate flexibility in the v2
> >> restore code which allows a v1 -> v2 transformation on a stream without 
> >> seeking.
> > forking/execing in libxc might be problematic, fitting it into libxl
> > might be easier, since it has infrastructure for that sort of thing.
> 
> libxl is not the only user of libxc, and fixing it there would not help
> the other consumers of libxc.
> 
> Furthermore, unless the consumer has an out-of-band detection method
> (libxl can easily be made to have, Xapi less so easy, and no idea about
> other consumers), xc_domain_restore() is the first piece of code capable
> of detecting a legacy stream without needing to seek.
> 
> >
> > Or maybe the fact that most of this already happens in a process which
> > libxl spawns for that purpose means that libxc can safely fork because
> > the application in that case is under our control.
> 
> Exactly the same for Xapi, which uses a separate process which functions
> similarly to xc_save/restore but does domain build as well, which is why
> I am hoping this is an acceptable way of fixing the problem.
> 

IMO we should just replace xc_domain_restore. xc_domain_restore is
executed in a the helper process which can fork easily a process if
needed. The idea is that xc_domain_restore read first bytes of the
stream (4 is enough) and if not valid it possibly fork and call python
code (or whatever) passing handle and bytes read and getting a new
handle with converted data. If you think probably we'll call fork before
any libxc function.

Another consideration about these patches should be file names and code
split thinking about ARM migration too. Too many functions are in x86
specific files. For instance xc_domain_restore2 (in restore.c) should
call a restore_arch_pv instead of a restore_x86_pv.

Frediano



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel