Re: [Patch v6 02/13] tools/python: Scripts relating to migrationv2 streams

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

On 28/07/14 16:20, Ian Campbell wrote:
> On Mon, 2014-07-07 at 18:37 +0100, Andrew Cooper wrote:
>> * format.py contains structure and constants as per the stream specificion.
>>
>> * convert-legacy-stream.py will take a legacy migration stream as an input,
>>   and produce a v2 stream as an output.
>>
>> * verify-stream-v2.py will verify a stream against the v2 specification.
>>
>> format.py lives as part of the regular xen library, while
>> convert-legacy-stream and verify-stream-v2 are installed as standalone scripts
>> into PRIVATE_BINDIR.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> CC: Ian Campbell <Ian.Campbell@citrix.com>
>> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
>>
>> ---
>> v6:
>>  * Move to be part of tools/python and installed in proper locations
>> ---
>>  tools/python/Makefile                         |    5 +
>>  tools/python/scripts/convert-legacy-stream.py |  622 +++++++++++++++++++++++++
>>  tools/python/scripts/verify-stream-v2.py      |  456 ++++++++++++++++++
>>  tools/python/setup.py                         |    1 +
>>  tools/python/xen/streamv2/format.py           |  148 ++++++
> I think you could reasonably take the xen.migration namespace for the
> library bit of this, streamv2 isn't very meaningful in the long run.

Ok

>
> Have you developed any particular unit tests for the conversion stuff as
> you've been developing this? I think we could check them in if yes, so
> long as the input dataset isn't an entire domain of course...

andrewcoop@andrewcoop:/local/xen.git$ ls -hlA tools/python/v2tests/
total 285M
-rw-rw-r-- 1 andrewcoop xendev  65M Jul 11 11:35 conv-v1-t64-hvm
-rw-rw-r-- 1 andrewcoop xendev  40M Jul 11 11:35 conv-v1-t64-pv32
-rw-rw-r-- 1 andrewcoop xendev  40M Jul 11 11:35 conv-v1-t64-pv64
-rw-rw-r-- 1 andrewcoop xendev 1.1K Jul  6 20:06 Makefile
-rw-r--r-- 1 andrewcoop xendev  65M May  6 14:09 v1-t64-hvm
-rw-r--r-- 1 andrewcoop xendev  40M May  6 14:05 v1-t64-pv32
-rw-r--r-- 1 andrewcoop xendev  40M May  6 14:04 v1-t64-pv64

They are the smallest domains I could create and suspend, but are still
too large to reasonably be committed

I will see if I can come up with something smaller which can be
committed, but nothing comes immediately to mind.

>
>> +        elif marker == -1: # XC_SAVE_ID_ENABLE_VERIFY_MODE
> You could define names for all these.

I considered that, but they are used exactly once, and commented in
place.  I decided that, on balance, it was better without.

>
> I'm not really enough of a Pythonista to make any particularly
> insightful review comments, but it all looks fine to me.
>
>> +squahsed_page_data_records = 0
> "squashed" (repeatedly)
>
> Ian.
>

Oops.

~Andrew