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

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 10/04/14 14:05, Frediano Ziglio wrote:
> 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.

Which is surely agreeing with me... unless I am getting rather confused?

>
> 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.

Very specifically not.  xc_domain_restore2() currently contains
domain-agnostic restoration code.  Yet-to-implement are
restore_x86_hvm() and restore_arm() which are expected to be in
restore_{x68_hvm,arm}.c.  It is possible that some of the current helper
functions in restore_x86_pv.c should be prompted to common.

This is explicitly to undo the current rats nest of code in
xc_domain_{save,restore}().  I don't know why the PV and HVM migration
code was merged together in the past, but they have almost nothing in
common other than the format of the page batches (not even the content),
and wedging the code together has resulted in functions substantially
more complicated than the sum of their useful parts.

~Andrew