[CRIU] [PATCH 2/7] pycriu: add python package, v3

[Pavel Emelyanov]

On 12/02/2014 05:22 PM, Ruslan Kuprieiev wrote:
> On 12/02/2014 04:16 PM, Pavel Emelyanov wrote:
>> On 12/02/2014 05:05 PM, Ruslan Kuprieiev wrote:
>>> On 12/02/2014 03:04 PM, Pavel Emelyanov wrote:
>>>>> diff --git a/pycriu/.gitignore b/pycriu/.gitignore
>>>>> new file mode 100644
>>>>> index 0000000..8d503da
>>>>> --- /dev/null
>>>>> +++ b/pycriu/.gitignore
>>>>> @@ -0,0 +1,3 @@
>>>>> +*_pb2.py
>>>>> +*.pyc
>>>>> +rpc.py
>>>>> diff --git a/pycriu/Makefile b/pycriu/Makefile
>>>>> new file mode 100644
>>>>> index 0000000..135fe1c
>>>>> --- /dev/null
>>>>> +++ b/pycriu/Makefile
>>>>> @@ -0,0 +1,16 @@
>>>>> +all: images rpc.py
>>>>> +
>>>>> +.PHONY: all images clean
>>>>> +
>>>>> +images:
>>>>> +	$(Q) $(MAKE) -C images all
>>>>> +
>>>>> +# rpc_pb2.py doesn't depend on any other file, so
>>>>> +# it is safe to rename it, dropping ugly _pb2 suffix.
>>>>> +rpc.py:
>>>>> +	$(Q) protoc -I=$(SRC_DIR)/protobuf/ --python_out=./ $(SRC_DIR)/protobuf/$(@:.py=.proto)
>>>>> +	$(Q) mv $(@:.py=_pb2.py) $@
>>>>
>>>> Why do we need rpc.py?
>>>
>>> Because it is handy to have all pythonic criu stuff in one
>>> package called pycriu.
>>
>> Hm... OK, will we then have a patch for p.haul that would get
>> rpc.py from CRIU rather than generating one itself?
>>
> 
> No, i think we should generate it and put into pycriu. Install pycriu
> along with criu and make p.haul import pycriu to have access to rpc
> and crit stuff.

Yes, this is what I meant, thanks :)

>>>>
>>>>> +clean:
>>>>> +	$(Q) $(MAKE) -C images clean
>>>>> +	$(Q) $(RM) rpc.py *.pyc
>>>>> diff --git a/pycriu/__init__.py b/pycriu/__init__.py
>>>>> new file mode 100644
>>>>> index 0000000..5205973
>>>>> --- /dev/null
>>>>> +++ b/pycriu/__init__.py
>>>>> @@ -0,0 +1,2 @@
>>>>> +import rpc
>>>>> +import images
>>>>> diff --git a/pycriu/images/.gitignore b/pycriu/images/.gitignore
>>>>> new file mode 100644
>>>>> index 0000000..234bfe9
>>>>> --- /dev/null
>>>>> +++ b/pycriu/images/.gitignore
>>>>> @@ -0,0 +1,4 @@
>>>>> +*.pyc
>>>>> +*_pb2.py
>>>>> +magic.py
>>>>> +pb.py
>>>>> diff --git a/pycriu/images/Makefile b/pycriu/images/Makefile
>>>>> new file mode 100644
>>>>> index 0000000..6aa115e
>>>>> --- /dev/null
>>>>> +++ b/pycriu/images/Makefile
>>>>> @@ -0,0 +1,26 @@
>>>>> +all: pb.py protobuf magic.py
>>>>> +
>>>>> +.PHONY: all protobuf clean pb.py
>>>>> +
>>>>> +proto := $(filter-out $(SRC_DIR)/protobuf/rpc.proto, $(wildcard $(SRC_DIR)/protobuf/*.proto))
>>>>> +proto-py-modules := $(foreach m,$(proto),$(subst -,_,$(notdir $(m:.proto=_pb2))))
>>>>> +
>>>>> +# We don't need rpc_pb2.py here, as it is not related to the
>>>>> +# images.
>>>>> +# Unfortunately, we can't drop ugly _pb2 suffixes here, because
>>>>> +# some _pb2 files depend on others _pb2 files.
>>>>> +protobuf:
>>>>> +	$(Q) protoc -I=$(SRC_DIR)/protobuf --python_out=./ $(proto)
>>>>> +
>>>>> +magic.py: $(SRC_DIR)/scripts/magic-gen.py $(SRC_DIR)/include/magic.h
>>>>> +	$(E) "  GEN  " $@
>>>>> +	$(Q) python $^ $@
>>>>> +
>>>>> +pb.py: protobuf
>>>>> +	$(Q) echo "# Autogenerated. Do not edit!" > $@
>>>>> +	$(Q) for m in $(proto-py-modules); do \
>>>>> +		echo "from $$m import *" >> $@ ;\
>>>>> +	done
>>>>> +
>>>>> +clean:
>>>>> +	$(Q) $(RM) ./*_pb2.py ./*.pyc magic.py pb.py
>>>>> diff --git a/pycriu/images/__init__.py b/pycriu/images/__init__.py
>>>>> new file mode 100644
>>>>> index 0000000..379943b
>>>>> --- /dev/null
>>>>> +++ b/pycriu/images/__init__.py
>>>>> @@ -0,0 +1,3 @@
>>>>> +from magic import *
>>>>> +from images import *
>>>>> +from pb import *
>>>>> diff --git a/pycriu/images/images.py b/pycriu/images/images.py
>>>>> new file mode 100644
>>>>> index 0000000..afcaa87
>>>>> --- /dev/null
>>>>> +++ b/pycriu/images/images.py
>>>>> @@ -0,0 +1,208 @@
>>>>> +#!/bin/env python
>>>>> +import io
>>>>> +import google
>>>>> +import struct
>>>>> +import os
>>>>> +import sys
>>>>> +
>>>>> +from magic import *
>>>>> +from pb import *
>>>>> +
>>>>> +class _cr_desc:
>>>>> +	def __init__(self, magic, head, body='', single=False):
>>>>
>>>> I'm not sure the single bit is needed. But what's needed is some
>>>> sign that image may have raw data inside. For example:
>>>>
>>>> # ../crit convert --in dump/static/pipe00/6596/1/pipes-data.img
>>>> Traceback (most recent call last):
>>>>     ...
>>>>     File "/usr/lib/python2.7/site-packages/google/protobuf/internal/decoder.py", line 685, in _RaiseInvalidWireType
>>>>       raise _DecodeError('Tag had invalid wire type.')
>>>> google.protobuf.message.DecodeError: Tag had invalid wire type.
>>>>
>>>> That's because crit tries todecode raw pipe data as message.
>>>>
>>>
>>> Not sure about single bit, but will try to optimize it.
>>
>> But you don't use one anyway :)
>>
> 
> It is used right below:
> +        self.magic_name = magic
> +        self.magic = eval(magic+"_MAGIC")
> +        self.head_name = head
> +        self.head = eval(head)
> +        if single:
> +            self.body_name = ''
> +            self.body = lambda : None
> +        else:
> +            if body == '':
> +                body = head
> +            self.body_name = body
> +            self.body = eval(body)

This usage doesn't seem to be correct. You should provide the
list of objects present in the image regardless of whether it's
a single-entry one or not :)

Oh, and while we're at it. You've made the 'body' to be optional
parameter, while in real images optional is the header one. Thus
I'd make the header be optional.

Thanks,
Pavel